Skip to content

Commit 9851e93

Browse files
bghgaryCopilot
andauthored
Make UBSan findings fatal in sanitized builds (#1676)
## 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 #5 SHA** — picks up [BabylonJS/glslang#5](BabylonJS/glslang#5) (suppress UBSan enum check on SPIR-V mask bit-combining operators). 3. **Bump bgfx.cmake to merged #115 (UBSan fix)** — picks up [BabylonJS/bgfx.cmake#115](BabylonJS/bgfx.cmake#115), which bumps the bgfx submodule to the cherry-pick of [bkaradzic/bgfx#3688](bkaradzic/bgfx#3688) (default-init the SortKey ints/bools so we don't load undefined values into bitfields) via [BabylonJS/bgfx#60](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] --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 81a7bfe commit 9851e93

3 files changed

Lines changed: 10 additions & 3 deletions

File tree

.github/workflows/build-linux.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ on:
1919

2020
env:
2121
CMAKE_VERSION: '3.31.6'
22+
UBSAN_OPTIONS: halt_on_error=1:print_stacktrace=1:symbolize=1
23+
ASAN_OPTIONS: abort_on_error=1
2224

2325
jobs:
2426
build:

.github/workflows/build-macos.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ on:
2222

2323
env:
2424
CMAKE_VERSION: '3.31.6'
25+
UBSAN_OPTIONS: halt_on_error=1:print_stacktrace=1:symbolize=1
26+
ASAN_OPTIONS: abort_on_error=1
2527

2628
jobs:
2729
build:

CMakeLists.txt

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,15 @@ FetchContent_Declare(base-n
3535
EXCLUDE_FROM_ALL)
3636
FetchContent_Declare(bgfx.cmake
3737
GIT_REPOSITORY https://github.com/BabylonJS/bgfx.cmake.git
38-
GIT_TAG c6575c5644e37ccc06b83d59d1d536cf8ec9f265
38+
GIT_TAG e5f3f31c1ec8a376dde75f8b20366e2696810ea0
3939
EXCLUDE_FROM_ALL)
4040
FetchContent_Declare(CMakeExtensions
4141
GIT_REPOSITORY https://github.com/BabylonJS/CMakeExtensions.git
4242
GIT_TAG 631780e42886e5f12bfd1a5568c7395f1d657f43
4343
EXCLUDE_FROM_ALL)
4444
FetchContent_Declare(glslang
4545
GIT_REPOSITORY https://github.com/BabylonJS/glslang.git
46-
GIT_TAG 39a80699a315cb7f66c4ab3180edd4e2910fab28
46+
GIT_TAG 284e4301e5a6b44b279635276588db7cdd942624
4747
EXCLUDE_FROM_ALL)
4848
FetchContent_Declare(googletest
4949
URL "https://github.com/google/googletest/archive/refs/tags/v1.17.0.tar.gz"
@@ -159,7 +159,10 @@ if(ENABLE_SANITIZERS)
159159

160160
string(JOIN "," SANITIZER_FLAGS ${SANITIZERS})
161161

162-
add_compile_options(-fsanitize=${SANITIZER_FLAGS} -fno-omit-frame-pointer)
162+
# -fno-sanitize-recover=all makes UBSan findings fatal. Without it, UBSan
163+
# only prints a diagnostic to stderr and execution continues, so CI stays
164+
# green while latent UB goes unreported.
165+
add_compile_options(-fsanitize=${SANITIZER_FLAGS} -fno-omit-frame-pointer -fno-sanitize-recover=all)
163166
add_link_options(-fsanitize=${SANITIZER_FLAGS})
164167
elseif(MSVC)
165168
# MSVC only supports AddressSanitizer

0 commit comments

Comments
 (0)