llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.25k stars 12.08k forks source link

Warnings do not consider __builtin_assume #40986

Open davidstone opened 5 years ago

davidstone commented 5 years ago
Bugzilla Link 41641
Version trunk
OS Linux
CC @dwblaikie,@zygoloid

Extended Description

The following code emits: <source>:9:1: warning: control may reach end of non-void function [-Wreturn-type]

int f(unsigned x) {
    __builtin_assume(x <= 1);
    switch (x) {
        case 0:
            return 0;
        case 1:
            return 1;
    }
}

https://godbolt.org/z/rJGpzJ

I'm not sure how feasible fixing this would be, but I would like to be able to have behavior similar to switching over an enum when I know the range of the integer is limited.

ec04fc15-fa35-46f2-80e1-5d271f2ef708 commented 5 years ago

This warning is based on a simple check to see if there is any control flow path that reaches the end of the function. It is intentionally not value-sensitive (it doesn't care that some paths might be impossible because there is no combination of values that would cause them to be executed), in order to keep the runtime low enough that the warning can be enabled by default. (The Clang Static Analyzer has a more precise warning if you prefer.)

That said, we do have some logic to ignore paths that miss all cases in a switch statement (for a switch over an enum); for instance, we do not warn on:

enum E { a, b, c }; int f(E x) { switch (x) { case a: return 0; case b: return 1; case c: return 2; } }

... even though control can fall off the function if x is (E)3. We could consider extending this to handle switches over non-enumeration types, and effectively treat all paths in which no cases of a 'switch' match to be false. We'd need to do some analysis to understand the false positive/negative rates we'd be trading off here.

There are alternative ways to write your switch statement to avoid the false-positive warning, such as:

int f(unsigned x) { switch (x) { case 0: return 0; case 1: return 1; } __builtin_unreachable(); }

(or "default: __builtin_unreachable();" if you prefer).