[PATCH] D146466: [clang] diagnose function fallthrough

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> Yes, but kernel developers would prefer to diagnose issues at compile time when possible, rather that at runtime. Function fallthough is diagnosable at compile time, as this patch demonstrates.

I'm not saying the diagnostic is useless.  I'm saying users are going to find cases where their perfectly good code blows up, and so the warning will get disabled for every large codebase except the Linux kernel.  And every user that goes through the process of seeing the false positive and disabling the warning will start distrusting other warnings that are actually reliable.

> Note how __ubsan_handle_out_of_bounds is literally marked RECOVERABLE in https://github.com/llvm/llvm-project/blob/fb8d894f23c5e805f0c87d89fb9d6c0eed3a0e72/compiler-rt/lib/ubsan/ubsan_handlers.h#L90-L91, and yet we generate a call to it that will fall off the end of the function when that callback returns.

Using "recoverable" UBSan doesn't actually affect the code we generate after the diagnostic, and it doesn't suppress compiler optimizations that turn provably undefined code into "unreachable".  So often the code blows up anyway.  Maybe we should try to do something different here in some cases.  For out-of-bounds specifically, I'm not sure what a fix for that would look like, though; there isn't any intuitive behavior for an out-of-bounds access.

> Can you or @efriedma demonstrate a concrete case you're thinking of? I've tried:

The cases I'm thinking of are cases which *do* actually appear to fall through, but don't because of invariants that aren't visible to the compiler.  Functions that look like `void f() { __builtin_unreachable() }` actually exist in LLVM.  (The pattern tends to show up with virtual functions...)  Or a function may or may not be noreturn depending on the argument values.  Or jump threading generated a codepath that has unconditional undefined behavior, but we can't actually prove it because there's a call to a function that isn't marked willreturn.

Have you tried building LLVM with this to see what shows up?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146466