bryancall commented on code in PR #13610: URL: https://github.com/apache/trafficserver/pull/13610#discussion_r3917530484
########## CMakeLists.txt: ########## @@ -449,6 +450,35 @@ elseif(ENABLE_TSAN) add_link_options(-g -fsanitize=thread) endif() +# UndefinedBehaviorSanitizer. Deliberately not part of the either/or chain above: +# UBSan instruments arithmetic, shifts, and type loads rather than replacing the +# allocator or the thread runtime, so it composes with an asan build +# (-fsanitize=address,undefined) as well as standing on its own. +# +# vptr is excluded. It needs a matching type_info at every polymorphic access, and +# a plugin loaded with dlopen does not reliably share type identity with the main +# image, so the check reports the plugin boundary rather than a real bug. +# +# Findings are non-fatal by default so a single run reports every distinct site +# instead of stopping at the first. Set UBSAN_OPTIONS=halt_on_error=1 at runtime +# to abort instead, which is what a gating job would want. +if(ENABLE_UBSAN) + # The checks are instrumentation, but they call into a runtime library that some + # distributions package separately from the compiler (libubsan for GCC). Without it + # the build gets a bare "cannot find libubsan.so" from the linker after compiling + # hundreds of objects, so establish it here and name what is missing. + include(CheckLinkerFlag) + check_linker_flag(CXX -fsanitize=undefined HAS_UBSAN_RUNTIME) + if(NOT HAS_UBSAN_RUNTIME) + message( + FATAL_ERROR + "ENABLE_UBSAN needs the undefined behavior sanitizer runtime, which this toolchain cannot link. On GCC that runtime is the libubsan package." + ) + endif() + add_compile_options(-g -fsanitize=undefined -fno-sanitize=vptr -fno-omit-frame-pointer) + add_link_options(-g -fsanitize=undefined -fno-sanitize=vptr) Review Comment: Right, removed in 3160385412. Confirmed against the generated build: `-fno-sanitize=vptr` now appears in 0 of 183 link blocks, and `-fsanitize=undefined` still appears in 147 so the runtime is still linked. ########## CMakeLists.txt: ########## @@ -449,6 +450,35 @@ elseif(ENABLE_TSAN) add_link_options(-g -fsanitize=thread) endif() +# UndefinedBehaviorSanitizer. Deliberately not part of the either/or chain above: +# UBSan instruments arithmetic, shifts, and type loads rather than replacing the +# allocator or the thread runtime, so it composes with an asan build +# (-fsanitize=address,undefined) as well as standing on its own. +# +# vptr is excluded. It needs a matching type_info at every polymorphic access, and +# a plugin loaded with dlopen does not reliably share type identity with the main +# image, so the check reports the plugin boundary rather than a real bug. +# +# Findings are non-fatal by default so a single run reports every distinct site +# instead of stopping at the first. Set UBSAN_OPTIONS=halt_on_error=1 at runtime +# to abort instead, which is what a gating job would want. Review Comment: Agreed, and the comment was the inaccurate part. Fixed in 3160385412 by asking for it explicitly with `-fsanitize-recover=undefined`, which both clang 22 and GCC 16 accept. Worth recording why it matters beyond consistency: recovery is not uniformly available. `clang++ -fsanitize-recover=unreachable` fails with `unsupported argument 'unreachable'`, because `unreachable` and `return` cannot recover by construction and still abort. The comment now says that rather than claiming findings are simply non-fatal. Verified after the change: 133/133 unit tests pass with 210 findings logged, so a run still reports every distinct site instead of stopping at the first. This comment came through twice, at lines 464 and 479 — replying here covers both. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
