Copilot commented on code in PR #13610: URL: https://github.com/apache/trafficserver/pull/13610#discussion_r3917600831
########## 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: The configure-time failure message is phrased as if the only failure mode is a missing UBSan runtime package. `check_linker_flag()` can also fail because the toolchain simply doesn’t support `-fsanitize=undefined`, in which case the “On GCC … libubsan” guidance is misleading. Consider wording the error to cover both unsupported-flag and missing-runtime cases, with the GCC package note as a conditional hint. ########## 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: The UBSan rationale comment says it “composes with an asan build” but this block is deliberately outside the ASAN/TSAN if/elseif, so it also composes with TSAN. Updating the comment to mention both avoids misleading future readers. -- 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]
