devsw-prayas wrote: Yeah, agreed, alloca+store+load per candidate isn't "small runtime cost, no ABI impact." That's just not the UBSan contract. Easiest fix: pull `uninitialized-read` out of the default `Undefined` group, make it opt-in like `unsigned-integer-overflow` already is. Keeps the check, drops the "changes what `-fsanitize=undefined` costs you by default" problem.
Checked this before agreeing to anything. Turns out the "address never taken" bit isn't a C23 thing at all, it's part of the definition of *indeterminate value* itself, added by [[DR338](https://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_338.htm)](https://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_338.htm) against C99/C11. So it's been true since C11. Given that I'd rather keep the `!CPlusPlus` gate as-is (all C dialects) instead of narrowing to C23 via `getLangOpts()`, since narrowing would actually make it miss real cases in C11/C17. The "C23 6.3.2.1p2" citation was my bad, that's just the current edition's clause number for an older rule; will fix the naming/docs instead of the gating. Though if you'd rather keep it C23-only anyway (e.g. smaller blast radius while it's new), I'm fine with that too, just wanted the spec history on the table first. **Naming vs. `-fsanitize=memory`:** Fair, genuinely confusing. No strong opinion on the rename, happy to defer. **MSan annotations:** Actually went and read `MemorySanitizer.cpp` instead of guessing. Userspace MSan already poisons every stack alloca at function entry by default (via memset or a runtime call, depending on build flags) and unpoisons on store, basically the exact state-tracking this check needs, already built and already paid for. (Kernel-mode KMSAN has a separate poisoning path I haven't checked, so this is a userspace-MSan claim specifically.) The one gap: `visitLoadInst` propagates shadow forward but only ever checks the *pointer's* shadow, gated behind `ClCheckAccessAddress`, never the loaded value's own shadow. So the front-end side of this would just be tagging qualifying loads and adding a shadow check on the loaded value in `visitLoadInst`, smaller than what's in this PR now. Tradeoff: it'd only fire under `-fsanitize=memory`, so plain UBSan users lose the diagnostic entirely. Honestly I think all four of these are really one thing. MSan's job is catching uninitialized memory use, and uninitialized memory use is already UB by definition. Normally that overlap doesn't matter because MSan only fires at sinks while UBSan checks are local/stateless, so they never actually compete. This rule is the weird case: UB the moment you read it, no sink needed, which makes it read like a UBSan check on paper but only catchable with MSan-shaped state. So it's not really "which flag" or "what do we call it," it's "which sanitizer actually owns this," and everything else falls out of that answer. https://github.com/llvm/llvm-project/pull/207529 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
