Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add non-archived dependencies matcher to linker for PlatformIO #4355

Closed

Conversation

kylefleming
Copy link
Contributor

Ensures that dependencies with the build setting "libArchive": false (e.g. GDBStub) are linked properly when using PlatformIO.

Basic Infos

Hardware

Hardware: d1 mini
Core Version: master

Description

When using PlatformIO, GDBStub doesn't work properly. It throws a compile error with the d1 mini, which I've included below.

However, even if the alignment is fixed for those methods with .align 4 in gdbstub-entry.S, there is still an underlying issue. The true issue is that the gdbstub functions that are labeled with ATTR_GDBFN (aka ICACHE_RAM_ATTR) aren't actually being stored in iram, which also means that the ISR handlers won't be called. This is caused by GDBStub requiring that libArchive be set to false in the library's build settings (for reasons related to weak function aliasing, see #3488), thus no longer matching the existing PlatformIO linker patterns in the linker file.

This pull request addresses that and fixes #3903, which reports the same issue.

Note: I also fixed a typo in the linker file related to not properly escaping a backslash.

platformio.ini

[env:d1_mini]
platform = espressif8266
framework = arduino
board = d1_mini

Sketch

#include <Arduino.h>

#include <GDBStub.h>

void setup() {
}

void loop() {
}

Debug Messages

> Executing task: platformio run <

[Mon Feb 12 18:06:58 2018] Processing d1_mini (platform: espressif8266; board: d1_mini; framework: arduino)
--------------------------------------------------------------------------------------------------------------------------------
Verbose mode can be enabled via `-v, --verbose` option
PLATFORM: Espressif 8266 > WeMos D1 R2 & mini
SYSTEM: ESP8266 80MHz 80KB (4MB Flash)
Library Dependency Finder -> http://bit.ly/configure-pio-ldf
LDF MODES: FINDER(chain) COMPATIBILITY(light)
Collected 26 compatible libraries
Scanning dependencies...
Dependency Graph
|-- <GDBStub> v0.2
Compiling .pioenvs/d1_mini/lib294/GDBStub/internal/gdbstub-entry.o
Compiling .pioenvs/d1_mini/lib294/GDBStub/internal/gdbstub.o
Compiling .pioenvs/d1_mini/src/main.o
Archiving .pioenvs/d1_mini/libFrameworkArduinoVariant.a
Indexing .pioenvs/d1_mini/libFrameworkArduinoVariant.a
Compiling .pioenvs/d1_mini/FrameworkArduino/Esp.o
Compiling .pioenvs/d1_mini/FrameworkArduino/FS.o
Compiling .pioenvs/d1_mini/FrameworkArduino/FunctionalInterrupt.o
Compiling .pioenvs/d1_mini/FrameworkArduino/HardwareSerial.o
Compiling .pioenvs/d1_mini/FrameworkArduino/IPAddress.o
Compiling .pioenvs/d1_mini/FrameworkArduino/MD5Builder.o
Compiling .pioenvs/d1_mini/FrameworkArduino/Print.o
Compiling .pioenvs/d1_mini/FrameworkArduino/Schedule.o
Compiling .pioenvs/d1_mini/FrameworkArduino/Stream.o
Compiling .pioenvs/d1_mini/FrameworkArduino/StreamString.o
Compiling .pioenvs/d1_mini/FrameworkArduino/Tone.o
Compiling .pioenvs/d1_mini/FrameworkArduino/Updater.o
Compiling .pioenvs/d1_mini/FrameworkArduino/WMath.o
Compiling .pioenvs/d1_mini/FrameworkArduino/WString.o
Compiling .pioenvs/d1_mini/FrameworkArduino/abi.o
Compiling .pioenvs/d1_mini/FrameworkArduino/base64.o
Compiling .pioenvs/d1_mini/FrameworkArduino/cbuf.o
Compiling .pioenvs/d1_mini/FrameworkArduino/cont.o
Compiling .pioenvs/d1_mini/FrameworkArduino/cont_util.o
Compiling .pioenvs/d1_mini/FrameworkArduino/core_esp8266_eboot_command.o
Compiling .pioenvs/d1_mini/FrameworkArduino/core_esp8266_flash_utils.o
Compiling .pioenvs/d1_mini/FrameworkArduino/core_esp8266_i2s.o
Compiling .pioenvs/d1_mini/FrameworkArduino/core_esp8266_main.o
Compiling .pioenvs/d1_mini/FrameworkArduino/core_esp8266_noniso.o
Compiling .pioenvs/d1_mini/FrameworkArduino/core_esp8266_phy.o
Compiling .pioenvs/d1_mini/FrameworkArduino/core_esp8266_postmortem.o
Compiling .pioenvs/d1_mini/FrameworkArduino/core_esp8266_si2c.o
Compiling .pioenvs/d1_mini/FrameworkArduino/core_esp8266_timer.o
Compiling .pioenvs/d1_mini/FrameworkArduino/core_esp8266_wiring.o
Compiling .pioenvs/d1_mini/FrameworkArduino/core_esp8266_wiring_analog.o
Compiling .pioenvs/d1_mini/FrameworkArduino/core_esp8266_wiring_digital.o
Compiling .pioenvs/d1_mini/FrameworkArduino/core_esp8266_wiring_pulse.o
Compiling .pioenvs/d1_mini/FrameworkArduino/core_esp8266_wiring_pwm.o
Compiling .pioenvs/d1_mini/FrameworkArduino/core_esp8266_wiring_shift.o
Compiling .pioenvs/d1_mini/FrameworkArduino/debug.o
Compiling .pioenvs/d1_mini/FrameworkArduino/heap.o
Compiling .pioenvs/d1_mini/FrameworkArduino/libb64/cdecode.o
Compiling .pioenvs/d1_mini/FrameworkArduino/libb64/cencode.o
Compiling .pioenvs/d1_mini/FrameworkArduino/libc_replacements.o
Compiling .pioenvs/d1_mini/FrameworkArduino/pgmspace.o
Compiling .pioenvs/d1_mini/FrameworkArduino/sntp-lwip2.o
Compiling .pioenvs/d1_mini/FrameworkArduino/spiffs/spiffs_cache.o
Compiling .pioenvs/d1_mini/FrameworkArduino/spiffs/spiffs_check.o
Compiling .pioenvs/d1_mini/FrameworkArduino/spiffs/spiffs_gc.o
Compiling .pioenvs/d1_mini/FrameworkArduino/spiffs/spiffs_hydrogen.o
Compiling .pioenvs/d1_mini/FrameworkArduino/spiffs/spiffs_nucleus.o
Compiling .pioenvs/d1_mini/FrameworkArduino/spiffs_api.o
Compiling .pioenvs/d1_mini/FrameworkArduino/spiffs_hal.o
Compiling .pioenvs/d1_mini/FrameworkArduino/time.o
Compiling .pioenvs/d1_mini/FrameworkArduino/uart.o
Compiling .pioenvs/d1_mini/FrameworkArduino/umm_malloc/umm_malloc.o
Archiving .pioenvs/d1_mini/libFrameworkArduino.a
Indexing .pioenvs/d1_mini/libFrameworkArduino.a
Linking .pioenvs/d1_mini/firmware.elf
.pioenvs/d1_mini/lib294/GDBStub/internal/gdbstub.o: In function `gdbReadCommand$part$3':
gdbstub.c:(.iram.text+0x6f9): dangerous relocation: call0: misaligned call target: gdbstub_set_hw_breakpoint
gdbstub.c:(.iram.text+0x796): dangerous relocation: call0: misaligned call target: gdbstub_del_hw_watchpoint
collect2: error: ld returned 1 exit status
*** [.pioenvs/d1_mini/firmware.elf] Error 1
================================================== [ERROR] Took 4.20 seconds ==================================================

Ensures that dependencies with the build setting `"libArchive": false` (e.g. GDBStub) are linked properly when using PlatformIO.
@kylefleming kylefleming changed the title Add non-archived dependencies matcher to linker Add non-archived dependencies matcher to linker for PlatformIO Feb 13, 2018
@ivankravets
Copy link
Collaborator

@kylefleming could you navigate to ~/.platformio/packages/framework-arduinoespressif8266/libraries/GDBStub and remove library.json or rename to library.json.bak?

Now, please switch to upstream/development version of PIO Core via pio upgrade --dev command.

Does it work properly now?

@kylefleming
Copy link
Contributor Author

@ivankravets Yep, it seems to work. I assume this is related to fixing weak linking symbols from archives as described here: #3321 (comment)?

The issue about "libArchive": false libraries still stands though. If a library isn't archived it won't get caught by the linker.

@ivankravets
Copy link
Collaborator

We had a discussion about this issue with @igrr and library.json was a temporary solution. We changed an order for archived libs when linking final firmware in PlatformIO Core 3.5.2 (is planned for release next week). So, now it should have the same behavior of linking (searching for symbols) as Arduino IDE: project source files > user libraries > framework libraries > built-in libraries.

P.S: I fixed the issue with a missed slash in LD and removed our manifest. See #4363

@kylefleming
Copy link
Contributor Author

@ivankravets This PR still addresses a valid issue. The issue is related to using "libArchive": false which any library could still use, so making GDBStub an archive again only means that no library currently exhibits that behavior. Unless you plan on getting rid of the libArchive flag, I think this PR should still be considered.

The issue, to recap, is that .literal* and .text* symbols in a non-archived platformio library will go in iram instead of flash (because it would fall through and match the catch-all on this line). Flash is where they would go with an archived platformio library (here) or a non-platformio arduino library (here). The behavior should be consistent regardless of where it came from, otherwise you get bugs like what I described above with GDBStub.

For reference, the GDBStub libraries (with platformio environment named d1_mini) look like this:

.pioenvs/d1_mini/lib294/GDBStub/internal/gdbstub-entry.o
.pioenvs/d1_mini/lib294/GDBStub/internal/gdbstub.o

Hence using this matcher.

(Note: I'm ignoring the .iram.text part found here since #4356 will fix that part of it)

@ivankravets ivankravets reopened this Feb 15, 2018
@ivankravets
Copy link
Collaborator

@kylefleming What will be if a library has nested folders instead of 1-level structure?

Would be good to find a solution here #4356 (comment)

ivankravets added a commit that referenced this pull request Feb 20, 2018
Drop PlatformIO lines from LD script; append object suffix to the end of target name // Resolve #4355
@ivankravets
Copy link
Collaborator

Resolved in #4399

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linker error with GDBStub
2 participants