[PATCH] D81678: Introduce frozen attribute at call sites for stricter poison analysis

[Nuno Lopes via Phabricator via cfe-commits]

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