ci: Enable UBSan for 'longlong' builds in CI, add stack size for sanitizer builds. #17735

projectgus · 2025-07-22T01:54:05Z

Summary

Spun out from py/objint_longlong: Fix left shift of negative values. #17734 - enable UBSan in CI as suggested by @jepler
Scale up the unix port stack sizes substantially when sanitizers are enabled. The size of some MicroPython C stack frames with UBSan and GCC 12 (as used in CI) appears to be quite high. Whatever causes this seems to be fixed in later GCC.
To support uniform scaling of stack sizes, added some macros to apply the same stack size multiplier to both the main thread and the default thread stack size on unix port. Previously the unix port main thread stack was doubled on ARM builds but the default thread stack size was not, now they are both doubled (and the same macros are used when sanitizers are enabled).
Added a custom MicroPython macro to mark if sanitizers are enabled. As far as I can tell there's no way to programmatically detect if sanitizers are enabled on gcc<14, the other macros are all Clang-only.
Rewrote the scaling mechanism for the stress/recurse_iternext.py test, as this test fails on sanitizer builds with the increased stack. (Sanitizer build stack usage is only increased due to certain C function calls, but this test - I think - consumes stack in a way which isn't impacted by sanitizers so it can now handle many more iterations compared to a standard build). This took a while to find a good approach, but what's there seems more accurate than the previous heuristic and doesn't take noticeably more time to execute.

Testing

Tested in CI and by running CI functions locally with an ubuntu-22.04 container and with my local newer GCC.
Re-ran the updated stress/recurse_iternext.py test on unix port with and without sanitizers, esp32 port with SPIRAM, and esp8266 port.

Trade-offs and Alternatives

Could leave UBSan disabled in CI for the 'longlong' variant, but (as demonstrated) it is useful to find bugs.

This work was funded through GitHub Sponsors

codecov · 2025-07-22T02:37:54Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.38%. Comparing base (020eeba) to head (22deeeb).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #17735   +/-   ##
=======================================
  Coverage   98.38%   98.38%           
=======================================
  Files         171      171           
  Lines       22297    22297           
=======================================
  Hits        21938    21938           
  Misses        359      359

☔ 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.

github-actions · 2025-07-22T02:39:19Z

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

projectgus · 2025-08-13T01:34:47Z

@jepler Curious of your opinion on this PR, if you have any time to look?

dpgeorge · 2025-08-13T02:31:54Z

I tested the new recursive_iternext.py test on PYBD_SF6, where it previously failed when used with the native emitter, and it now passes.

Also tested on PYBD_SF2, PYBV10, RPI_PICO. It passes on all of those. Very good!

jepler

I'm in favor of it.

I looked into alternatives for detecting ubsan/asan and while gcc documentation shows they were grumpy at having to implement __has_feature, it does work as needed.

jepler · 2025-08-13T13:46:38Z

py/misc.h

@@ -43,6 +43,11 @@ typedef unsigned int uint;
 #ifndef __has_builtin
 #define __has_builtin(x) (0)
 #endif
+#ifndef __has_feature
+// This macro is supported by Clang and gcc>=14
+#define __has_feature(x) (0)


gcc isn't keen on __has_feature. Their docs say

Note that ‘__has_feature’ and ‘__has_extension’ are not recommended for use in new code, and are only provided for compatibility with Clang.

That said, I don't see a better way to autodetect the sanitizers, though. (gcc has a predefined macro for asan but clang doesn't; neither defines a macro for ubsan)

Given that you defensively check whether has_feature is available and that this works in practice in clang and gcc-14 (I checked gcc-14) and obey the MP_UBSAN / MP_ASAN in preference to detection I think it's fine. There's also explicit documentation from clang that this is how to check for asan. The corresponding bit was missing from the ubsan documentation but it does work too (tested in gcc-14 and clang-19).

ports/unix/stack_size.h

dpgeorge · 2025-08-14T14:21:29Z

I retested the revised recursive_iternext.py test on PYBD_SF2 and PYBD_SF6, in bytecode, mpy and native mode, and it passes.

Apart from the above comment about the license, this looks good to go in.

Also rewrite the sanitizer argument variables to not assume a variant. longlong variant currently fails in this config, due to a bug fixed in follow-up commit. Signed-off-by: Angus Gratton <angus@redyak.com.au>

Clang and gcc>=14 can use __has_feature() to detect if a sanitizer is enabled, but older GCC has no mechanism - need to set a macro explicitly for this to be recognised. Necessary for increasing some resource limits in sanitizer builds. Important not to use to avoid real issues! This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>

The specific problem seems to be that the runtime "Python stack frame" function call is several times more expensive in stack usage when running with UBSan on older GCC (observed on gcc 11.4 as used in CI, would get 'RuntimeError: maximum recursion depth exceeded' when running some tests with UBSan enabled.) Other stack usage (i.e. from pushing things on the stack in Python) stays the same. Whatever causes the usage seems to be mostly gone in later GCC versions. Includes a refactor to apply the same stack size multipliers for the default thread stack size same as the main stack size. This goes in a new port-specific header as it depends on macros in misc.h, so can't be in mpconfigport.h. A side effect of this is that the default thread stack size is now doubled on ARM, same as the main stack size. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>

Necessary on the unix port when running with sanitizers, as the newly increased stack size can run all tests at N=5000 without raising RuntimeError, and increasing N to fix this causes issues on other configurations. This way the test progressively builds a deeper data structure until it fails with RuntimeError. This is theoretically slower, but not noticeably so in reality. Signed-off-by: Angus Gratton <angus@redyak.com.au>

dpgeorge

Thanks for updating.

projectgus added tests Relates to tests/ directory in source port-unix labels Jul 22, 2025

projectgus mentioned this pull request Jul 22, 2025

py/objint_longlong: Fix left shift of negative values. #17734

Merged

projectgus force-pushed the ci/longlong_ubsan branch from 3b2a4a8 to 008b8c5 Compare July 22, 2025 02:27

This comment was marked as outdated.

Sign in to view

dpgeorge mentioned this pull request Jul 23, 2025

py/obj: Improve REPR_C accuracy and include it in CI tests #17731

Merged

projectgus added this to the release-1.27.0 milestone Jul 23, 2025

dpgeorge mentioned this pull request Jul 24, 2025

longlong: Add sanitizer builds, fix revealed problems. #17709

Closed

projectgus force-pushed the ci/longlong_ubsan branch 2 times, most recently from 1dc48bf to 818de93 Compare July 29, 2025 01:09

projectgus force-pushed the ci/longlong_ubsan branch 4 times, most recently from f7b8aee to 86ec523 Compare August 6, 2025 00:37

projectgus mentioned this pull request Aug 6, 2025

py/misc: Add explicit dependency on py/mpconfig.h. #17843

Merged

projectgus changed the title ~~ci: Enable UBSan for 'longlong' builds in CI, add stack margin for sanitizer builds.~~ ci: Enable UBSan for 'longlong' builds in CI, add stack size for sanitizer builds. Aug 6, 2025

projectgus force-pushed the ci/longlong_ubsan branch 8 times, most recently from 5aea471 to dc24ac6 Compare August 13, 2025 01:09

projectgus requested a review from dpgeorge August 13, 2025 01:34

projectgus force-pushed the ci/longlong_ubsan branch from dc24ac6 to 9438e5b Compare August 13, 2025 05:49

jepler reviewed Aug 13, 2025

View reviewed changes

jepler approved these changes Aug 13, 2025

View reviewed changes

dpgeorge reviewed Aug 14, 2025

View reviewed changes

ports/unix/stack_size.h Show resolved Hide resolved

projectgus added 4 commits August 19, 2025 10:33

tools/ci: Add UBSan to longlong CI build.

d81d56c

Also rewrite the sanitizer argument variables to not assume a variant. longlong variant currently fails in this config, due to a bug fixed in follow-up commit. Signed-off-by: Angus Gratton <angus@redyak.com.au>

projectgus force-pushed the ci/longlong_ubsan branch from 9438e5b to 22deeeb Compare August 19, 2025 00:33

dpgeorge approved these changes Aug 19, 2025

View reviewed changes

dpgeorge merged commit 22deeeb into micropython:master Aug 19, 2025
110 of 112 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

ci: Enable UBSan for 'longlong' builds in CI, add stack size for sanitizer builds. #17735

ci: Enable UBSan for 'longlong' builds in CI, add stack size for sanitizer builds. #17735

Uh oh!

projectgus commented Jul 22, 2025 •

edited

Loading

Uh oh!

This comment was marked as outdated.

codecov bot commented Jul 22, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Jul 22, 2025

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

projectgus commented Aug 13, 2025

Uh oh!

dpgeorge commented Aug 13, 2025

Uh oh!

jepler left a comment

Uh oh!

jepler Aug 13, 2025

Uh oh!

Uh oh!

dpgeorge commented Aug 14, 2025

Uh oh!

dpgeorge left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ci: Enable UBSan for 'longlong' builds in CI, add stack size for sanitizer builds. #17735

ci: Enable UBSan for 'longlong' builds in CI, add stack size for sanitizer builds. #17735

Uh oh!

Conversation

projectgus commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Trade-offs and Alternatives

Uh oh!

This comment was marked as outdated.

codecov bot commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Jul 22, 2025

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

projectgus commented Aug 13, 2025

Uh oh!

dpgeorge commented Aug 13, 2025

Uh oh!

jepler left a comment

Choose a reason for hiding this comment

Uh oh!

jepler Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dpgeorge commented Aug 14, 2025

Uh oh!

dpgeorge left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

projectgus commented Jul 22, 2025 •

edited

Loading

codecov bot commented Jul 22, 2025 •

edited

Loading