bryancall commented on code in PR #13610: URL: https://github.com/apache/trafficserver/pull/13610#discussion_r3918024652
########## CMakeLists.txt: ########## @@ -449,6 +450,40 @@ 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. Review Comment: Fair point, fixed in 3fa62821c2. The comment now names both, since the block sits outside the asan/tsan chain precisely so it works with either: asan gives `-fsanitize=address,undefined` and tsan gives `-fsanitize=thread,undefined`. It also notes that only the asan pairing has a preset, since that is the combination that gets asked for, while the option itself does not restrict it. ########## CMakeLists.txt: ########## @@ -449,6 +450,40 @@ 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 recover, so a single run reports every distinct site instead of stopping +# at the first. That is the default for most checks, but which checks recover by +# default varies between compilers, so ask for it explicitly rather than inherit it. +# A few checks cannot recover by construction (unreachable, return) and still abort. +# Set UBSAN_OPTIONS=halt_on_error=1 at runtime to stop on the first finding, 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() Review Comment: Agreed, and this was the more misleading of the two. `check_linker_flag()` failing does not distinguish an unsupported flag from a supported flag with no runtime, so pointing at the libubsan package as the cause could send someone the wrong way entirely. Fixed in 3fa62821c2. The message now covers both cases and keeps the package note as a GCC-specific hint rather than the diagnosis. I also renamed the result variable from `HAS_UBSAN_RUNTIME` to `HAS_UBSAN_SUPPORT`, since the old name asserted the same narrow reading. Verified both directions: GCC without libubsan reports `HAS_UBSAN_SUPPORT - Failed` and the new message, clang reports `HAS_UBSAN_SUPPORT - Success` and configures. -- 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]
