You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "bryancall (via GitHub)" <gi...@apache.org> on 2026/09/01 14:38:09 UTC
[PR] Add ENABLE_UBSAN build option [trafficserver]
bryancall opened a new pull request, #13610:
URL: https://github.com/apache/trafficserver/pull/13610
Adds UndefinedBehaviorSanitizer as a build option next to the existing `ENABLE_ASAN` and `ENABLE_TSAN`.
### Why this is not part of the asan/tsan either-or chain
ASAN replaces the allocator and TSAN replaces the thread runtime, so those two are mutually exclusive and the existing code enforces that with an `if`/`elseif`. UBSan only instruments arithmetic, shifts, and type loads, so it composes with either of them. The new block sits outside that chain, which is what lets `dev-asan-ubsan` produce `-fsanitize=address,undefined` in a single build.
### Presets
A hidden `ubsan` preset matching the shape of the existing hidden `asan` and `tsan` presets, plus two visible ones:
```
cmake --preset dev-ubsan
cmake --preset dev-asan-ubsan
```
### Two deliberate choices
`vptr` is excluded. That check 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 it reports the plugin boundary rather than a real defect.
Findings are non-fatal, so a single run reports every distinct site instead of stopping at the first. Setting `UBSAN_OPTIONS=halt_on_error=1` at runtime aborts instead, which is what a gating job would want.
### No CI job turns this on
Enabling it anywhere would be a separate change. The option is inert until someone asks for one of the presets.
### What it reports on the current tree
For context on whether the option earns its place, on this tree with clang 22 on Fedora:
* unit tests: 211 runtime errors across 57 distinct sites, with all 179 tests still passing
* a full 572 test autest run: 617 distinct sites
Grouping those by root cause collapses them a great deal, and a handful look worth fixing. The largest single group is a hasher buffer that is declared as a plain `char` array and then has a polymorphic object constructed into it, which makes every access through it misaligned on the URL hash and cache key path. Another is the plugin entry point, declared at the call site as `void (*)(int, char **)` while every plugin in the tree defines `TSPluginInit(int, const char *argv[])`.
Those fixes are separate pull requests. This one only adds the option so the findings are reproducible.
### Testing
Configured and built with clang 22 on Fedora 44 using `dev-ubsan` with autest and experimental plugins enabled. Build and install both clean, `ctest` 179/179 passing, and the resulting binary links the UBSan runtime handlers.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add ENABLE_UBSAN build option [trafficserver]
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add ENABLE_UBSAN build option [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13610:
URL: https://github.com/apache/trafficserver/pull/13610#discussion_r3915897056
##########
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:
The comment asserts non-fatal behavior, but the build flags don’t explicitly select recover/non-recover mode (defaults vary by compiler/checks). To make the behavior consistent and aligned with the comment, explicitly pass a recover-mode flag (e.g., `-fsanitize-recover=undefined`), and optionally document how to make it fatal via flags vs `UBSAN_OPTIONS`.
##########
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:
`-fno-sanitize=vptr` is a compile-time instrumentation control; it doesn’t affect the link step. Keeping it in `add_link_options(...)` is unnecessary and can cause avoidable toolchain-compatibility issues. Recommend removing `-fno-sanitize=vptr` from `add_link_options(...)` and leaving it only in `add_compile_options(...)`.
##########
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:
The comment asserts non-fatal behavior, but the build flags don’t explicitly select recover/non-recover mode (defaults vary by compiler/checks). To make the behavior consistent and aligned with the comment, explicitly pass a recover-mode flag (e.g., `-fsanitize-recover=undefined`), and optionally document how to make it fatal via flags vs `UBSAN_OPTIONS`.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add ENABLE_UBSAN build option [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13610:
URL: https://github.com/apache/trafficserver/pull/13610#discussion_r3905387989
##########
CMakeLists.txt:
##########
@@ -449,6 +450,23 @@ 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)
+ add_compile_options(-g -fsanitize=undefined -fno-sanitize=vptr -fno-omit-frame-pointer)
+ add_link_options(-g -fsanitize=undefined -fno-sanitize=vptr)
Review Comment:
`add_compile_options(...)` applies to all languages, including C. The flag `-fno-sanitize=vptr` is C++-specific and can cause C compilation to fail when `ENABLE_UBSAN` is enabled (depending on compiler/toolchain). Restrict `-fno-sanitize=vptr` to C++ compilation via a generator expression (e.g., only for `COMPILE_LANGUAGE:CXX`), and consider dropping/guarding the link-time `-fno-sanitize=vptr` entirely since it’s a compile-time instrumentation control.
##########
CMakeLists.txt:
##########
@@ -449,6 +450,23 @@ 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)
+ add_compile_options(-g -fsanitize=undefined -fno-sanitize=vptr -fno-omit-frame-pointer)
+ add_link_options(-g -fsanitize=undefined -fno-sanitize=vptr)
+endif()
Review Comment:
The UBSan block unconditionally adds toolchain-specific flags without checking whether the current compiler/linker supports them. This can break configuration/builds on unsupported compilers/platforms when `ENABLE_UBSAN` is turned on. Use CMake feature checks (e.g., `check_cxx_compiler_flag` / `check_linker_flag`) and conditionally append only the supported flags (and emit a clear fatal error if `ENABLE_UBSAN` is requested but `-fsanitize=undefined` isn’t available).
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add ENABLE_UBSAN build option [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13610:
URL: https://github.com/apache/trafficserver/pull/13610#discussion_r3918045969
##########
CMakeLists.txt:
##########
@@ -449,6 +450,43 @@ 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 either of them as well as
+# standing on its own: asan gives -fsanitize=address,undefined and tsan gives
+# -fsanitize=thread,undefined. Only the asan pairing has a preset, since that is
+# the combination developers ask for, but the option itself does not restrict it.
+#
+# 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)
+ # This can fail two ways: the toolchain does not know -fsanitize=undefined at all,
+ # or it knows the flag but cannot link the runtime library, which some distributions
+ # package separately from the compiler. Without the check the second case surfaces as
+ # a bare "cannot find libubsan.so" from the linker after compiling hundreds of
+ # objects, so establish it here and let the message cover both.
+ include(CheckLinkerFlag)
+ check_linker_flag(CXX -fsanitize=undefined HAS_UBSAN_SUPPORT)
+ if(NOT HAS_UBSAN_SUPPORT)
+ message(
+ FATAL_ERROR
+ "ENABLE_UBSAN was requested, but this toolchain cannot link -fsanitize=undefined. Either the compiler does not support the flag, or it does and the sanitizer runtime is missing. If the compiler is GCC, that runtime is the libubsan package."
Review Comment:
The fatal error message hard-codes a specific distro package name ("the libubsan package"). On some platforms (e.g., Debian/Ubuntu), the GCC UBSan runtime is packaged under different names (commonly libubsan1), so this guidance can be misleading even though the underlying diagnosis is correct.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add ENABLE_UBSAN build option [trafficserver]
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on code in PR #13610:
URL: https://github.com/apache/trafficserver/pull/13610#discussion_r3915758945
##########
CMakeLists.txt:
##########
@@ -449,6 +450,23 @@ 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)
+ add_compile_options(-g -fsanitize=undefined -fno-sanitize=vptr -fno-omit-frame-pointer)
+ add_link_options(-g -fsanitize=undefined -fno-sanitize=vptr)
Review Comment:
`-fno-sanitize=vptr` is a negative flag, so disabling a check that does not apply to C is accepted rather than rejected. Verified on both toolchains: the tree has 3 C object rules and all 3 build with the flag under clang 22, and GCC 16 compiled 55 objects (C and C++) before failing at link for an unrelated reason. `clang -fsanitize=undefined -fno-sanitize=vptr -c t.c` and the gcc equivalent both exit 0.
Leaving it applied to both languages keeps this block the same shape as the asan and tsan blocks above it.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add ENABLE_UBSAN build option [trafficserver]
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on code in PR #13610:
URL: https://github.com/apache/trafficserver/pull/13610#discussion_r3915760590
##########
CMakeLists.txt:
##########
@@ -449,6 +450,23 @@ 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)
+ add_compile_options(-g -fsanitize=undefined -fno-sanitize=vptr -fno-omit-frame-pointer)
+ add_link_options(-g -fsanitize=undefined -fno-sanitize=vptr)
+endif()
Review Comment:
Good catch, fixed in 400c90435d.
`-fsanitize=undefined` links against a runtime that some distributions package separately from the compiler. Fedora ships libasan with gcc but puts libubsan in its own package, so a GCC build here compiled 55 objects and then failed with
```
ld.bfd: cannot find /usr/lib64/libubsan.so.1.0.0: No such file or directory
```
with nothing connecting that to the option that caused it. There is now a `check_linker_flag` at configure time that fails immediately and names the missing package. Confirmed both directions: GCC without libubsan stops at configure in a few seconds, clang reports `HAS_UBSAN_RUNTIME - Success` and builds.
Worth noting `ENABLE_TSAN` has the same latent problem on this platform, since libtsan is also a separate package, but that is pre-existing and I would rather fix it separately than widen this change.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add ENABLE_UBSAN build option [trafficserver]
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add ENABLE_UBSAN build option [trafficserver]
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on code in PR #13610:
URL: https://github.com/apache/trafficserver/pull/13610#discussion_r3918206896
##########
CMakeLists.txt:
##########
@@ -449,6 +450,43 @@ 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 either of them as well as
+# standing on its own: asan gives -fsanitize=address,undefined and tsan gives
+# -fsanitize=thread,undefined. Only the asan pairing has a preset, since that is
+# the combination developers ask for, but the option itself does not restrict it.
+#
+# 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)
+ # This can fail two ways: the toolchain does not know -fsanitize=undefined at all,
+ # or it knows the flag but cannot link the runtime library, which some distributions
+ # package separately from the compiler. Without the check the second case surfaces as
+ # a bare "cannot find libubsan.so" from the linker after compiling hundreds of
+ # objects, so establish it here and let the message cover both.
+ include(CheckLinkerFlag)
+ check_linker_flag(CXX -fsanitize=undefined HAS_UBSAN_SUPPORT)
+ if(NOT HAS_UBSAN_SUPPORT)
+ message(
+ FATAL_ERROR
+ "ENABLE_UBSAN was requested, but this toolchain cannot link -fsanitize=undefined. Either the compiler does not support the flag, or it does and the sanitizer runtime is missing. If the compiler is GCC, that runtime is the libubsan package."
Review Comment:
Correct, that was the Fedora spelling. Fixed in 9cc894940a by dropping the package name rather than trying to enumerate distributions, since any specific name will be wrong somewhere. The message now says the GCC runtime ships separately under a name that varies by distribution, which stays actionable without making a claim that is false on some platform.
The only remaining `libubsan` mention is inside a quoted linker error in the comment above, which is the actual output a developer sees rather than a package name.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add ENABLE_UBSAN build option [trafficserver]
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on PR #13610:
URL: https://github.com/apache/trafficserver/pull/13610#issuecomment-5517774451
[approve ci clang-analyzer]
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add ENABLE_UBSAN build option [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add ENABLE_UBSAN build option [trafficserver]
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall merged PR #13610:
URL: https://github.com/apache/trafficserver/pull/13610
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org