[PATCH] D81678: Introduce frozen attribute at call sites for stricter poison analysis
Nuno Lopes via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 23 07:26:17 PDT 2020
nlopes added a comment.
I'm a bit concerned with this patch as it increases the amount of UB that LLVM exploits without any study of the impact.
For example, right now it's ok do this with clang (not with constants; make it less trivial so clang doesn't fold it right away):
int f() { return INT_MAX + 1; }
While technically this is UB in C, when lowered to LLVM IR, this function returns poison.
When the frozen attribute is attached, the function will now trigger UB in LLVM IR as well.
Is this what we want? It would be worthwhile to at least compile a few programs to check if they break. Also, what's the plan to detect these cases in ubsan? We shouldn't be exploiting new UB without having good warnings/checks in place IMO.
Note that pure function calls with 'frozen' arguments become harder to hoist from loops, for example. Since now calling this pure function can trigger UB if one of the arguments is poison. You would need to introduce frozen beforehand. I don't see this issue addressed in this patch.
For msan & friends, this patch makes sense, as they operate in a rely-guarantee way. Initialization of memory is checked, so it can be relied upon by the compiler later on.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81678/new/
https://reviews.llvm.org/D81678
More information about the llvm-commits
mailing list