esp-open-sdk icon indicating copy to clipboard operation
esp-open-sdk copied to clipboard

Enable libstdc++ v2

Open Sermus opened this issue 8 years ago • 43 comments

This is a reworked pull requiest for #144 after comments made by @pfalcon and @jcmvbkbc. The commits are better structured. The STL SGI tests are not included in this request because there is no way to build 1MB+ images in current version of esp-open-sdk.

Sermus avatar Apr 01 '16 06:04 Sermus

Seems the patches aren't sufficient to get esp_stdcpp_port/ and esp_newlib_port/ populated by git submodule {sync|update}... So, can you please explain how to add them to the submodule tree?

$ git submodule 
 46f160f7cdb6b993afca1a685ddff5b3aec656b6 crosstool-NG (crosstool-ng-1.22.0-53-g46f160f)
 5bd20afa6869b682001250e2a95c261390f1098d esp-open-lwip (remotes/origin/sdk-1.5.0)
-ab0b40c677d3f914533a1e1f98f422bc1bade68d esp_newlib_port
-d3008f47d087b1ad6e42ca135cf27f27c5845e16 esp_stdcpp_port
 5eb1563501211b6661d00582eb280df0c96e3f82 esptool (remotes/origin/esp-open-sdk)
 e4bcc63c9c016e4f8848e7e8f512438ca857531d lx106-hal (heads/master)

susisstrolch avatar Apr 02 '16 09:04 susisstrolch

Not sure what exactly you're trying to do. Cloning with recursive submodule checkout seems to work fine: git clone --recursive -b enable_libstdc++_v2 https://github.com/Sermus/esp-open-sdk.git

Sermus avatar Apr 02 '16 10:04 Sermus

For YOUR repo - yes. But not when applying this pull request against pfalcon/esp-open-sdk: Having a clean local clone from pfalcon/esp-open-sdk. Applying your pull request localy:

git fetch origin pull/146/head:libstdc++v2
git checkout libstdc++v2
git submodule sync
git submodule update

Result: as shown in my last message - your submodules aren't pulled in, so the build will fail badly:

...
Installing vendor SDK headers into toolchain sysroot
Installing vendor SDK libs into toolchain sysroot
Installing vendor SDK linker scripts into toolchain sysroot
Installing port lib headers into toolchain sysroot
cp: cannot stat ‘esp_stdcpp_port/cpp_routines.h’: No such file or directory
make: *** [standalone] Error 1

susisstrolch avatar Apr 02 '16 14:04 susisstrolch

Shouldn't have you used "git submodule update --init" since these two are new to your .git/config?

Sermus avatar Apr 02 '16 15:04 Sermus

Thanks!

susisstrolch avatar Apr 03 '16 06:04 susisstrolch

I've started trying to play with this. So far I have run into a few issues

  • I would prefer do_global_ctors() to be declared extern "C" (or just defined in a .c file), because I want to call it from my user_main.c file. It may just be me, but it feels wrong to be 'in' c++ before the global constructors have been called, since the global constructors in a translation unit are supposed to have been called by the time a call into that translation unit runs:

It is implementation-defined whether or not the dynamic initialization (8.5, 9.4, 12.1, 12.6.1) of an object of namespace scope is done before the first statement of main. If the initialization is deferred to some point in time after the first statement of main, it shall occur before the first use of any function or object defined in the same translation unit as the object to be initialized.

  • Attempting to link -lstdc++port is causing me duplicate symbol errors:
xtensa-lx106-elf/sysroot/usr/lib\libstdc++port.a(cpp_routines.o): In function `operator new(unsigned int)':
cpp_routines.cpp:(.text._Znwj+0x0): multiple definition of `operator new(unsigned int)'
xtensa-lx106-elf/sysroot/lib\libstdc++irom.a(new_op.o):esp-open-sdk/crosstool-NG/.build/src/gcc-4.8.5/libstdc++-v3/libsupc++/new_op.cc:45: first defined here
xtensa-lx106-elf/bin/../xtensa-lx106-elf/sysroot/usr/lib\libstdc++port.a(cpp_routines.o): In function `operator delete(void*)':
cpp_routines.cpp:(.text._ZdlPv+0x0): multiple definition of `operator delete(void*)'
xtensa-lx106-elf/bin/../xtensa-lx106-elf/sysroot/lib\libstdc++irom.a(del_op.o):esp-open-sdk/crosstool-NG/.build/src/gcc-4.8.5/libstdc++-v3/libsupc++/del_op.cc:45: first defined here
  • It appears that libstdc++irom.a did not actually wind up having its symbols moved to the .irom.text section. I believe this is due to libstdc++ having been built with -ffunction-sections -fdata-sections, and the objcopy rename section only matching exact section names (not the prefix, i.e. only .text not .text.somefunction sections get renamed).

jeremyd2019 avatar Apr 06 '16 05:04 jeremyd2019

Thanks for the feedback. I started looking at the issue 1 and end up by reworking stdcpp_port throwing away garbage no more needed there. This fixed first two issues from your comment. Please pull in updates.

As for the third one, you're right. libstdc++ is built sectionized and objcopy doesn't move content of those sections to .irom.text. I didn't find a way to make objcopy do such thing because it doesn't accept wildcards in section name specifier. Doing this section-by-section looks overexpensive. This is the reason why i created romized versions of linker scripts which make .text.* and .literal.* sections go to irom during link phase. If you have a better option please let me know.

Sermus avatar Apr 06 '16 07:04 Sermus

I played with this a little and had some success by modifying the linker script as follows to just put text and literal sections from libstdc++irom.a into irom. I noticed you were already doing something similar with libm.a in your ld-script-irom.patch.

--- xtensa-lx106-elf/xtensa-lx106-elf/sysroot/usr/lib/eagle.app.v6.ld   2016-04-06 10:29:05.166730000 -0700
+++ xtensa-lx106-elf/xtensa-lx106-elf/sysroot/usr/lib/eagle.app.v6.test.ld  2016-04-06 22:57:08.686971600 -0700
@@ -102,11 +102,14 @@
     *(.gnu.linkonce.e.*)
     *(.gnu.version_r)
     *(.eh_frame)
+    . = (. + 3) & ~ 3;
     /*  C++ constructor and destructor tables, properly ordered:  */
+    __init_array_start = ABSOLUTE(.);
     KEEP (*crtbegin.o(.ctors))
     KEEP (*(EXCLUDE_FILE (*crtend.o) .ctors))
     KEEP (*(SORT(.ctors.*)))
     KEEP (*(.ctors))
+    __init_array_end = ABSOLUTE(.);
     KEEP (*crtbegin.o(.dtors))
     KEEP (*(EXCLUDE_FILE (*crtend.o) .dtors))
     KEEP (*(SORT(.dtors.*)))
@@ -151,6 +154,14 @@
   } >dram0_0_seg :dram0_0_bss_phdr
 /* __stack = 0x3ffc8000; */

+  .irom0.text : ALIGN(4)
+  {
+    _irom0_text_start = ABSOLUTE(.);
+    *(.irom0.literal .irom.literal .irom.text.literal .irom0.text .irom.text)
+    *libstdc++irom.a:(.literal .text .literal.* .text.*)
+    _irom0_text_end = ABSOLUTE(.);
+  } >irom0_0_seg :irom0_0_phdr
+
   .text : ALIGN(4)
   {
     _stext = .;
@@ -198,13 +209,6 @@
     *(.gnu.linkonce.lit4.*)
     _lit4_end = ABSOLUTE(.);
   } >iram1_0_seg :iram1_0_phdr
-
-  .irom0.text : ALIGN(4)
-  {
-    _irom0_text_start = ABSOLUTE(.);
-    *(.irom0.literal .irom.literal .irom.text.literal .irom0.text .irom.text)
-    _irom0_text_end = ABSOLUTE(.);
-  } >irom0_0_seg :irom0_0_phdr
 }

 /* get ROM code address */

jeremyd2019 avatar Apr 07 '16 06:04 jeremyd2019

Perhaps it would even make sense to put *irom.a:(.text .literal .text.* .literal.*) in there, and then the user could just (re)name libs they wanted to put in the irom section libsomethingirom.a.

jeremyd2019 avatar Apr 07 '16 06:04 jeremyd2019

Yeah, this would work. At the same time i'm really in doubt even introducing the original changes into the linker scripts. Generally, doing so we bury a requirement to how the user application shall be built into a linker script, this smells bad. On the other hand, we seem to have no way out. Either we do this way or we don't do at all.

Sermus avatar Apr 07 '16 06:04 Sermus

Now i have a particular case of using this buried knowledge. I tried to build SGI STL sample and fail to link because implementations generated from templates are quite big and wouldn't fit into iram. The fix was quite easy: i preliminarily packed the compiler objects into sgitestsirom.a so that it applies to the linker script rule. But i can admit than this is definitely better than making .text.* go to irom I initially introduced in the linker scripts. So, thanks for the feedback, i'll incorporate your suggestion into the linker script patch.

Sermus avatar Apr 07 '16 07:04 Sermus

I agree on not being overly fond of making a magic name trigger this behavior. Learning from investigating this, I think I am of the opinion that any reasonably complex project would probably need to make its own custom ld script to make sure everything goes where it needs to. The magic name trick might be enough for a simple project, or to get somebody started when they run out of iram.

jeremyd2019 avatar Apr 08 '16 05:04 jeremyd2019

Yes, exactly. This gives at least a good hint where to go when you face such an issue.

Sermus avatar Apr 08 '16 05:04 Sermus

Just for information. I did a build test for Micropython project with the new environment. The built binary is different from what is built with the current esp-open-sdk head revision (mainly because gcc changed from 4.8.2 to 4.8.5 I suppose). The binary is loaded into esp8266. It seems to work as expected. At least print("Hello, World"); does what it has to.

Sermus avatar Apr 08 '16 07:04 Sermus

@Sermus: Great, thanks! I don't have time to look in more detail at your patches currently, but my comments would be exactly along the way of "how many projects with good testsuite coverage were tested with these patches?"

pfalcon avatar Apr 08 '16 08:04 pfalcon

Can you recommend more or less representative project set you usually use for such purposes? I'll do my best to check if they are ok with my changes.

Sermus avatar Apr 08 '16 08:04 Sermus

Can you recommend more or less representative project set you usually use for such purposes? I'll do my best to check if they are ok with my changes.

MicroPython is one such project I'm personally working on and which has extensive testsuite coverage (>90%, used to be ~95%). Note that it's currently under fast-paced development and to back it, the rock solid toolchain is required (you just can't vary all possible things at once).

There're other popular ESP8266 projects, like NodeMCU or Arduino, but I have no idea about their test process and coverage.

pfalcon avatar Apr 10 '16 14:04 pfalcon

Except upmerging crosstool-NG the changes i'm pushing in are more "addon" than "change". As I wrote in one of my previous comments i'm trying to keep backward compatibility in every possible aspect. Except switching crosstool-NG, the changes are:

  • Adding romized versions of libstdc++ and libm.
  • Adding romized versions of linker scripts.
  • Adding two new libs
  • Adding STL tests as an example

"Adding" means i don't force people use them. They may not use them keeping functionality/source and binary layout/build procedures as they were before. IMHO the only thing we need to focus to make ourselves sure that the patch doesn't break is checking if crosstool-NG components still work as intended. I just wanted to be sure we mean the same saying this shall be tested.

Since you're one of the contributors to MicroPython could you please advice how i can run the representative test suite on ESP8266 (and what is "the representative test suite" since AFAIU not all the functionality is supported)?

Sermus avatar Apr 11 '16 04:04 Sermus

how i can run the representative test suite on ESP8266

Please see here: http://forum.micropython.org/viewtopic.php?f=16&t=1731

(and what is "the representative test suite" since AFAIU not all the functionality is supported)?

Wikipedia has good coverage of this question: https://en.wikipedia.org/wiki/Code_coverage

Other news is that esptool was switched to the upstream after all (thanks to your feedback and discussion with its maintainer).

pfalcon avatar Apr 22 '16 18:04 pfalcon

)))) no need to explain what the term code coverage means in general. The question was about the micropython test suite you could consider representative. I've got the answer though, many thanks. I'll run the test suite and let you know the results.

Sermus avatar Apr 22 '16 18:04 Sermus

Simple question - simple answer. "Representative" word was related to "various projects", not just MicroPython, and meant something "one which exercises vast majority of project code". As you can see from a badge on https://github.com/micropython/micropython , currently, 91% of MicroPython code is covered by its testsuite, in some definition of "covered". Wikipedia article gives good overview of various "coverage" criteria, from which it's clear that one which is used for non-mission-critical projects, is the weakest possible. Moral of that: there can't be too much testing, and usually there's few to none. Testing on several projects with testsuites of MicroPython's level (90%+ in terms of gcoverage) should be good enough though.

pfalcon avatar Apr 22 '16 18:04 pfalcon

Hello Paul, I ran micropython's test suites for the versions built with the mainstream toolchain and with the c++-enabled one. The results are identical.

For me the test suite reports: 412 tests performed (13321 individual testcases) 412 tests passed 48 tests skipped: builtin_compile class_descriptor exception_chain fun_name ordereddict1 parser slice_attrs string_splitlines machine1 machine_mem urandom_extra vfs_fat_ramdisk bytearray_construct bytes_construct cmath_fun cmath_fun_special complex1 float2int float2int_doubleprec float_divmod int_big_float math_fun_special string_format true_value types meminfo memstats native_closure native_const native_misc viper_addr viper_args viper_binop_arith viper_binop_comp viper_binop_comp_imm viper_binop_multi_comp viper_cond viper_error viper_misc viper_ptr16_load viper_ptr16_store viper_ptr32_load viper_ptr32_store viper_ptr8_load viper_ptr8_store viper_subscr rge_sm sys_exc_info

Sermus avatar May 01 '16 12:05 Sermus

Thanks for testing that, your contribution and your patience! We should get initial release of MicroPython esp8266 next week, and that should unfreeze changes to esp-open-sdk. It may still take some time (and there're other things in queue first like upgrade to SDK 1.5.3), but we're getting there.

pfalcon avatar May 01 '16 16:05 pfalcon

Hi Paul, Is there any plan for integration window? Should I do upmerge?

Sermus avatar May 13 '16 07:05 Sermus

What do you mean by upmerge? Definitely please rebase your branch against master when you have a chance (github shows there're conflicts). Unfortunately, I'm still backlogged on looking into this. Well, your patch being merged into any open-source project means that a maintainer is ready to maintain your code, or that you yourself will be around to answer any issues. Both take time, and thanks for doing the 2nd part ;-). I hope to have some progress on the weekend (at least create a separate branch and merge something there).

pfalcon avatar May 13 '16 10:05 pfalcon

Yes, i meant to say pulling recent changes from master in and resolving conflicts. The reason for the question was my natural laziness. I wouldn't like to do the same twice meaning if i rebase now and the request won't be merged anytime soon after most likely i'll be forced to rebase once again. That's why i wonder when you're ready to pull in the changes. Then i'll rebase and do final clearence.

Sermus avatar May 13 '16 10:05 Sermus

Feel free to put it off for now.

pfalcon avatar May 13 '16 10:05 pfalcon

Sorry for making noise around this, but how do i know when this should be done? The only way i see is polling you from time to time. Last time you gave clear evidence when the next poll shall be performed.

Sermus avatar May 13 '16 10:05 Sermus

I'll write something here.

pfalcon avatar May 13 '16 11:05 pfalcon

Ok, let's start like this: https://github.com/pfalcon/esp-open-sdk/commits/gcc-4.8.5 . Feel free to rebase against that branch at your convenience. No hurry - I'll need to test switch to gcc 4.8.5 well first, then proceed with the rest. Thanks.

pfalcon avatar May 13 '16 20:05 pfalcon