nxp-mcuxpresso / mcux-sdk

MCUXpresso SDK
BSD 3-Clause "New" or "Revised" License
340 stars 149 forks source link

[BUG] C/ELF/Unix constructors/destructors cannot work #208

Open stefanct opened 3 months ago

stefanct commented 3 months ago

Describe the bug In the MCUXpresso linker scripts the .init, .fini (and .preinit) sections are only enabled by cpp_info.ldt (i.e., for C++ projects) and almost(?) all startup functions within this SDK do not call __libc_init_array() unless they get compiled for C++. However, these sections are not C++-specific at all but usable in C as well (and predate C++ by quite some time). One can set respective attributes on the functions (e.g., __attribute__((constructor))) to enable execution before/after main().

To Reproduce

__attribute__((constructor)) void constr(void) {
    __asm volatile ("nop");
}

Set a break point on it and start debugging.

Expected behavior Execution stops at the function or a build error. However, neither happens. I have changed the startup code for my rt1021 to always call __libc_init_array() and the linker scripts to make it work as intended. Alternatively, I think, GCC could be built with --disable-initfini-array but I am not entirely sure if that would immediately throw a build error for such examples... and if it would affect C++.

Additional context https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-constructor-function-attribute https://refspecs.linuxbase.org/elf/gabi4+/ch4.sheader.html#special_sections https://www.ibm.com/docs/en/xl-c-and-cpp-linux/16.1.1?topic=attributes-constructor-destructor

Apart from fixing the linker scripts, this is approximately what's needed for the startup code: fix.txt (yes, uploading .patch files is broken, thanks m$)

nxpmike commented 2 months ago

hi @stefanct

per my test, it requires:

  1. change the startup_mimxrt1189_cm33.c as your fix.
  2. add link script, as attached.
  3. uncheck the "No startup or default libs(-nostdlib)", as attached. Otherwise with above 2 steps, it report "undefined reference to __libc_init_array"
  4. add missing function _lseek, _write, _read, _close. Otherwise with above 3 steps, it report "undefined reference to _lseek, _write, _read, _close"

BTW, my mcuxpresso version is 11.9.0, and default REDLIB is used.

what is your steps to make it work?

settings main_text.txt

stefanct commented 2 months ago

I am still working on a suitable solution but what you write is about what I have. Alternatively to implementing the system call functions (_write etc.) one can either add --specs=nosys.specs or -ffreestanding, I think. One can omit fini_array etc. usually because (C) destructors don't make a lot of sense in an embedded system.

redlib is not an option for us since it's closed-source and we need C++ support. Since my project's deadline is in a few weeks I don't have time to update and port all my workarounds... but AFAICT nothing relevant has changed between the versions anyway (at least in the SDK).

nxpmike commented 2 months ago

I'm afraid there is side effects with ”--specs=nosys.specs“ or ”-ffreestanding“, especially ”-ffreestanding“ significantly downgrade performance, both C++ and C get impacted.

Alternatively, step4 can be omitted, if you choose the newlibnano or newlib. lib

stefanct commented 2 months ago

You are referring to the -fno-builtin implied by -ffreestanding? But what would degrade performance with nosys?

nxpmike commented 2 months ago

first of all, with internal evaluation, we decide to not support this feature(attribute((constructor))) out of the box, because:

  1. there is conflict with default library setting(redlib[nohost-nf]), which requires additional manual settings/code change.
  2. it drags unnecessary additional library code which increase code size for all MCUs Interested users could manually enable it by changing project code and settings, which is introduced in this thread.

for your questions: -fno-builtin implied by -ffreestanding, is one factor, meanwhile I found the the final image size get bigger with -ffreestanding, see below picture(the BOARD_FLASH). "--specs=nosys.specs", it may has no impact on performance. I can see there is build warning. BTW, again, if you use newlib/newlibnano/redlib(none), then step4 is not needed.

image