Skip to content

ci: run the unit tests under a 256 KiB stack - #2495

Merged
ronaldtse merged 6 commits into
mainfrom
ci-limited-stack-2347
Sep 26, 2026
Merged

ronaldtse merged 6 commits into
mainfrom
ci-limited-stack-2347

Conversation

@ronaldtse

Copy link
Copy Markdown
Contributor

Summary

  • Adds a dedicated Ubuntu leg which builds the Botan shared configuration and runs the unit tests after ulimit -s 256, as suggested by @ni4 in Add test runner with limited stack size. #2347.
  • Stack overruns which would only surface on constrained deployments now fail loudly in CI, while the existing unrestricted legs are left unchanged.
  • The leg excludes the slow tests, matching the other Ubuntu jobs, and runs ctest serially so a stack fault is easy to attribute.

Test plan

  • the limited-stack leg passes on this PR
  • the existing unrestricted Ubuntu legs continue to pass

Some code paths can overrun the default stack size, as reported in issue
#2347. A dedicated Ubuntu leg now builds the Botan shared configuration
and runs ctest after ulimit -s 256, so stack overruns fail loudly in CI
rather than only on constrained deployments. The existing unrestricted
legs are left unchanged.
@ronaldtse

Copy link
Copy Markdown
Contributor Author

@ni4 this adds the limited-stack CI leg from #2347: a dedicated Ubuntu job runs the unit tests after ulimit -s 256 so stack overruns surface in CI. Whenever convenient, a review would be appreciated.

@ronaldtse
ronaldtse requested a review from ni4 September 21, 2026 09:24
@codecov

codecov Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.45%. Comparing base (26482f6) to head (15299fb).
⚠️ Report is 15 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2495   +/-   ##
=======================================
  Coverage   85.45%   85.45%           
=======================================
  Files         125      125           
  Lines       23042    23042           
=======================================
  Hits        19691    19691           
  Misses       3351     3351           

☔ View full report in Codecov by Harness.
📢 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.

@ni4 ni4 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, but we seem to have segfault in CI.

The segfault of test_ffi_key_export_autocrypt under the 256 KiB stack
is a stack overflow inside the regex implementation, not in library
code: validating the multi-kilobyte base64 export with a regex makes
the matcher recurse once per repetition of the character class, which
libstdc++ does without any depth bound, and the recursion exhausts the
limited stack. Replace the validation with a direct check that the
value consists of base64 characters and carries at most two padding
characters, which does not recurse. Also restore the workflow to its
pre-debug form, now that the temporary gdb capture has served its
purpose.
@ronaldtse

Copy link
Copy Markdown
Contributor Author

The segfault is root-caused, and it turns out to be good news for the library. The backtrace captured with gdb shows the crash is unbounded recursion inside the C++ standard library regex engine: the test validated the multi-kilobyte base64 export with std::regex_search, and the matcher of libstdc++ recurses once per repetition of the character class, which overflows the 256 KiB stack. The library code itself, including the export path, stays well within the budget, and the crash is absent on platforms whose regex implementations do not recurse this way, which is why it only appeared on Linux.

The fix replaces the regex validation with a direct check that the exported value consists of base64 characters and carries at most two padding characters, which does not recurse. With the fix, the test and the other export tests pass locally under the same 256 KiB limit. The temporary gdb instrumentation has been removed and the workflow is back to its intended form. Thank you @ni4 for insisting on chasing this one down; the leg now does exactly what issue #2347 asked of it, failing loudly when the stack budget is exceeded and passing when the code respects it.

The character-set scan passed the body length into the overload that
limits the length of the alphabet, not the search range, which reads
past the end of the alphabet literal on implementations that do not
clamp it to the string length, as the AddressSanitizer run of the
Windows sanitizer leg caught. Search a substring of the value instead,
which needs no length bookkeeping.
@ronaldtse
ronaldtse merged commit 96f35e3 into main Sep 26, 2026
144 of 145 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants