Skip to content

Fix #141: macOS ARM doesn't support leak sanitizer#151

Merged
lefticus merged 1 commit into
cpp-best-practices:mainfrom
leftibot:fix/issue-141-macos-arm-doesn-t-support-leak-sanitizer
May 2, 2026
Merged

Fix #141: macOS ARM doesn't support leak sanitizer#151
lefticus merged 1 commit into
cpp-best-practices:mainfrom
leftibot:fix/issue-141-macos-arm-doesn-t-support-leak-sanitizer

Conversation

@leftibot
Copy link
Copy Markdown
Contributor

@leftibot leftibot commented May 2, 2026

Automated fix by @leftibot.

What changed

Fix #141: skip leak sanitizer on Apple platforms
LeakSanitizer is unsupported on macOS (clang rejects -fsanitize=leak on
arm64-apple-darwin and other Apple targets); leak detection there is
provided by AddressSanitizer with ASAN_OPTIONS=detect_leaks=1. When
myproject_ENABLE_SANITIZER_LEAK is requested on an Apple host,
myproject_enable_sanitizers now warns and drops it from the sanitizer
list instead of producing an unsupported -fsanitize=leak compile flag.
Adds two CMake-script regression tests that stub target_*_options to
verify leak is filtered out on Apple and still propagated elsewhere.

Files

 cmake/Sanitizers.cmake                             |  9 +++-
 test/CMakeLists.txt                                | 10 ++++
 .../test_sanitizers_apple_excludes_leak.cmake      | 62 ++++++++++++++++++++++
 .../test_sanitizers_non_apple_includes_leak.cmake  | 52 ++++++++++++++++++
 4 files changed, 132 insertions(+), 1 deletion(-)

Closes #141

Triggered by @lefticus.

LeakSanitizer is unsupported on macOS (clang rejects -fsanitize=leak on
arm64-apple-darwin and other Apple targets); leak detection there is
provided by AddressSanitizer with ASAN_OPTIONS=detect_leaks=1. When
myproject_ENABLE_SANITIZER_LEAK is requested on an Apple host,
myproject_enable_sanitizers now warns and drops it from the sanitizer
list instead of producing an unsupported -fsanitize=leak compile flag.
Adds two CMake-script regression tests that stub target_*_options to
verify leak is filtered out on Apple and still propagated elsewhere.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 20.50%. Comparing base (ec01c73) to head (6d6061e).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #151   +/-   ##
=======================================
  Coverage   20.50%   20.50%           
=======================================
  Files           5        5           
  Lines         239      239           
  Branches      111      111           
=======================================
  Hits           49       49           
+ Misses        186      179    -7     
- Partials        4       11    +7     
Flag Coverage Δ
Linux 18.36% <ø> (ø)
Windows 21.02% <ø> (ø)
macOS 18.87% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@lefticus
Copy link
Copy Markdown
Member

lefticus commented May 2, 2026

@leftibot investigate build failures

@leftibot
Copy link
Copy Markdown
Contributor Author

leftibot commented May 2, 2026

Hi @lefticus — I tried to address the review feedback but couldn't produce a working change. May need human attention.

@lefticus
Copy link
Copy Markdown
Member

lefticus commented May 2, 2026

@leftibot can you determine why the wasm builds are failing by looking at the GitHub build logs?

@leftibot
Copy link
Copy Markdown
Contributor Author

leftibot commented May 2, 2026

Looked at the failing build-and-deploy job (run 25257675941). This is not caused by this PR — the same failure is on main (run 25258204211) and started after the last green wasm build on 2026-04-15.

Root cause

The link step for intro.html fails inside wasm-opt:

em++: warning: ASYNCIFY=1 is not compatible with -fwasm-exceptions.
              Parts of the program that mix ASYNCIFY and exceptions will not compile. [-Wemcc]
...
Fatal: Module::getFunction: __asyncify_get_call_index does not exist
em++: error: '... wasm-opt ... --asyncify ...' failed (returned 1)

We are passing both flags from cmake/Emscripten.cmake:

  • -fwasm-exceptions at lines 50–51 (add_compile_options / add_link_options)
  • -sASYNCIFY=1 at line 93 (target_link_options in myproject_configure_wasm_target)

The combination has always been documented as fragile (see the warning), but the pipeline now hard-errors instead of producing a half-broken binary. The trigger is almost certainly an emscripten upgrade: .github/workflows/wasm.yml:23 pins setup-emsdk to version: 'latest', so any new emsdk release rolls in unreviewed. Last green run was Apr 15; first red was May 2.

Suggested fixes (any one unblocks CI)

  1. Pin emsdk in .github/workflows/wasm.yml:23 to the last known-good release (e.g. whatever was current on 2026-04-15, around 3.1.65-ish — verify against emsdk-archive). This is the smallest change and matches how mymindstorm/setup-emsdk is intended to be used in CI.
  2. Drop the asyncify+wasm-exceptions combo. Pick one:
    • Replace -fwasm-exceptions with the legacy JS-based path (-sDISABLE_EXCEPTION_CATCHING=0) — keeps -sASYNCIFY=1 working but loses native wasm EH performance/size.
    • Or switch -sASYNCIFY=1 to -sJSPI=1 (requires modern browsers but is the upstream-recommended replacement for asyncify when EH is desired).
  3. Investigate whether asyncify is still needed. It's there to support emscripten_sleep / async ops in FTXUI's WASM glue (per the comment at cmake/Emscripten.cmake:92). If the FTXUI sample no longer hits any async path under PROXY_TO_PTHREAD=1 (which already runs main off the browser thread), you may be able to simply drop -sASYNCIFY=1 and -sASYNCIFY_STACK_SIZE entirely.

Recommendation: open a separate issue for this — it's orthogonal to the leak-sanitizer-on-Apple work in this PR, and merging this PR shouldn't be blocked by it. Short term, pinning emsdk gets CI green; medium term, option (2)/(3) is the durable fix.

I have not attempted any of these changes from this PR's branch, since they don't belong here.

@lefticus lefticus merged commit a3971f5 into cpp-best-practices:main May 2, 2026
33 of 34 checks passed
@lefticus lefticus mentioned this pull request May 2, 2026
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.

macOS ARM doesn't support leak sanitizer

2 participants