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.

Reply via email to