[Bug analyzer/98223] New: gcc.dg/analyzer/pr94851-1.c XPASSes

[Bug analyzer/98223] New: gcc.dg/analyzer/pr94851-1.c XPASSes

[ro at gcc dot gnu.org]

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

            Bug ID: 98223
           Summary: gcc.dg/analyzer/pr94851-1.c XPASSes
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: ro at gcc dot gnu.org
  Target Milestone: ---

Between 20201007 (dae673abd37d400408959497e50fe1f3fbef5533) and 20201008
(3e1123e52f8eca2650efa0bc81768792d328961f),
gcc.dg/analyzer/pr94851-1.c began to XPASS:

XPASS: gcc.dg/analyzer/pr94851-1.c bogus leak (test for bogus messages, line
43)

I initially saw it on i386-pc-solaris2.11 and sparc-sun-solaris2.11 (both 32
and
64-bit), but according to gcc-testresults, it happens on many targets
(everywhere?).

Maybe just remove the xfail?

[Bug analyzer/98223] gcc.dg/analyzer/pr94851-1.c XPASSes

[ro at gcc dot gnu.org]

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

Rainer Orth <ro at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.0

[Bug analyzer/98223] gcc.dg/analyzer/pr94851-1.c XPASSes

[dmalcolm at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2021-01-05

--- Comment #1 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Thanks for filing this.  I'm seeing it with x86_64-pc-linux-gnu.

1644d7f4c1c4f99231d7de5e35fa7ce2d2e2c4c6 builds and does not have the XPASS
fcae5121154d1c3382b056bcc2c563cedac28e74 aka "Hybrid EVRP and testcases" builds
and has the XPASS

(the intervening commit (90e88fd376bb9ad6223a1f5ccd803d1bd9539b05 aka "Ranger
classes doesn't build)

Hence it looks like the hybrid EVRP range changed this testcase.

[Bug analyzer/98223] gcc.dg/analyzer/pr94851-1.c XPASSes

[dmalcolm at gcc dot gnu.org]

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

--- Comment #2 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
I believe the existing false positive may relate to bug 97072, where the
analyzer doesn't capture that the pointer to the malloc-ed buffer has been
written to one of the fields (perhaps due to state-merging), and hence reports
a leak when the frame is popped.

Comparing the before/after EVRP patch gimple dumps, I see:
    --- pr94851-1.c.071i.analyzer.before    2021-01-05 11:08:02.809471975 -0500
    +++ pr94851-1.c.071i.analyzer.after     2021-01-05 11:04:26.915680635 -0500
    @@ -39,7 +39,7 @@ pamark ()
         goto <bb 6>; [5.50%]

       <bb 6> [local count: 114863531]:
    -  # p_2 = PHI <p_8(4), p_8(5)>
    +  # p_2 = PHI <0B(4), p_8(5)>
       # last_16 = PHI <last_10(4), last_10(5)>
       if (p_2 != 0B)
         goto <bb 7>; [70.00%]

i.e. the ranger EVRP patch adds the "knowledge" that p_2 is NULL when reached
from BB 4.

Comparing the .supergraph.eg.dot dumps I see that both before/after consider a
malloc leak, but after the EVRP patch it considers the path in which the loop
isn't entered due to p being NULL.  It's considering the shortest path through
the egraph as:

      24   │   while (p != (AMARK *)NULL && p->m_name != (char)c) {  <-- (1) p
NULL here, skipping the loop
      25   │     last = p;
      26   │     p = p->m_next;
      27   │   }
      28   │ 
      29   │   if (p != (AMARK *)NULL) { <-- (2) p non-NULL here, skipping the
malloc
      30   │     printf("over writing mark %c\n", c);
      31   │   } else {
      32   │     if ((p = (AMARK *)malloc(sizeof(AMARK))) == (AMARK *)NULL)
      ...
      43   │   p->m_name = (char)c; <-- (3) to here

So the improved PHI from EVRP means that we hit bug 96374, which means that we
reject the false positive, albeit for the wrong reasons.

I plan to remove the xfail for now, but am recording this information here
since the false leak may reappear if we fix bug 96374 without fixing bug 97072.

[Bug analyzer/98223] gcc.dg/analyzer/pr94851-1.c XPASSes

[dmalcolm at gcc dot gnu.org]

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

--- Comment #3 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
i.e. it rejects the path as infeasible since p_2 needs to be NULL and then
non-NULL for the path conditions to be satisfied

[Bug analyzer/98223] gcc.dg/analyzer/pr94851-1.c XPASSes

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 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:df1eba3ceada6e8990c00ccfa6c5a2c9b1c13334

commit r11-6495-gdf1eba3ceada6e8990c00ccfa6c5a2c9b1c13334
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Tue Jan 5 20:51:50 2021 -0500

    analyzer: remove xfail [PR98223]

    The bogus leak message went away after
    fcae5121154d1c3382b056bcc2c563cedac28e74 (aka "Hybrid EVRP and
    testcases") due to that patch improving a phi node in the gimple input
    to the analyzer.

    gcc/testsuite/ChangeLog:
            PR analyzer/98223
            * gcc.dg/analyzer/pr94851-1.c: Remove xfail.

[Bug analyzer/98223] gcc.dg/analyzer/pr94851-1.c XPASSes

[dmalcolm at gcc dot gnu.org]

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

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

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

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