[Bug analyzer/99716] New: -Wanalyzer-double-fclose when fclose is called inside a loop

[Bug analyzer/99716] New: -Wanalyzer-double-fclose when fclose is called inside a loop

[rjones at redhat dot com]

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

            Bug ID: 99716
           Summary: -Wanalyzer-double-fclose when fclose is called inside
                    a loop
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: rjones at redhat dot com
  Target Milestone: ---

This seems pretty bogus to me:

#include <stdio.h>
#include <stdlib.h>

int
main ()
{
  int i;

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

$ gcc -O2 -fanalyzer /tmp/fclose-loop.c -o /tmp/fclose-loop
/tmp/fclose-loop.c: In function ‘main’:
/tmp/fclose-loop.c:12:5: warning: double ‘fclose’ of FILE ‘fp’
[-Wanalyzer-double-fclose]
   12 |     fclose (fp);
      |     ^~~~~~~~~~~
  ‘main’: events 1-7
    |
    |    9 |   for (i = 0; i < 2; ++i) {
    |      |               ~~^~~
    |      |                 |
    |      |                 (1) following ‘true’ branch (when ‘i != 2’)...
    |      |                 (5) following ‘true’ branch (when ‘i != 2’)...
    |   10 |     FILE *fp = fopen ("/tmp/test", "w");
    |      |                ~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                |
    |      |                (2) ...to here
    |      |                (3) opened here
    |      |                (6) ...to here
    |   11 |     fprintf (fp, "hello");
    |   12 |     fclose (fp);
    |      |     ~~~~~~~~~~~  
    |      |     |
    |      |     (4) first ‘fclose’ here
    |      |     (7) second ‘fclose’ here; first ‘fclose’ was at (4)

This is with:
gcc-11.0.0-0.19.fc35.x86_64

I do not have any later version of GCC to test, but also I don't see any
other bug about this.

[Bug analyzer/99716] -Wanalyzer-double-fclose when fclose is called inside a loop

[dmalcolm at gcc dot gnu.org]

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

--- Comment #1 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Thanks.  Looks very similar to bug 93695 - the analyzer "gets confused" in how
it tracks allocations in a loop (albeit with a different kind of resource, and
thus worth tracking as a separate bug).

[Bug analyzer/99716] -Wanalyzer-double-fclose when fclose is called inside a loop

[dmalcolm at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-03-23
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED

--- Comment #2 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Confirmed.

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

We're not transitioning fp_7 from "unchecked" back to "opened" at the 2nd
fopen.

Also, I think we're missing a warning about "fp" possibly being NULL, for the
case where the fopen fails.

[Bug analyzer/99716] -Wanalyzer-double-fclose when fclose is called inside a loop

[rjones at redhat dot com]

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

--- Comment #3 from Richard W.M. Jones <rjones at redhat dot com> ---
FYI the original code is:
https://github.com/libguestfs/libguestfs/blob/e0a11061035d47b118c95706240bcc17fd576edc/tests/mount-local/test-parallel-mount-local.c#L299-L335

[Bug analyzer/99716] -Wanalyzer-double-fclose when fclose is called inside a loop

[dmalcolm at gcc dot gnu.org]

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

--- Comment #4 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
(In reply to David Malcolm from comment #2)
> Also, I think we're missing a warning about "fp" possibly being NULL, for
> the case where the fopen fails.

I've filed this as bug 99738.

[Bug analyzer/99716] -Wanalyzer-double-fclose when fclose is called inside a loop

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 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/99716] -Wanalyzer-double-fclose when fclose is called inside a loop

[dmalcolm at gcc dot gnu.org]

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

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

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

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