[Bug analyzer/99044] New: use-after-free false positive

[Bug analyzer/99044] New: use-after-free false positive

[antonio.chirizzi at gmail dot com]

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

            Bug ID: 99044
           Summary: use-after-free false positive
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: antonio.chirizzi at gmail dot com
  Target Milestone: ---

Created attachment 50157
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50157&action=edit
Reproducer for the bug

Please find attached the reproducer for this use-after-free which looks a false
positive.

ls-tree-with-commit.c: In function ‘oid2strbuf_free’:
ls-tree-with-commit.c:53:17: warning: use after ‘free’ of ‘e_strbuf’ [CWE-416]
[-Wanalyzer-use-after-free]
   53 |                 strbuf_release(e_strbuf->value);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ‘oid2strbuf_free’: events 1-6
    |
    |   51 |         while ((e = hashmap_iter_next(&iter))) {
    |      |                ^
    |      |                |
    |      |                (1) following ‘true’ branch (when ‘e’ is
non-NULL)...
    |      |                (4) following ‘true’ branch (when ‘e’ is
non-NULL)...
    |   52 |                 struct oid2strbuf *e_strbuf = (struct oid2strbuf
*)e;
    |      |                                    ~~~~~~~~
    |      |                                    |
    |      |                                    (2) ...to here
    |      |                                    (5) ...to here
    |   53 |                 strbuf_release(e_strbuf->value);
    |      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                 |
    |      |                 (6) use after ‘free’ of ‘e_strbuf’; freed at (3)
    |   54 |                 free(e_strbuf->value);
    |   55 |                 free(e);
    |      |                 ~~~~~~~
    |      |                 |
    |      |                 (3) freed here



use "gcc -fanalyzer -c ls-tree-with-commit.c"

[Bug analyzer/99044] use-after-free false positive in loop

[dmalcolm at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2021-02-09
             Status|UNCONFIRMED                 |ASSIGNED
            Summary|use-after-free false        |use-after-free false
                   |positive                    |positive in loop

--- Comment #1 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Thanks for filing this bug report.

Confirmed with trunk.  Looks like the analyzer gets confused about the repeated
frees relating to "e", where it seems to confuse the value of e in the first
iteration with that of the second iteration.

Similar to PR analyzer/93695 in that respect.

[Bug analyzer/99044] use-after-free false positive in loop

[cvs-commit at gcc dot gnu.org]

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

--- 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:71fc4655ab86ab66b40165b2cb49c1395ca82a9a

commit r11-7820-g71fc4655ab86ab66b40165b2cb49c1395ca82a9a
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Wed Mar 24 20:47:57 2021 -0400

    analyzer; reset sm-state for SSA names at def-stmts
[PR93695,PR99044,PR99716]

    Various false positives from -fanalyzer involve SSA names in loops,
    where sm-state associated with an SSA name from one iteration is
    erroneously reused in a subsequent iteration.

    For example, PR analyzer/99716 describes a false
      "double 'fclose' of FILE 'fp'"
    on:

      for (i = 0; i < 2; ++i) {
        FILE *fp = fopen ("/tmp/test", "w");
        fprintf (fp, "hello");
        fclose (fp);
      }

    where the gimple of the loop body is:

      fp_7 = fopen ("/tmp/test", "w");
      __builtin_fwrite ("hello", 1, 5, fp_7);
      fclose (fp_7);
      i_10 = i_1 + 1;

    where fp_7 transitions to "closed" at the fclose, but is not
    reset at the subsequent fopen, leading to the false positive
    when the fclose is re-reached.

    The fix is to reset sm-state for svalues that involve an SSA name
    at the SSA name's def-stmt, since the def-stmt effectively changes
    the meaning of those related svalues.

    gcc/analyzer/ChangeLog:
            PR analyzer/93695
            PR analyzer/99044
            PR analyzer/99716
            * engine.cc (exploded_node::on_stmt): Clear sm-state involving
            an SSA name at the def-stmt of that SSA name.
            * program-state.cc (sm_state_map::purge_state_involving): New.
            * program-state.h (sm_state_map::purge_state_involving): New decl.
            * region-model.cc (selftest::test_involves_p): New.
            (selftest::analyzer_region_model_cc_tests): Call it.
            * svalue.cc (class involvement_visitor): New class
            (svalue::involves_p): New.
            * svalue.h (svalue::involves_p): New decl.

    gcc/testsuite/ChangeLog:
            PR analyzer/93695
            PR analyzer/99044
            PR analyzer/99716
            * gcc.dg/analyzer/attr-malloc-CVE-2019-19078-usb-leak.c: Remove
            xfail.
            * gcc.dg/analyzer/pr93695-1.c: New test.
            * gcc.dg/analyzer/pr99044-1.c: New test.
            * gcc.dg/analyzer/pr99044-2.c: New test.
            * gcc.dg/analyzer/pr99716-1.c: New test.
            * gcc.dg/analyzer/pr99716-2.c: New test.
            * gcc.dg/analyzer/pr99716-3.c: New test.

[Bug analyzer/99044] use-after-free false positive in loop

[dmalcolm at gcc dot gnu.org]

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

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

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

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