[Bug analyzer/108028] Misleading -fanalyzer messages at -O2 and above

["mengli.ming at outlook dot com" <gcc-bugzilla@gcc.gnu.org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108028

--- Comment #3 from ming mengli <mengli.ming at outlook dot com> ---
(In reply to David Malcolm from comment #1)
> Thanks for filing this bug.
> 
> There are several things going on here.
> 
> (A): the analyzer is considering the function "f" as called standalone, as
> well as the case where it's called from "main", rather than merely
> considering it when it's called from "main".  There are a few other bug
> reports about that; it's not clear to me what we should do for this case; is
> it expected that such functions are only ever called from main?
> 
> The situation is clearer if we simply delete "main" from the reproducer. 
> With that, we see:
> 
>   'f': events 1-3
>     |
>     |    7 |     if (p && (0 == q))
>     |      |        ^
>     |      |        |
>     |      |        (1) following 'true' branch...
>     |    8 |     {
>     |    9 |         __analyzer_eval(p && (0 == q));
>     |      |         ~~~~~~~~~~~~~~~
>     |      |         |
>     |      |         (2) ...to here
>     |......
>     |   14 |         *q = 1;
>     |      |         ~~~~~~
>     |      |            |
>     |      |            (3) dereference of NULL '0'
>     |
> 
> 
> (B) arguably the CFG event (1) to (2) is poorly worded; at (1) we have
> "following 'true' branch...", which suggests it always follows the 'true'
> branch, whereas it's merely considering what happens *if* we take the 'true'
> branch.
> 
> If it read: e.g.:
> 
>   'f': events 1-3
>     |
>     |    7 |     if (p && (0 == q))
>     |      |        ^
>     |      |        |
>     |      |        (1) considering following 'true' branch (implying that
> 'q' is NULL)...
>     |    8 |     {
>     |    9 |         __analyzer_eval(p && (0 == q));
>     |      |         ~~~~~~~~~~~~~~~
>     |      |         |
>     |      |         (2) ...to here
>     |......
>     |   14 |         *q = 1;
>     |      |         ~~~~~~
>     |      |            |
>     |      |            (3) dereference of NULL '0'
>     |
> 
> the analyzer would be more obviously correct and useful.
> 
> Or even "considering when 'q' is NULL; following 'true' branch..."
> 
> I've been experimenting with making the wording here clearer
> (i): should make a distinction between when the analyzer chooses one of
> several paths to consider, versus when the choice of execution path is
> already determined by previous choices
> (ii): would be nice to capture that q's nullness is the most interesting
> property at the "if" statement with respect to the warning, and express that
> to the user.
> 
> 
> (C) The analyzer runs relatively late compared to most static analyzers, so
> the optimization levels affect things.  In particular, consider the gimple
> IR seen by -fanalyzer for the assignment:
>      *q = 1;
> in the block guarded by (0 == q).
> 
> At -O1 we have:
>      *q_10(D) = 1;
> but at -O2 it converts it to:
>      MEM[(int *)0B] = 1;
> 
> Arguably it's a bug here that we only warn at -O2 and above; analyzing "f"
> standalone, we ought to be complaining about the null dereference without
> needing -O2.
> (specifically, at -O2 -fanalyzer sees a deref of NULL, whereas at -O1 it
> merely sees a deref of INIT_VAL(q), whilst knowing the constraint that
> INIT_VAL(q) is NULL.
> The __analyzer_eval results are also due to gimple IR differences caused by
> the optimizer.
> 
> Also, perhaps we should run earlier; I probably ought to file a bug about
> that, it's a big can of worms.

Thanks a lot for your explanation of this bug, it helps a lot. I think it's
perfectly feasible to consider both cases for that such functions to be
analyzed standalone and from the main function, sorry I didn't express myself
clearly on that point. Feel free to let us know if we could do anything to help
with these. We hope gcc static analyzer will get better.