Skip to content

Make UBSan findings fatal in sanitized builds#1676

Merged
bghgary merged 3 commits intoBabylonJS:masterfrom
bghgary:sanitizer-fail-fast
Apr 25, 2026
Merged

Make UBSan findings fatal in sanitized builds#1676
bghgary merged 3 commits intoBabylonJS:masterfrom
bghgary:sanitizer-fail-fast

Conversation

@bghgary
Copy link
Copy Markdown
Contributor

@bghgary bghgary commented Apr 24, 2026

Context

Our sanitized macOS/Linux jobs enable -fsanitize=address,undefined[,vptr], so the UB checks are active — but UBSan is recoverable by default. On a hit it prints a diagnostic to stderr and keeps running. The process doesn't abort, the test doesn't fail, CI stays green, and the diagnostic is buried in test output nobody reads.

Adding -fno-sanitize-recover=all is what converts each UBSan hit into a SIGABRT → test failure. We were missing it.

Change

Three commits:

  1. Make UBSan findings fatal in sanitized builds. Add -fno-sanitize-recover=all to the Clang sanitizer add_compile_options line in CMakeLists.txt, and set UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1:symbolize=1 and ASAN_OPTIONS=abort_on_error=1 in build-macos.yml / build-linux.yml as runtime belt-and-suspenders. (MSVC only supports ASan, which already aborts on error.)
  2. Bump glslang to merged PR Investigate using BGFX's built-in multithreaded capabilities #5 SHA — picks up BabylonJS/glslang#5 (suppress UBSan enum check on SPIR-V mask bit-combining operators).
  3. Bump bgfx.cmake to merged XR basic input #115 (UBSan fix) — picks up BabylonJS/bgfx.cmake#115, which bumps the bgfx submodule to the cherry-pick of bkaradzic/bgfx#3688 (default-init the SortKey ints/bools so we don't load undefined values into bitfields) via BabylonJS/bgfx#60.

The dependency bumps in commits 2 and 3 fix every UBSan finding currently being printed (and ignored) by the sanitized jobs on master, so this PR should be green on first run.

Out of scope

  • metal-cpp's intentional "nil-messaging" pattern. Apple documents sending messages to nil as well-defined in Obj-C, but C++ UBSan doesn't know that. Not hit on the current pin; if a future metal-cpp bump trips it, the standard fix is a scoped -fno-sanitize=null on metal-cpp only.
  • -fsanitize=nullability (Apple _Nonnull attribute checks, separate from null) — worth considering as a follow-up once this lands and the baseline is clean.
  • Windows ASan already aborts on error; no change needed there.

[Created by Copilot on behalf of @bghgary]

bghgary and others added 3 commits April 24, 2026 15:34
UBSan is recoverable by default: diagnostics are printed to stderr but
execution continues, so CI stays green while latent UB goes unreported.

Add -fno-sanitize-recover=all to the Clang sanitizer options, and set
UBSAN_OPTIONS=halt_on_error=1 as a runtime belt-and-suspenders on the
macOS and Linux sanitized workflows. (MSVC only supports ASan, which
already aborts on error.)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Now that BabylonJS/glslang#5 (Suppress UBSan enum check on SPIR-V mask
bit-combining operators) has merged, point FetchContent at the
merged-on-main SHA instead of the bghgary fork branch used for testing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Picks up the cherry-pick of bkaradzic/bgfx#3688 (default-init the
SortKey ints/bools so we don't load undefined values into bitfields)
via BabylonJS/bgfx.cmake#115, which bumps the bgfx submodule to
BabylonJS/bgfx@7e4749e6 on update-bgfx.

This is the canonical merged SHA on master; it replaces the temporary
bghgary/bgfx.cmake validation pin used while bkaradzic/bgfx#3688 was
in review.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bghgary bghgary force-pushed the sanitizer-fail-fast branch from ae66199 to b814753 Compare April 24, 2026 22:34
@bghgary bghgary marked this pull request as ready for review April 24, 2026 22:39
Copilot AI review requested due to automatic review settings April 24, 2026 22:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the sanitized build configuration so UndefinedBehaviorSanitizer (UBSan) findings fail CI instead of only emitting stderr diagnostics, and refreshes pinned third-party dependencies to versions that eliminate current UBSan hits.

Changes:

  • Make UBSan non-recoverable in sanitized builds by adding -fno-sanitize-recover=all to Clang/GNU sanitizer compile flags in CMake.
  • Set UBSAN_OPTIONS and ASAN_OPTIONS in macOS/Linux GitHub Actions workflows to halt/abort on sanitizer errors and improve diagnostics.
  • Bump the pinned SHAs for bgfx.cmake and glslang to versions that address known UBSan findings.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
CMakeLists.txt Updates dependency pins and ensures UBSan findings are fatal when ENABLE_SANITIZERS=ON.
.github/workflows/build-macos.yml Adds runtime sanitizer env options to make UBSan/ASan errors fail the job with better stack traces.
.github/workflows/build-linux.yml Adds runtime sanitizer env options to make UBSan/ASan errors fail the job with better stack traces.

@bghgary bghgary merged commit 9851e93 into BabylonJS:master Apr 25, 2026
32 checks passed
@bghgary bghgary deleted the sanitizer-fail-fast branch April 25, 2026 05:24
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.

3 participants