Skip to content

sys/cpp_new_delete: always enable the module when C++ is used#20348

Merged
maribu merged 1 commit intoRIOT-OS:masterfrom
benpicco:cpp_new_delete-always
Feb 7, 2024
Merged

sys/cpp_new_delete: always enable the module when C++ is used#20348
maribu merged 1 commit intoRIOT-OS:masterfrom
benpicco:cpp_new_delete-always

Conversation

@benpicco
Copy link
Copy Markdown
Contributor

@benpicco benpicco commented Feb 7, 2024

Contribution description

Newer toolchains appear to expect the presence of __dso_handle, so always enable the module.
The linker will collect unused sections anyway.

Testing procedure

sys/cpp_ctors builds again on Ubuntu 23.10.

On master it would produce

/usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/bin/ld: /usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/lib/thumb/v7e-m/nofp/libstdc++_nano.a(eh_globals.o): in function `_GLOBAL__sub_I___cxa_get_globals_fast':
./build_nano/thumb/v7e-m/nofp/libstdc++/libsupc++/../../../../../../src/libstdc++-v3/libsupc++/eh_globals.cc:85: undefined reference to `__dso_handle'
/usr/lib/gcc/arm-none-eabi/12.2.1/../../../arm-none-eabi/bin/ld: /home/benpicco/dev/RIOT/tests/sys/cpp_ctors/bin/nucleo-wl55jc/tests_cpp_ctors.elf: hidden symbol `__dso_handle' isn't defined

Issues/PRs references

@github-actions github-actions bot added Platform: AVR Platform: This PR/issue effects AVR-based platforms Area: cpu Area: CPU/MCU ports Area: sys Area: System labels Feb 7, 2024
@benpicco benpicco added Area: C++ Area: C++ wrapper CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Feb 7, 2024
@riot-ci
Copy link
Copy Markdown

riot-ci commented Feb 7, 2024

Murdock results

✔️ PASSED

453a8be sys/cpp_new_delete: always enable the module when C++ is used

Success Failures Total Runtime
10013 0 10015 09m:37s

Artifacts

@maribu maribu enabled auto-merge February 7, 2024 10:54
@maribu maribu added this pull request to the merge queue Feb 7, 2024
@maribu
Copy link
Copy Markdown
Member

maribu commented Feb 7, 2024

@MrKevinWeiss This might be something to backport. I think it is safe to assume that this doesn't break things, and hence, doesn't invalidate release testing.

@benpicco benpicco added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Feb 7, 2024
@MrKevinWeiss
Copy link
Copy Markdown
Contributor

MrKevinWeiss commented Feb 7, 2024

OK...

@MrKevinWeiss MrKevinWeiss mentioned this pull request Feb 7, 2024
Closed
86 tasks
@MrKevinWeiss
Copy link
Copy Markdown
Contributor

MrKevinWeiss commented Feb 7, 2024

Actually, since this is a fix for a specific distro I think it can land just in the 2024.01-branch and we will include it in future point releases if needed (it takes quite a while to get it in otherwise)...

@benpicco
Copy link
Copy Markdown
Contributor Author

benpicco commented Feb 7, 2024

It should become relevant when riotdocker switches to Ubuntu 24.04

@maribu
Copy link
Copy Markdown
Member

maribu commented Feb 7, 2024

For the CI the docker image is tagged. But a user running BUILD_IN_DOCKER=1 may expect the most recent docker image to just work.

Anyway, if it is in the 2024.01-branch, that is already good. I personally recommend going for the branch rather than the tag anyway, as the branch gets relevant fixes backported well after the release. The downside is that no regression tests are performed, unless a point release is made.

Merged via the queue into RIOT-OS:master with commit 4df5306 Feb 7, 2024
@MrKevinWeiss
Copy link
Copy Markdown
Contributor

MrKevinWeiss commented Feb 7, 2024

It should become relevant when riotdocker switches to Ubuntu 24.04

won't that be after the next release?

@benpicco benpicco deleted the cpp_new_delete-always branch February 7, 2024 13:16
@benpicco benpicco mentioned this pull request Feb 26, 2024
Merged
gdoffe added a commit to cogip/mcu-firmware that referenced this pull request Mar 14, 2024
@gdoffe
makefiles: fix -nostartfiles and up to C++20
df01b6f 
Presence of __dso_handle signal has been solved in RIOT by PR 20348.
Thus removal of -nostartfiles option is no more necessary.
Also bump C++ version to C++20.

[1] RIOT-OS/RIOT#20348
gdoffe added a commit to cogip/mcu-firmware that referenced this pull request Mar 30, 2024
@gdoffe
makefiles: fix -nostartfiles and up to C++20
e332a01 
Presence of __dso_handle signal has been solved in RIOT by PR 20348.
Thus removal of -nostartfiles option is no more necessary.
Also bump C++ version to C++20.

[1] RIOT-OS/RIOT#20348
gdoffe added a commit to cogip/mcu-firmware that referenced this pull request Apr 8, 2024
@gdoffe
makefiles: fix -nostartfiles
562b229 
Presence of __dso_handle signal has been solved in RIOT by PR 20348.
Thus removal of -nostartfiles option is no more necessary.
Also CXXEXFLAGS setup can be done before include of RIOT
Makefile.include

[1] RIOT-OS/RIOT#20348
gdoffe added a commit to cogip/mcu-firmware that referenced this pull request Apr 12, 2024
@gdoffe
makefiles: fix -nostartfiles
05b36d9 
Presence of __dso_handle signal has been solved in RIOT by PR 20348.
Thus removal of -nostartfiles option is no more necessary.
Also CXXEXFLAGS setup can be done before include of RIOT
Makefile.include

[1] RIOT-OS/RIOT#20348
gdoffe added a commit to cogip/mcu-firmware that referenced this pull request Apr 21, 2024
@gdoffe
makefiles: fix -nostartfiles
7da89e3 
Presence of __dso_handle signal has been solved in RIOT by PR 20348.
Thus removal of -nostartfiles option is no more necessary.
Also CXXEXFLAGS setup can be done before include of RIOT
Makefile.include

[1] RIOT-OS/RIOT#20348

Signed-off-by: Gilles DOFFE <[email protected]>
@MrKevinWeiss MrKevinWeiss added this to the Release 2024.04 milestone Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maribu maribu maribu approved these changes

@kYc0o kYc0o Awaiting requested review from kYc0o kYc0o is a code owner

@nandojve nandojve Awaiting requested review from nandojve nandojve is a code owner

Assignees

No one assigned

Labels

Area: C++ Area: C++ wrapper Area: cpu Area: CPU/MCU ports Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: AVR Platform: This PR/issue effects AVR-based platforms Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch

Projects

None yet

Milestone

Release 2024.04

Development

Successfully merging this pull request may close these issues.

4 participants

@benpicco @riot-ci @maribu @MrKevinWeiss