Added option to disable Zephyrs C++ implementation

[vanwinkeljan]

Blocked by: PR #19002, without this PR libcxx tests will fail

---

Added a Kconfig option to disable Zephyrs C++ implementation for operator
new, delete, pure virtual functions and vtables in case stdlibc++ is used.

Main reason is that Zephyr implementation of new and delete uses k_malloc/k_free instead of malloc/free and this only in case a kernel heap size is defined.

[vanwinkeljan]

closing and opening PR as some CI checks were not executed

[zephyrbot]

All checks are passing now.

Review history of this comment for details about previous failed status.
Note that some checks might have not completed yet.

[pabigot]

In principle this appears to makes sense, but it increases the complexity of the relationships between the host of piecemeal C++ Kconfig options. Without a test or example it's not obvious how and when to use the new flag. (In fact we probably need a documentation page describing C++ features and which flags need to be turned on to support them.)

In an attempt to probe its effects I rebased this on top of #18242 and checked the C++ library test added in that commit. It fails to link on several platforms, diagnosing issues with orphan sections.

I would like to see #18242 merged first (since without it C++ in Zephyr is broken for non-Zephyr toolchains), then its test case updated or or another added to confirm that this does what it's intended to do.

[vanwinkeljan]

I would like to see #18242 merged first (since without it C++ in Zephyr is broken for non-Zephyr toolchains), then its test case updated or or another added to confirm that this does what it's intended to do

Ok once merge I will have a look at it and add a test case to see if new works without kernel space heap. Fyi I did some basic testing on nrf52 based boards (and native posix but that doesn't count).

Alternative would be to update the zephyr implementation to allow to select how the allocate memory (k_malloc vs malloc), but the you end up with a malloc based implementation that comes very close to the standard libc implementation.

[vanwinkeljan]

@pabigot Just rebased and had look at the failing tests.

Issues seems to be caused by the fact that libcxx is compiled with exceptions enabled.

On ARM platforms a section is added to store exception handling:

zephyr/include/arch/arm/cortex_m/scripts/linker.ld

Lines 215 to 223 in d0ef464

	#if defined (CONFIG_CPLUSPLUS)
	SECTION_PROLOGUE(.ARM.extab,,)
	{
	/*
	* .ARM.extab section containing exception unwinding information.
	*/
	*(.ARM.extab* .gnu.linkonce.armextab.*)
	} GROUP_LINK_IN(ROMABLE_REGION)
	#endif

Other platforms are either missing suche section in their linker files or are not defining them correctly (still have to double check).

The correct solution would be to include two version of libcxx in the toolchain, one without exceptions enabled and one with (in case CONFIG_EXCEPTIONS is set) and link against the correct version.

And just for completeness: using this PR on the nrf51 target results in 24kB extra ROM usage.

[pabigot]

Ugh.

Google suggests that llvm's libstc++ could possibly be built without exception support. I didn't quickly find evidence that can be done with GCC. Also, I'm not sure it's a good idea. I suspect CONFIG_LIB_CPLUSPLUS should require CONFIG_EXCEPTIONS, but I also haven't looked closely at how Zephyr works with exceptions to see whether this could work.

I suspect the linker sections need to be provided for all platforms.

I think it's fine to require a non-minimal libc for any C++ code that requires dynamic allocation, and the (GNU) toolchain versions are based on std::malloc. Why do we even need the overrides for new and delete? @Benichou34 can you provide insight?

[vanwinkeljan]

@pabigot

I suspect the linker sections need to be provided for all platforms.

Just a quick update on the linker sections if you add CONFIG_EXCEPTIONS to libcxx test on master, the same orphant sections errors pop-up.

Shall I go head and fix the linker scripts (in a new PR) or just create a new issue and link it to #18554?

[pabigot]

I think the linker section updates would fall under the support for exceptions that's currently embedded in #4028, so no need for an issue for that. At some point #4028 should probably be closed after creating new issues for whatever isn't already done.

Do please create an issue supporting this PR, though, and replace the "no issue, relates to heap alloc with real libc++" description in #18554 with a link to it. Especially if the purpose of this change is specifically to find a way to deal with new/delete that doesn't depend on the kernel heap API, because I'm not sure this is the right way to solve that. (It may be, I just haven't had a chance to comprehend the alternatives.)

[vanwinkeljan]

I'm not sure this is the right way to solve that

I agree here, a solution would be to use malloc/free instead of k_malloc/k_free so that at least the memory is acquired from the same pool for both new and malloc.

The next step could be to make the behaviour of malloc/free configurable to use one of the following scenarios:

1. kernel heap (k_malloc/k_free)
2. dedicated memory pool in kernel space
3. dedicated memory pool in user space
4. libc heap

[Benichou34]

@pabigot

new/delete are implemented to use C++ without libstdc++, and potentially without libc.

Perhaps the easiest way to deal with C++ dynamic alloation is :

• If no libc defined (and CONFIG_HEAP_MEM_POOL_SIZE > 0): new/delete uses k_malloc/k_free
• If a libc is defined, but not libstdc++: new/delete uses malloc/free
• If libstdc++ is used: new/delete uses the standard libstdc++ implementation

[vanwinkeljan]

Added test case to test zephys implementation

[andrewboie]

Why are these excluded?
This all looks really suspicious. Can we do the right thing with a "filter:" expression, or "min_ram:" or whatever it is that prevents the test running on these boards instead?

[pabigot]

Best guess from #19507 (comment) is this is either a docker issue or a toolchain issue related to multilib x86 toolchain builds.

[vanwinkeljan]

Good one on the min_ram I will update accordingly for 96b_meerkat96 colibri_imx7d_m4 warp7_m4 pico_pi_m4 with min_flash as it was a ROM size limitation.

For the qemu_x86_coverage qemu_x86_64 I have no idea and was copied from the initial test case in th yaml file.

The qemu_x86_long looks like a toolchain issue.

As @pabigot mentioned the arch_exclude: posix is a CI docker image limitation.

[andrewboie]

OK.

Do we have bugs open on the x86 issues?

[vanwinkeljan]

Do we have bugs open on the x86 issues?

I don't think there is an issue for qemu_x86_long toolchain problem.

Further did a quick test forqemu_x86_coverage and qemu_x86_64

qemu_x86_coverage just hangs

qemu_x86_64 fails to compile:

zephyr-sdk-0.10.3/x86_64-zephyr-elf/x86_64-zephyr-elf/include/c++/8.3.0/limits:1599:35: error: SSE register return with SSE disabled
       min() _GLIBCXX_USE_NOEXCEPT { return __FLT_MIN__; }
                                   ^

[pabigot]

I think #19735 will help with the x86 issues but for some reason it completely fails shippable right now.

[vanwinkeljan]

I think #19735 will help with the x86 issues but for some reason it completely fails shippable right now.

It did, update PR accordingly

[vanwinkeljan]

Rebased on top of latest master and removed qemu_x86_64 from platform_exclude list for exception tests