devsw-prayas wrote: Reworked this per your suggestion, folded into MSan instead of a standalone UBSan check, same shape as `-fsanitize-memory-param-retval`. Covers all four points you raised: cost model's opt-in now instead of default-on in `Undefined`, C11/DR338 scoping is the same but not tied to C23 in the framing anymore, naming sits under `-fsanitize=memory` so no collision, and the check actually rides MSan's own shadow tracking instead of running alongside it.
Old UBSan implementation is gone (`Sanitizers.def`, `SanitizerHandler.h`, `ubsan_checks.inc`, `ubsan_handlers.h/.cpp`). New on the MSan side: - `MD_msan_check_uninit_read` metadata kind - `CheckLocalUninitReads` on `MemorySanitizerOptions` - `visitLoadInst` hook, calls `insertCheckShadowOf` on the load's result. That function already existed but only ever got called on the pointer operand, never the loaded value itself, that's the actual gap - `-fsanitize-memory-local-address-never-taken` (your wording, kept it), through `Options.td`, `CodeGenOptions.def`, `BackendUtil.cpp`, `SanitizerArgs` - `UninitLocalVarVisitor` in `CGExprScalar.cpp` now tags the load with metadata after it's emitted instead of calling a handler before it Two bugs I hit building this. First, `visitLoadInst` passing `&I` straight in as the check anchor causes a dominance violation, the load's own shadow gets built via `NextNodeIRBuilder` which inserts after `I`, so the check ends up referencing something that doesn't dominate it yet. Fixed by grabbing `I.getNextNode()` before `NextNodeIRBuilder` runs and anchoring there instead, same pointer `NextNodeIRBuilder` uses internally. Second, marshalled `CC1Option` flags don't auto-forward from driver mode into the `-cc1` job, that only handles parsing on the `-cc1` side. Had to add explicit forwarding in `SanitizerArgs.cpp`, copied the pattern from how `param-retval` does it. Without it the flag parses fine and just never actually reaches the compile. Also changed something I wasn't planning to touch. The old candidate-tracking erased a variable on any assignment anywhere in the function, or any asm output, a whole-function check with no path sensitivity, which I'd called out as a limitation originally. Doesn't need that anymore. MSan's shadow is already path-sensitive at runtime through phi merging, so a path that actually initializes the variable comes out with a clean shadow and the check just passes. Tested both guards in and out, each time confirmed silent with the guard, confirmed still correct (not a false positive) with it removed, full regression run after each. Zero regressions. Calling this out separately since it's a real tradeoff and not mine to decide quietly, correctness here now depends on MSan's shadow being right, not a guard I control. I think the testing supports dropping the guards, but if you want them back regardless I'll add them, they're cheap. Couple smaller things. Tried `__builtin_unreachable()` to mark a dead branch for a cleaner test case, didn't work, hitting it at runtime is itself UB and what looked like a correct warning was actually falling through to an unrelated `__msan_warning_noreturn()` nearby, pure code-layout coincidence at `-O0`. Checked the IR, no control-flow edge into the checked block from there. Left it out of the test matrix, noting it as a caveat instead. No extra handling needed for asm output operands. MSan already zeroes their shadow on its own, confirmed in the IR, there's a `store i32 0` right after the asm block before the value gets written. So asm-initialized vars read back clean under this check too, for free. New test: `compiler-rt/test/msan/local_uninit_read.c`. Positive case is a dead load (`int x; x;`), not `return x;`, since vanilla MSan already catches that through `param-retval`'s own return tracking and wouldn't test anything new. Also covers no-flag, address-taken, asm-init, aggregate, and C++ negatives, plus the conditional path-sensitive case both ways. 878/940 check-ubsan, 344/352 check-msan, no regressions. https://github.com/llvm/llvm-project/pull/207529 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
