[Bug analyzer/101962] New: Analyzer NULL false positive with pointer manipulation

[Bug analyzer/101962] New: Analyzer NULL false positive with pointer manipulation

[dmalcolm at gcc dot gnu.org]

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

            Bug ID: 101962
           Summary: Analyzer NULL false positive with pointer manipulation
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: dmalcolm at gcc dot gnu.org
  Target Milestone: ---

-fanalyzer emits two warnings on this code:

#define NULL ((void *)0)

int *
func1(int *ptr) {
  if (!ptr)
    return NULL;
  return ++ptr;
}

int
main() {
  int stack;
  int *a = &stack;
  a = func1(a);
  a = func1(a);
  return *a;
}

Compiler Explorer link: https://godbolt.org/z/ohecfvdd8

gcc 11.2 emits:
  <source>:16:10: warning: dereference of NULL 'a' [CWE-476]
[-Wanalyzer-null-dereference]
     16 |   return *a;
        |          ^~
for the path in which ptr is non-NULL in the first call, and then NULL in the
2nd call, i.e. for which &stack == (NULL) - 1.

Whilst this is technically correct, it won't occur in practise and is thus
effectively a false positive that we shouldn't warn for.

trunk also emits:
  <source>:16:10: warning: use of uninitialized value '*a' [CWE-457]
[-Wanalyzer-use-of-uninitialized-value]
     16 |   return *a;
        |          ^~
for the path in which ptr is non-NULL in both calls, and so we're effectively
accessing (&stack)[2], which is a true problem in the software under test, but
would be better to report as an out-of-bounds warning (the analyzer doesn't yet
do bounds checking).

Downstream report: https://bugzilla.redhat.com/show_bug.cgi?id=1995092

[Bug analyzer/101962] Analyzer NULL false positive with pointer manipulation

[dmalcolm at gcc dot gnu.org]

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

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2021-08-18

--- Comment #1 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Am testing a fix.

[Bug analyzer/101962] Analyzer NULL false positive with pointer manipulation

[cvs-commit at gcc dot gnu.org]

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

--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:e82e0f149b0aba660896ea9aa12c442c07a16d12

commit r12-3094-ge82e0f149b0aba660896ea9aa12c442c07a16d12
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Mon Aug 23 14:07:39 2021 -0400

    analyzer: assume that POINTER_PLUS_EXPR of non-NULL is non-NULL [PR101962]

    gcc/analyzer/ChangeLog:
            PR analyzer/101962
            * region-model.cc (region_model::eval_condition_without_cm):
            Refactor comparison against zero, adding a check for
            POINTER_PLUS_EXPR of non-NULL.

    gcc/testsuite/ChangeLog:
            PR analyzer/101962
            * gcc.dg/analyzer/data-model-23.c: New test.
            * gcc.dg/analyzer/pr101962.c: New test.

[Bug analyzer/101962] Analyzer NULL false positive with pointer manipulation

[dmalcolm at gcc dot gnu.org]

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

--- Comment #3 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Should be fixed on trunk for gcc 12 by the above commit.

I plan to backport this to gcc 11; keeping it open until that's done.

[Bug analyzer/101962] Analyzer NULL false positive with pointer manipulation

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mpolacek at gcc dot gnu.org

--- Comment #4 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Any update on the backport?

[Bug analyzer/101962] Analyzer NULL false positive with pointer manipulation

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by David Malcolm
<dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:de0656f98640a57cd9dfdb090264afaa06ba46cc

commit r11-9374-gde0656f98640a57cd9dfdb090264afaa06ba46cc
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Mon Aug 23 14:07:39 2021 -0400

    analyzer: assume that POINTER_PLUS_EXPR of non-NULL is non-NULL [PR101962]

    Backported from commit r12-3094-ge82e0f149b0aba660896ea9aa12c442c07a16d12,
    dropping the expected "use of uninitialized value" warning from
    pr101962.c

    gcc/analyzer/ChangeLog:
            PR analyzer/101962
            * region-model.cc (region_model::eval_condition_without_cm):
            Refactor comparison against zero, adding a check for
            POINTER_PLUS_EXPR of non-NULL.

    gcc/testsuite/ChangeLog:
            PR analyzer/101962
            * gcc.dg/analyzer/data-model-23.c: New test.
            * gcc.dg/analyzer/pr101962.c: New test.

    Signed-off-by: David Malcolm <dmalcolm@redhat.com>

[Bug analyzer/101962] Analyzer NULL false positive with pointer manipulation

[dmalcolm at gcc dot gnu.org]

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

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #6 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Backported to gcc 11 by the above commit.  I don't plan to backport to gcc 10;
marking this as resolved.

[Bug analyzer/101962] Analyzer NULL false positive with pointer manipulation

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:9bbcee450deb0f561b096924a3f148369333e54c

commit r13-3819-g9bbcee450deb0f561b096924a3f148369333e54c
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Tue Nov 8 17:49:07 2022 -0500

    analyzer: eliminate region_model::eval_condition_without_cm [PR101962]

    In r12-3094-ge82e0f149b0aba I added the assumption that
    POINTER_PLUS_EXPR of non-NULL is non-NULL (for PR analyzer/101962).

    Whilst working on another bug, I noticed that this only works
    when the LHS is known to be non-NULL via
    region_model::eval_condition_without_cm, but not when it's known through
    a constraint.

    This distinction predates the original commit of the analyzer in GCC 10,
    but I believe it became irrelevant in the GCC 11 rewrite of the region
    model code (r11-2694-g808f4dfeb3a95f).

    Hence this patch eliminates region_model::eval_condition_without_cm in
    favor of all users simply calling region_model::eval_condition.  Doing
    so enables the "POINTER_PLUS_EXPR of non-NULL is non-NULL" assumption to
    also be made when the LHS is known through a constraint (e.g. a
    conditional).

    gcc/analyzer/ChangeLog:
            PR analyzer/101962
            * region-model-impl-calls.cc: Update comment.
            * region-model.cc (region_model::check_symbolic_bounds): Fix
            layout of "void" return.  Replace usage of
            eval_condition_without_cm with eval_condition.
            (region_model::eval_condition): Take over body of...
            (region_model::eval_condition_without_cm): ...this subroutine,
            dropping the latter.  Eliminating this distinction avoids issues
            where constraints were not considered when recursing.
            (region_model::compare_initial_and_pointer): Update comment.
            (region_model::symbolic_greater_than): Replace usage of
            eval_condition_without_cm with eval_condition.
            * region-model.h
            (region_model::eval_condition_without_cm): Delete decl.

    gcc/testsuite/ChangeLog:
            PR analyzer/101962
            * gcc.dg/analyzer/data-model-23.c (test_3): New test.

    Signed-off-by: David Malcolm <dmalcolm@redhat.com>