[Bug c++/108385] New: false positive -Wfree-nonheap-object

[Bug c++/108385] New: false positive -Wfree-nonheap-object

[steveire at gmail dot com]

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

            Bug ID: 108385
           Summary: false positive -Wfree-nonheap-object
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: steveire at gmail dot com
  Target Milestone: ---

Sorry I was not able to reduce this further. Changing almost anything makes the
bug no-longer reproduce:


```

#include <vector>
#include <cstdint>
#include <cstring>

class DataType {
 public:
  DataType() {
   }

  DataType get() const;

 private:
  double v = 0.0;
  char values[41];
};

class ptrType {
 public:
  DataType someMethod() const {
    DataType t;
    t = t.get();
    return t;
  }
};

class AnotherDataType {
 public:
  typedef uint32_t size_type;

  AnotherDataType() : _size(0), _data(new double[0]) {}

  explicit AnotherDataType(size_type size) : _size(size), _data(new
double[size]) {}

  virtual ~AnotherDataType() { delete[] _data; }

  uint32_t size() const { return _size; }

  double& operator()(size_type i) { return _data[i]; }

  AnotherDataType get(const AnotherDataType& b) const
  {
    AnotherDataType c(size());

    auto aItr = _data;
    auto cItr = c.begin();
    auto endp = _data + _size;

    for (; aItr != endp; ++aItr, ++cItr) {
      (*cItr) = (*aItr);
    }
    return c;
  }

  double sum() const {
    double sum = *_data;
    auto aItr = _data;
    for (; aItr != _data + _size; ++aItr) {
      sum = (*aItr);
    }
    return sum;
  }

  double* begin() { return _data; }

 private:
  size_type _size;
  double* _data;
};

AnotherDataType anotherMethod(const ptrType* ptrType1) {
  ptrType1->someMethod();
  return {};
}

struct otherStruct {
  const ptrType* ptrType1;
  std::vector<double> q1;
};

static double minF(otherStruct* params) {
  auto err = anotherMethod(params->ptrType1);

  return (err.get(err)).sum();
}

struct someStruct {
  double (*f)(otherStruct* params);
  otherStruct* params;
};

void foo(someStruct function) {
  std::vector<double> v;

  minF(function.params);
}

void why() {
  someStruct func;
  func.f = &minF;
  foo(func);
}
```

Godbolt link: https://godbolt.org/z/nqvsezj49

[Bug tree-optimization/108385] false positive -Wfree-nonheap-object

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
-std=c++20 -O2

[Bug tree-optimization/108385] false positive -Wfree-nonheap-object

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The warning is gone on the trunk.

[Bug tree-optimization/108385] [12 Regression] false positive -Wfree-nonheap-object

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|false positive              |[12 Regression] false
                   |-Wfree-nonheap-object       |positive
                   |                            |-Wfree-nonheap-object
                 CC|                            |hubicka at gcc dot gnu.org,
                   |                            |marxin at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2023-01-13

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
Fixed on master with r13-1450-gd2a898666609452e and it started with
r12-4787-gb8ef019ab938471f.

[Bug tree-optimization/108385] [12 Regression] false positive -Wfree-nonheap-object

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.3
             Blocks|                            |85316
                 CC|                            |aldyh at gcc dot gnu.org,
                   |                            |amacleod at redhat dot com
           Keywords|                            |missed-optimization

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
<bb 2> [local count: 118111600]:
_1 = params_3(D)->ptrType1;
err = anotherMethod (_1); [return slot optimization]
_31 = err._size;
_32 = (sizetype) _31;
iftmp.0_33 = _32 * 8;
_34 = operator new [] (iftmp.0_33);

<bb 3> [local count: 118111600]:
aItr_35 = err._data;
if (iftmp.0_33 != 0)
  goto <bb 4>; [89.00%]
else
  goto <bb 7>; [11.00%]

<bb 4> [local count: 105119324]:
__builtin_memcpy (_34, aItr_35, iftmp.0_33);
sum_9 = MEM[(double *)_34];
_38 = _34 + iftmp.0_33;
if (_34 != _38)
  goto <bb 5>; [89.00%]
else
  goto <bb 6>; [11.00%]

(gdb) p debug_bb_n (6)
<bb 6> [local count: 1271944]:
operator delete [] (_38);
goto <bb 10>; [100.00%]

there's a missed optimization.  We are on the path iftmp.0_33 != 0 so
the _34 != _38 check should evaluate to true and BB 6 with the deallocation
be unreachable.

The "fix" on trunk looks bogus and more like a missed optimization somewhere
causing this not to be exposed.

At VRP2 time we see

  <bb 3> [local count: 118111600]:
  aItr_35 = err._data;
  endp_39 = aItr_35 + iftmp.0_33;
  if (aItr_35 != endp_39)

and

  _38 = _34 + iftmp.0_33;
  if (_38 != _34)

it looks like we fail to infer a range for iftmp.0_33 from the BB3 condition
and fail to use that to simplify the later check.

The first check is rewritten to iftmp.0_33 != 0 by forwprop4 but that
doesn't rewrite the later check, likely because _39 has a single-use
but _38 does not.  On trunk both are rewritten to iftmp.0_33 != 0 by

 (simplify
  (op:c (nop_convert?@3 (pointer_plus@2 (convert1? @0) @1)) (convert2? @0))
  (if (tree_nop_conversion_p (TREE_TYPE (@2), TREE_TYPE (@0))
       && tree_nop_conversion_p (TREE_TYPE (@3), TREE_TYPE (@0))
       && (CONSTANT_CLASS_P (@1) || (single_use (@2) && single_use (@3))))
   (op @1 { build_zero_cst (TREE_TYPE (@1)); }))))

because _38 there _is_ a single use, likely due to that missed optimization
(jump threading?).

As said, the question is why we do not derive a range for iftmp.0_33 here
("here" is the minF function).  The dumps say on trunk

=========== BB 3 ============
Imports: iftmp.0_33  aItr_35 
Exports: iftmp.0_33  aItr_35  endp_39
         endp_39 : iftmp.0_33(I)  aItr_35(I)
iftmp.0_33      [irange] long unsigned int [0, 34359738360] NONZERO 0x7fffffff8
Equivalence set : [endp_39]
Equivalence set : [aItr_35]
    <bb 3> [local count: 118111600]:
    aItr_35 = err._data;
    endp_39 = aItr_35 + iftmp.0_33;
    if (aItr_35 != endp_39)
      goto <bb 4>; [89.00%]
    else
      goto <bb 7>; [11.00%]

3->4  (T) iftmp.0_33 :  [irange] long unsigned int [0, 34359738360] NONZERO
0x7fffffff8 
3->7  (F) iftmp.0_33 :  [irange] long unsigned int [0, 34359738360] NONZERO
0x7fffffff8

huh, so why for aItr_35 == endp_39 is the iftmp.0_33 range not zero?!

=========== BB 4 ============
Imports: iftmp.0_33  _34  
Exports: _16  iftmp.0_33  _34
         _15 : aItr_35(I)  
         _16 : iftmp.0_33(I)  _34(I)
         _47 : endp_39(I)   
_15     [irange] unsigned long [1, +INF]
iftmp.0_33      [irange] long unsigned int [0, 34359738360] NONZERO 0x7fffffff8
aItr_35 [irange] double * [1, +INF]
endp_39 [irange] double * VARYING
Equivalence set : [sum_9]
Equivalence set : [_16]
Partial equiv (_15 pe64 aItr_35)
Partial equiv (_47 pe64 endp_39)
Relational : (aItr_35 != endp_39)


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85316
[Bug 85316] [meta-bug] VRP range propagation missed cases

[Bug tree-optimization/108385] [12 Regression] false positive -Wfree-nonheap-object

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
[E]VRP testcase which shows the odd 'off' range on the false edge:

void bar(char *);

void foo (char *p, char *pp, int off)
{
  char *q = p + off;
  if (q != p)
    bar (q);
  char *qq = pp + off;
  if (qq != pp)
    bar (qq);
  bar (p);
  bar (pp);
}

[Bug tree-optimization/108385] [12 Regression] false positive -Wfree-nonheap-object

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #5)
> [E]VRP testcase which shows the odd 'off' range on the false edge:
> 
> void bar(char *);
> 
> void foo (char *p, char *pp, int off)
> {
>   char *q = p + off;
>   if (q != p)
>     bar (q);
>   char *qq = pp + off;
>   if (qq != pp)
>     bar (qq);
>   bar (p);
>   bar (pp);
> }

In the old VRP code the register_edge_assert_for_* routines would be
the ones to add pattern matching for this.  I can't find where such
code exists in ranger now - I suspected it might be in range-op
when solving q = p + off; with q == p, but that operates on ranges
and not equivalences only?

[Bug tree-optimization/108385] [12 Regression] false positive -Wfree-nonheap-object

[amacleod at redhat dot com]

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

--- Comment #7 from Andrew Macleod <amacleod at redhat dot com> ---
Created attachment 54269
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54269&action=edit
patch in testing

Patch is in testing.

We added relation processing to GORI during stage 1, but its very lightly used
as we haven't fleshed out a lot of cases.  I found a couple of minor issues
with it, which I will fix in this patch.

Furthermore, there was no op2_range implementation for pointer plus.  Normally
this is all that would have been needed.  I added it and this testcase should
now work.  At least the ranges for the offset to pointer_plus now reflect zero
or nonzero based on known equality/inequality of the 2 operands. ie:


    <bb 2> :
    _1 = (sizetype) off_5(D);
    q_7 = p_6(D) + _1;
    if (p_6(D) != q_7)
      goto <bb 3>; [INV]
    else
      goto <bb 4>; [INV]

_1 : [irange] sizetype [0, 2147483647][18446744071562067968, +INF]
2->3  (T) _1 :  [irange] sizetype [1, 2147483647][18446744071562067968, +INF]
2->3  (T) off_5(D) :    [irange] int [-INF, -1][1, +INF]
2->4  (F) _1 :  [irange] sizetype [0, 0] NONZERO 0x0
2->4  (F) off_5(D) :    [irange] int [0, 0] NONZERO 0x0


Check and see if it also resolves the original test.  It does update the
zero/nonzero outgoing ranges.

[Bug tree-optimization/108385] [12 Regression] false positive -Wfree-nonheap-object

[rguenth at gcc dot gnu.org]

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

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
LGTM, btw since this is a regression it would be nice to have it fixed "better"
on trunk.

[Bug tree-optimization/108385] [12 Regression] false positive -Wfree-nonheap-object

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Macleod <amacleod@gcc.gnu.org>:

https://gcc.gnu.org/g:1626ec53e8c1b9c245572417d380e3ed84990cff

commit r13-5579-g1626ec53e8c1b9c245572417d380e3ed84990cff
Author: Andrew MacLeod <amacleod@redhat.com>
Date:   Tue Jan 17 11:39:47 2023 -0500

    Add op2_range to pointer_plus.

    Implement op2_range for pointer_plus to determine the offset (operand 2) is
    zero or non-zero based on equality/inequality between the LHS and op1.
    Also allow GORI computations to continue if the LHS is VARYING and there
    is also a relation.

            PR tree-optimization/108385
            gcc/
            * gimple-range-gori.cc (gori_compute::compute_operand_range):
            Allow VARYING computations to continue if there is a relation.
            * range-op.cc (pointer_plus_operator::op2_range): New.

            gcc/testsuite/
            * gcc.dg/pr108385.c: New.

[Bug tree-optimization/108385] [12 Regression] false positive -Wfree-nonheap-object

[amacleod at redhat dot com]

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

--- Comment #10 from Andrew Macleod <amacleod at redhat dot com> ---
Fixed in GCC 13, unlikely to be ported to gcc12

[Bug tree-optimization/108385] [12 Regression] false positive -Wfree-nonheap-object

[amacleod at redhat dot com]

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

--- Comment #11 from Andrew Macleod <amacleod at redhat dot com> ---
To be clear, the reason it is unlikely to be ported to GCC12 is because this
depends on relation support in GORI to recognize the LHS and operand 1 are
equivalent.  That support was first added in GCC13, and is too significant a
chunk of work to port back to GCC12.

[Bug tree-optimization/108385] [12 Regression] false positive -Wfree-nonheap-object

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|12.3                        |12.4

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 12.3 is being released, retargeting bugs to GCC 12.4.