Add more UBSan rules to the production build #404

New issue

Open

opened 2024-06-19 03:57:49 +00:00 by jade · 1 comment

jade commented

2024-06-19 03:57:49 +00:00

Owner

We can make more behaviours defined by going harder on the UBSan no-runtime stuff we already have for signed overflow.

https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#available-checks

Stuff like:

return
returns-nonnull-attribute
shift
integer-divide-by-zero
enum
nullability-* (cuz we don't use it yet!)
unreachable
vla-bound

I would guess that you can probably enable that entire list in one shot and it probably would not have any perf impact (benchmarks required though).

Potentially more expensive (but would be really good) checks:

bounds
bool

Unsure which of these are supported by gcc, but at this point I just want at least some part of the fleet of users to have better checking because it will find bugs eventually!

We can make more behaviours defined by going harder on the UBSan no-runtime stuff we already have for signed overflow. https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#available-checks Stuff like: - return - returns-nonnull-attribute - shift - integer-divide-by-zero - enum - `nullability-*` (cuz we don't use it yet!) - unreachable - vla-bound I would guess that you can probably enable that entire list in one shot and it probably would not have any perf impact (benchmarks required though). Potentially more expensive (but would be really good) checks: - bounds - bool Unsure which of these are supported by gcc, but at this point I just want at least some part of the fleet of users to have better checking because it will find bugs eventually!

jade added the

stability

Area/build-packaging

labels

2024-06-19 03:57:49 +00:00

raito added the

release-blocker

label

2025-11-18 02:58:23 +00:00

raito added the

E/easy

label

2025-11-26 21:15:33 +00:00

pennae added this to the 2.95 milestone

2025-12-01 14:51:44 +00:00

raito removed the

release-blocker

label

2026-01-06 09:46:30 +00:00

pennae commented

2026-02-09 16:36:22 +00:00

Owner

we've benchmarked the entire set of checks suggested above, and perf impact is not super high (~2%). not sure they're very useful though: return has a compiler warning attached (that could be upgraded to an error instead), returns-nonnull-attribute and the nullability suite do nothing, vla-bound doesn't apply to us, bounds is partially handled by our stl and partially not useful because io-based bounds violations aren't found, etc.

integer-divide-by-zero, shift, and unreachable may be useful and are essentially free, so we could add those?

we've benchmarked the *entire* set of checks suggested above, and perf impact is not super high (~2%). not sure they're very useful though: `return` has a compiler warning attached (that could be upgraded to an error instead), `returns-nonnull-attribute` and the `nullability` suite do nothing, `vla-bound` doesn't apply to us, `bounds` is partially handled by our stl and partially not useful because io-based bounds violations aren't found, etc. `integer-divide-by-zero`, `shift`, and `unreachable` may be useful and are essentially free, so we could add those?

❤️ 1