Radovan =?utf-8?q?Božić?= <[email protected]>, Radovan =?utf-8?q?Božić?= <[email protected]>, Radovan =?utf-8?q?Božić?= <[email protected]> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/[email protected]>
AaronBallman wrote: > > I think this is already the precedent? [...] > > I don't think so. AFAICT the currently added attributes simply add semantics > guaranteed by some standard/are fundamental parts of the contract of a > function (e.g. `noreturn`), allow removing dead code or some amount of > constant folding (e.g. `pure`), or enable diagnostics (e.g. `format`). None > of these allow exploitation of UB the library could define I think. I consider whether a pointer is allowed to be null or not to be a fundamental part of the contract of the function; you don't? Noreturn is a good example -- if a noreturn function actually returns, [that's UB](https://eel.is/c++draft/dcl.attr.noreturn#2) and we do "exploit" it: https://godbolt.org/z/5YcMnbf11 I think builtins should model as much semantic information as we would like to glean, both for diagnostics and for optimizations. But if there are good reasons to ignore some of that information, then I think it's reasonable for us to do so. I don't think there are good reasons to ignore that information in this specific case, but we've ignored it in the past. > > Do you have some usage pattern you're worried about? If we have evidence > > this will be disruptive, that would be compelling to know about. > > I don't. I mostly didn't see any discussion of whether it's a good idea to > potentially introduce UB here, which worried me somewhat, especially since I > think this patch does introduce a novel idea. This does not *introduce* UB here. It's already UB and we have evidence that standard library implementations treat it as behaviorally undefined: https://godbolt.org/z/bqn16fWTs and optimize on it, even without the standard library marking anything explicitly: https://godbolt.org/z/3zY6MzcY9 So I don't think what this patch is doing is actually novel. That said, I don't think we have a good plan for hardened libraries. We have no notion of conditionally applying these markings in tablegen, and perhaps we need to think of something for that. e.g., if the user passes `-fhardened` to the compiler (or whatever we end up exposing), I can imagine we'd want to switch from `__attribute__((nonnull))` to `_Nonnull` semantics in that case. But I'm also not certain we need such a mechanism today for this PR, either. CC @rjmccall @rnk @efriedma-quic @jyknight for some additional opinions, in case my stance is off-base. https://github.com/llvm/llvm-project/pull/160988 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
