[PATCH] D146466: [clang] diagnose function fallthrough

Thu Mar 23 12:51:08 PDT 2023

nickdesaulniers added a comment.

In D146466#4217487 <https://reviews.llvm.org/D146466#4217487>, @efriedma wrote:

> The problem with a change like that is that it's not clear what the underlying semantic model is.  If we add a flag that says "try to generate code that matches Linux kernel developers' mental model of the underlying machine", or "loop unrolling should try to preserve undefined behavior", or "out-of-bounds memory accesses are well-defined", the semantics are unclear.  The point of having well-defined IR is that a developer can tell whether a transform is legal or not based on reading LangRef, as opposed to trying to blindly guess whether a transform is going to break some user's code.

I think it needs to be making specific cases of UB defined when specific santitizers trying to catch those UB cases are enabled, if those sanitizer call backs are expected to be recoverable (i.e. the ones that are not noreturn).  (This reminds me of a transform I //think// LLVM does where the `blockaddress` of a removed basic block gets replaced with `1`).

> Making clang generate different IR for specific load/division/etc. operators is much easier to specify; IR has the same meaning it always has, we're just generating IR that's well-defined in more cases.

So perhaps if `-fsanitize=array-bounds` was enabled, clang would add metadata to each load to say "this is an array bounds sanitizer load" (and metadata for `udiv` when `-fsanitize=integer-divide-by-zero`)? I think then we'd still need to teach `simplifyInstruction` to look for such metadata and do something different (like replace with freeze or 0 or 1 or whatever).

Or did you have something else in mind?

(Is it worth moving this discussion to another RFC? I would like to ask folks from https://users.cs.utah.edu/~regehr/papers/undef-pldi17.pdf their thoughts on `freeze` wrt this issue.)

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146466/new/

https://reviews.llvm.org/D146466