examples/natmod/btree: Fix build on RV32 with Picolibc. #17642
Conversation
|
Code size report: |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #17642 +/- ##
==========================================
+ Coverage 98.23% 98.44% +0.21%
==========================================
Files 171 171
Lines 22140 22192 +52
==========================================
+ Hits 21749 21847 +98
+ Misses 391 345 -46 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Very nice! |
examples/natmod/btree/Makefile
Outdated
| @@ -36,6 +36,10 @@ ifeq ($(ARCH),xtensa) | |||
| MPY_EXTERN_SYM_FILE=$(MPY_DIR)/ports/esp8266/boards/eagle.rom.addr.v6.ld | |||
| endif | |||
|
|
|||
| ifeq ($(ARCH),rv32imc) | |||
| CFLAGS += -D__PICOLIBC_ERRNO_FUNCTION=__errno | |||
| endif | |||
There was a problem hiding this comment.
Maybe this can go in py/dynruntime.mk so it's available for everyone? It seems pretty harmless to add unconditionally.
There was a problem hiding this comment.
Not totally sure on that. This works for btree only because there's this bit in the natmod's code:
int native_errno;
#if defined(__linux__)
int *__errno_location (void)
#else
int *__errno (void)
#endif
{
return &native_errno;
}Hardcoding the function name for something like this maybe it's not the best way, especially if you've had to reimplement errno's behaviour to be able to integrate a much more complex bit of code in a natmod. Sure, this can work through another layer of indirection using a configuration define so you can easily change __errno with something else only if picolibc is enabled, but at this point we're just recreating CMake and not in a good way :)
Something I can do, though, is make this define unconditional for this specific natmod - if you use picolibc for Arm or Xtensa you may still encounter the same issue if picolibc was built with TLS support enabled and its errno implementation is configured to use it.
Oh, I almost forgot, I should probably document this behaviour somewhere in docs/develop/natmod.rst as it's not entirely obvious how to solve this issue if you encounter it and expect something as simple as errno to just work.
Edit: Done!
There was a problem hiding this comment.
Something I can do, though, is make this define unconditional for this specific natmod
OK, that's a good solution. The btree natmod is after all the only one using errno.
Oh, I almost forgot, I should probably document this behaviour somewhere in
docs/develop/natmod.rst
Docs update looks good!
This commit fixes building the "btree" example natmod on RV32 when Picolibc is being used and uses thread-local storage for storing the errno variable. The fix is surprisingly simple: Picolibc allows overriding the function that will provide a pointer to the "errno" variable, and the btree natmod integration code already has all of this machinery set up as part of its library integration. Redirecting Picolibc to the already existing pointer provider function via a compile-time definition is enough to let the module compile and pass QEMU tests. This workaround will work on any Picolibc versions (Arm, RV32, Xtensa, etc.) even if TLS support was not enabled to begin with, and will effectively do nothing if the toolchain used will rely on Newlib to provide standard C library functions. Given that the btree module now builds and passes the relevant natmod tests, said module is now part of the QEMU port's natmod testing procedure, and CI now will build the btree module for RV32 as part to its checks. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
7a04586 to
b5a8ff5
Compare
This commit adds some documentation on what are the limitations of using Picolibc as a standard C library for native modules. This also contains a reference to the "errno" issue when building natmods on RV32 that the PR this commit is part of, as it is not obvious how to approach this issue when encountered for the first time. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
b5a8ff5 to
8af23ae
Compare
Summary
This PR fixes building the "btree" example natmod on RV32 when Picolibc is being used and uses thread-local storage for storing the errno variable.
The fix is surprisingly simple: Picolibc allows overriding the function that will provide a pointer to the "errno" variable, and the btree natmod integration code already has all of this machinery set up as part of its library integration. Redirecting Picolibc to the already existing pointer provider function via a compile-time definition is enough to let the module compile and pass QEMU tests.
This workaround will work on any Picolibc versions (Arm, RV32, Xtensa, etc.) even if TLS support was not enabled to begin with, and will effectively do nothing if the toolchain used will rely on Newlib to provide standard C library functions.
Given that the btree module now builds and passes the relevant natmod tests, said module is now part of the QEMU port's natmod testing procedure, and CI now will build the btree module for RV32 as part to its checks.
Testing
The
btreenatmod was built successfully when targeting RV32, and then QEMU'smake test_natmodbtree tests pass. Relevant CI tests also pass in my repo's branch (and hopefully will pass on this branch too!).