build system: Add libstdcpp feature and doc by maribu · Pull Request #14503 · RIOT-OS/RIOT

maribu · 2020-07-12T21:03:16Z

Contribution description

Add libstdcpp feature to indicate a platform is providing a libstdc++ implementation ready for use
The existing cpp feature now only indicates a working C++ toolchain without libstdc++. (E.g. still useful for the Arduino compatibility layer.)
Added libstdcpp as required feature were needed
Added some documentation on C++ on RIOT

Testing procedure

Read the generated doc
make info-boards-supported should yield the same results as before (especially the C++ tests and the C++ example should be tested)

Issues/PRs references

Suggested in #14481 (comment)

Adds the documentation suggested in: #14466

maribu · 2020-07-12T21:03:53Z

This is going to need KConfig integration. Can someone give me a pointer?

benpicco

Looks good to me.
I think for Kconfig you need to add a config HAS_LIBSTDCPP to kconfigs/Kconfig.features and then select HAS_LIBSTDCPP in the CPU's Kconfig.

Nothing depends on HAS_CPP yet, so there is nothing else to update.

doc/doxygen/src/using-cpp.md

maribu · 2020-07-13T08:39:42Z

@benpicco and/or @leandrolanzieri: Can you verify the KConfig integration is correct? In the legacy system a dependency to libstdcpp also pulls in the cpp feature. I translated it to KConfig by having libstdcpp depending on cpp. This is however the first case a feature depends on another feature. While it logically makes sense (you can only use libstdc++ with C++ support), I'm not sure if there are underlying assumptions that features do not depend on other features that I violate now.

leandrolanzieri · 2020-07-14T07:26:06Z

@benpicco and/or @leandrolanzieri: Can you verify the KConfig integration is correct? In the legacy system a dependency to libstdcpp also pulls in the cpp feature. I translated it to KConfig by having libstdcpp depending on cpp. This is however the first case a feature depends on another feature. While it logically makes sense (you can only use libstdc++ with C++ support), I'm not sure if there are underlying assumptions that features do not depend on other features that I violate now.

I think there may be a confusion on how features are modelled currently in Kconfig. We are defining these HAS_ symbols and selecting them from the boards/CPUs that provide them. That means that actually the feature libstdcpp has no dependency in cpp to be provided (in the Makefile.features both have to be provided). The dependency somehow appears on the usage. And here is the difference between the way features are modelled in Kconfig and the way they are modelled in Makefiles:

Makefiles
Features are provided by the board or CPU. Features may be 'used', in the case they are used different things can happen:

periph_ modules are included directly as a shortcut
Other module's inclusion may be triggered depending on the used features

Kconfig
Most of the features provided by boards and CPUs will always be provided. Features could also be provided by modules or packages. Here there is no concept of 'used' features, instead modules depend on them. That means that in order to use the RTC driver one needs to enable the RTC driver module (e.g. MOD_PERIPH_RTC), which depends on the RTC feature being provided (e.g. HAS_PERIPH_RTC). Or in the case of C++ some MOD_CPP that enables support and depends on HAS_CPP should be enabled.

Therefore I think that the symbol HAS_LIBSTDCPP should not depend on HAS_CPP.

maribu · 2020-07-14T07:30:36Z

OK, how instead can we enforce that no platform provides HAS_LIBSTDCPP without providing HAS_CPP?

leandrolanzieri · 2020-07-14T07:46:44Z

OK, how instead can we enforce that no platform provides HAS_LIBSTDCPP without providing HAS_CPP?

But that is not the case currently using Makefiles right? The feature libstdcpp can be provided without having cpp. Problems only arise when the feature is used.

As we are using select in Kconfig to provide features, dependencies are not checked. One way to have the behaviour that you describe would be to keep the dependency of HAS_LIBSTDCPP and use imply instead of select. That should take the dependencies of the symbol into account. But again, I think what we want goes more in this direction:

config MOD_CPP
    bool "C++ support"
    depends on HAS_CPP

config MOD_LIBSTDCPP
    bool "C++ standard library support"
    depends on MOD_CPP && HAS_LIBSTDCPP

####

config HAS_CPP
    bool
    help
        Indicates that C++ is supported.

config HAS_LIBSTDCPP
    bool
    help
        Indicates that support of libstdc++ is available.

maribu · 2020-07-14T08:21:55Z

The feature libstdcpp can be provided without having cpp.

No. You cannot provide a libstdc++ without C++ support. And Murdock would not let a PR adding libstdcpp without cpp get merged.

But if I understood you correct, this will also be the case when using modules to select the features?

Should it be MOD_CPP or MODULE_CPP? It seems every else the prefix is MODULE_.

leandrolanzieri · 2020-07-14T08:36:53Z

The feature libstdcpp can be provided without having cpp.

No. You cannot provide a libstdc++ without C++ support. And Murdock would not let a PR adding libstdcpp without cpp get merged.

I think here you are referring to the fact that we have a test that uses the feature libstdcpp which pulls the feature cpp and that's why this is enforced right? But that actually would not prevent someone from adding FEATURES_PROVIDED += libstdcpp to the Makefile.features without providing cpp, right?

But if I understood you correct, this will also be the case when using modules to select the features?

I'm not sure I understood this. Yes, I think we should provide features from modules (e.g. to provide implementations of APIs as you pointed out in #14123 (comment)) when modules are modelled in Kconfig.

Should it be MOD_CPP or MODULE_CPP? It seems every else the prefix is MODULE_.

Yes, but those symbols are generated automatically. I was pointing out how I think it should be modelled when we model modules dependencies in Kconfig. I was not asking you to add the modules symbols on this PR as that is part of the next step in migration (we still don't handle module inclusion from Kconfig), we are not there just yet.

maribu · 2020-07-14T08:52:00Z

But that actually would not prevent someone from adding FEATURES_PROVIDED += libstdcpp to the Makefile.features without providing cpp, right?

Locally this could be done, but it would result in compile time checks in Murdock failing. So it is enforced that this does not happen in the upstream branch.

as that is part of the next step in migration

OK. I dropped the modules. We should keep in mind that during that step to also model the dependency, so that automatic compile time tests are failing if someone provides HAS_LIBSTDCPP without HAS_CPP.

miri64 · 2020-07-14T10:36:03Z

Side-note: could be interesting to re-test #14458 when this is merged, as a few of the problems there were also due to libstdcpp.

cpu/native/Makefile.features

fjmolinas · 2020-07-15T09:12:38Z

@leandrolanzieri @benpicco should we trigger murdock on this one now?

maribu · 2020-07-15T09:19:49Z

Btw: May I squash?

leandrolanzieri · 2020-07-15T09:42:30Z

From my side yes

- Add libstdcpp feature to indicate a platform is providing a libstdc++ implementation ready for use - The existing cpp feature now only indicates a working C++ toolchain without libstdc++. (E.g. still useful for the Arduino compatibility layer.) - Added libstdcpp as required feature were needed - Added some documentation on C++ on RIOT

On FreeBSD, libstdc++ is known to not work with -m32. Thus, we don't provide it feature libstdcpp there.

benpicco

Looks good to me too.

maribu · 2020-07-23T18:20:41Z

Thanks for the reviews!

miri64 · 2020-07-27T15:08:26Z

On FreeBSD, for examples/riot_and_cpp, tests/cpp_ctor, tests/rmutex_cpp, and tests/pkg_utensor compilation still fails due to missing / incompatible libstdc++. As cpp_ctor was used in #14598 to gauge if it works, I'm not sure this is a false negative.

/usr/local/bin/ld: skipping incompatible /usr/local/lib/gcc9/gcc/x86_64-portbld-freebsd12.1/9.3.0/../../../libstdc++.so when searching for -lstdc++
/usr/local/bin/ld: skipping incompatible /usr/local/lib/gcc9/gcc/x86_64-portbld-freebsd12.1/9.3.0/../../../libstdc++.a when searching for -lstdc++
/usr/local/bin/ld: cannot find -lstdc++

maribu · 2020-07-27T15:39:00Z

@miri: Can you test with -nodefaultlibs added to the linker flags?

miri64 · 2020-07-27T18:26:56Z

[@miri64]: Can you test with -nodefaultlibs added to the linker flags?

I knew I shouldn't have deleted that vagrant image ^^"

maribu · 2020-07-27T18:35:28Z

Don't worry. Our bug tracker and mailing list is open to FreeBSD users, if the ever need this working. (And IMO the FreeBSD bug tracker might be even a better place to complain for -m32 broken on their system.)

miri64 · 2020-07-27T18:37:39Z

Don't worry. Our bug tracker and mailing list is open to FreeBSD users, if the ever need this working. (And IMO the FreeBSD bug tracker might be even a better place to complain for -m32 broken on their system.)

The nice thing about vagrant, it just takes time to renew the image ;-), just takes a vagrant up. Currently doing that :-).

miri64 · 2020-07-27T18:55:46Z

@miri64: Can you test with -nodefaultlibs added to the linker flags?

As that also removes libc, I tried

diff --git a/boards/native/Makefile.include b/boards/native/Makefile.include
index 944d40653..911fc5761 100644
--- a/boards/native/Makefile.include
+++ b/boards/native/Makefile.include
@@ -51,6 +51,9 @@ ifeq ($(OS_ARCH),x86_64)
   LINKFLAGS += -m32
 endif
 ifeq ($(OS),FreeBSD)
+  ifeq ($(DISABLE_LIBSTDCPP),1)
+    LINKFLAGS += -nodefaultlibs -lc -lm -lpthread
+  endif
   ifeq ($(OS_ARCH),amd64)
     LINKFLAGS += -m32 -DCOMPAT_32BIT -L/usr/lib32 -B/usr/lib32
   endif

With this however, there are still link errors remaining:

[vagrant@freebsd ~/RIOT]$ make -C tests/cpp_ctors/
make: Entering directory '/home/vagrant/RIOT/tests/cpp_ctors'
-e \033[1;32mBuilding application "tests_cpp_ctors" for "native" with MCU "native".\033[0m
-e
"gmake" -C /home/vagrant/RIOT/boards/native
"gmake" -C /home/vagrant/RIOT/boards/native/drivers
"gmake" -C /home/vagrant/RIOT/core
"gmake" -C /home/vagrant/RIOT/cpu/native
"gmake" -C /home/vagrant/RIOT/cpu/native/periph
"gmake" -C /home/vagrant/RIOT/cpu/native/stdio_native
"gmake" -C /home/vagrant/RIOT/cpu/native/vfs
"gmake" -C /home/vagrant/RIOT/drivers
"gmake" -C /home/vagrant/RIOT/drivers/periph_common
"gmake" -C /home/vagrant/RIOT/sys
"gmake" -C /home/vagrant/RIOT/sys/auto_init
"gmake" -C /home/vagrant/RIOT/sys/embunit
"gmake" -C /home/vagrant/RIOT/sys/test_utils/interactive_sync
/usr/local/bin/ld: /home/vagrant/RIOT/tests/cpp_ctors/bin/native/application_tests_cpp_ctors.a(tests-cpp_ctors): in function `tests_cpp_ctors_static_value':
/home/vagrant/RIOT/tests/cpp_ctors/tests-cpp_ctors.cpp:29: undefined reference to `__cxa_guard_acquire'
/usr/local/bin/ld: /home/vagrant/RIOT/tests/cpp_ctors/tests-cpp_ctors.cpp:29: undefined reference to `__cxa_guard_release'
/usr/local/bin/ld: /home/vagrant/RIOT/tests/cpp_ctors/tests-cpp_ctors.cpp:29: undefined reference to `__cxa_guard_abort'
/usr/local/bin/ld: /home/vagrant/RIOT/tests/cpp_ctors/tests-cpp_ctors.cpp:29: undefined reference to `_Unwind_Resume'
/usr/local/bin/ld: /home/vagrant/RIOT/tests/cpp_ctors/bin/native/application_tests_cpp_ctors.a(tests-cpp_ctors):(.eh_frame+0x4b): undefined reference to `__gxx_personality_v0'
collect2: error: ld returned 1 exit status
make: *** [/home/vagrant/RIOT/tests/cpp_ctors/../../Makefile.include:572: /home/vagrant/RIOT/tests/cpp_ctors/bin/native/tests_cpp_ctors.elf] Error 1
make: Leaving directory '/home/vagrant/RIOT/tests/cpp_ctors'

maribu · 2020-07-27T19:46:00Z

/home/vagrant/RIOT/tests/cpp_ctors/tests-cpp_ctors.cpp:29: undefined reference to `__cxa_guard_acquire'
/usr/local/bin/ld: /home/vagrant/RIOT/tests/cpp_ctors/tests-cpp_ctors.cpp:29: undefined reference to `__cxa_guard_release'
/usr/local/bin/ld: /home/vagrant/RIOT/tests/cpp_ctors/tests-cpp_ctors.cpp:29: undefined reference to `__cxa_guard_abort'

Those could be fixed with USEMODULE += cxx_ctor_guards. (Those were also needed by ATmega and were implemented platform independent, in case other platforms might also need it.)

But I have no idea about the _Unwind_Resume and the __gxx_personality_v0 :-/

miri64 · 2020-07-28T07:40:54Z

But I have no idea about the _Unwind_Resume and the __gxx_personality_v0 :-/

I think I have a feeling, why there used to be a dedicated make target to build with debug flags https://stackoverflow.com/a/62744129 (see also #13423)

miri64 · 2020-07-28T07:43:33Z

Ok, reverting that change does not help...

miri64 · 2020-07-28T07:49:28Z

Booya!

diff --git a/boards/native/Makefile.include b/boards/native/Makefile.include
index 944d40653..fdc4ad34b 100644
--- a/boards/native/Makefile.include
+++ b/boards/native/Makefile.include
@@ -51,6 +51,9 @@ ifeq ($(OS_ARCH),x86_64)
   LINKFLAGS += -m32
 endif
 ifeq ($(OS),FreeBSD)
+  ifeq ($(DISABLE_LIBSTDCPP),1)
+    LINKFLAGS += -nodefaultlibs -lgcc_s -lc -lc++ -lm -lpthread
+  endif
   ifeq ($(OS_ARCH),amd64)
     LINKFLAGS += -m32 -DCOMPAT_32BIT -L/usr/lib32 -B/usr/lib32
   endif
diff --git a/cpu/native/Makefile.dep b/cpu/native/Makefile.dep
index a1afcf708..81cdae7b1 100644
--- a/cpu/native/Makefile.dep
+++ b/cpu/native/Makefile.dep
@@ -1,3 +1,12 @@
+ifeq (FreeBSD,$(OS))
+  ifneq ($(DISABLE_LIBSTDCPP),1)
+    # static C++ constructors need guards for thread safe initialization
+    ifneq (,$(filter cpp,$(FEATURES_USED)))
+      USEMODULE += cxx_ctor_guards
+    endif
+  endif
+endif
+
 ifneq (,$(filter periph_spi,$(USEMODULE)))
   USEMODULE += periph_spidev_linux
 endif

miri64 · 2020-07-28T07:49:51Z

Will run the tests again first though :-)

miri64 · 2020-07-28T09:24:56Z

All C++-related tests now succeed in #14458, so I will open a PR.

maribu added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: doc Area: Documentation Area: C++ Area: C++ wrapper Area: build system Area: Build system labels Jul 12, 2020

maribu requested review from aabadie, benpicco, gschorcht and leandrolanzieri July 12, 2020 21:03

maribu requested review from DipSwitch, MichelRottleuthner, fjmolinas, haukepetersen, jia200x, kaspar030, smlng and thomaseichinger as code owners July 12, 2020 21:03

benpicco reviewed Jul 12, 2020

View reviewed changes

doc/doxygen/src/using-cpp.md Outdated Show resolved Hide resolved

maribu requested a review from cgundogan as a code owner July 13, 2020 07:18

miri64 reviewed Jul 14, 2020

View reviewed changes

cpu/native/Makefile.features Outdated Show resolved Hide resolved

maribu force-pushed the cpp-feature branch from d76ea52 to cf482c5 Compare July 15, 2020 09:45

maribu added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jul 15, 2020

cgundogan added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jul 15, 2020

cpu/native: Workaround for libstdcpp for FreeBSD

0ed7ead

On FreeBSD, libstdc++ is known to not work with -m32. Thus, we don't provide it feature libstdcpp there.

benpicco approved these changes Jul 19, 2020

View reviewed changes

benpicco merged commit f3bce19 into RIOT-OS:master Jul 23, 2020

maribu deleted the cpp-feature branch July 23, 2020 18:20

maribu mentioned this pull request Jul 23, 2020

cpu/{mips32r2_common,arm7_common,native}/Kconfig: Select HAS_LIBSTDCPP #14597

Merged

miri64 mentioned this pull request Jul 28, 2020

native: fix C++ for FreeBSD #14631

Closed

benpicco mentioned this pull request Jul 30, 2020

PicoLibc support as alternative to Newlib(-nano) #12305

Closed

3 tasks

Conversation

maribu commented Jul 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

maribu commented Jul 12, 2020

Uh oh!

benpicco left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maribu commented Jul 13, 2020

Uh oh!

leandrolanzieri commented Jul 14, 2020

Uh oh!

maribu commented Jul 14, 2020

Uh oh!

leandrolanzieri commented Jul 14, 2020

Uh oh!

maribu commented Jul 14, 2020

Uh oh!

leandrolanzieri commented Jul 14, 2020

Uh oh!

maribu commented Jul 14, 2020

Uh oh!

miri64 commented Jul 14, 2020

Uh oh!

Uh oh!

fjmolinas commented Jul 15, 2020

Uh oh!

maribu commented Jul 15, 2020

Uh oh!

leandrolanzieri commented Jul 15, 2020

Uh oh!

benpicco left a comment

Choose a reason for hiding this comment

Uh oh!

maribu commented Jul 23, 2020

Uh oh!

miri64 commented Jul 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maribu commented Jul 27, 2020

Uh oh!

miri64 commented Jul 27, 2020

Uh oh!

maribu commented Jul 27, 2020

Uh oh!

miri64 commented Jul 27, 2020

Uh oh!

miri64 commented Jul 27, 2020

Uh oh!

maribu commented Jul 27, 2020

Uh oh!

miri64 commented Jul 28, 2020

Uh oh!

miri64 commented Jul 28, 2020

Uh oh!

miri64 commented Jul 28, 2020

Uh oh!

miri64 commented Jul 28, 2020

Uh oh!

miri64 commented Jul 28, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

maribu commented Jul 12, 2020 •

edited

Loading

benpicco left a comment •

edited

Loading

miri64 commented Jul 27, 2020 •

edited

Loading