Skip to content

examples/natmod/btree: Fix build on RV32 with Picolibc. #17642

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

agatti
Copy link
Contributor

@agatti agatti commented Jul 8, 2025

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 btree natmod was built successfully when targeting RV32, and then QEMU's make test_natmod btree tests pass. Relevant CI tests also pass in my repo's branch (and hopefully will pass on this branch too!).

Copy link

github-actions bot commented Jul 8, 2025

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

Copy link

codecov bot commented Jul 8, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.44%. Comparing base (05342b0) to head (8af23ae).
Report is 7 commits behind head on master.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dpgeorge dpgeorge added the examples Relates to examples/ directory in source label Jul 9, 2025
@dpgeorge
Copy link
Member

dpgeorge commented Jul 9, 2025

Very nice!

@@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this can go in py/dynruntime.mk so it's available for everyone? It seems pretty harmless to add unconditionally.

Copy link
Contributor Author

@agatti agatti Jul 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@agatti agatti force-pushed the rv32-natmod-fix-btree branch from 7a04586 to b5a8ff5 Compare July 9, 2025 15:24
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>
@agatti agatti force-pushed the rv32-natmod-fix-btree branch from b5a8ff5 to 8af23ae Compare July 9, 2025 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
examples Relates to examples/ directory in source
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants