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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2022-11-29
     Ever confirmed|0                           |1
           Keywords|                            |missed-optimization

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
The array bound diagnostic happens on

__builtin_memcpy (_48, _137, _Num.12_51);

where we have

_89 = operator new (4);
_48 = _89 + 4;

so the destination has no space left, thus _Num.12_51 must be zero but the
actual guard of the memcpy is:

if (_Num_50 != 0)
  goto <bb 11>; [33.00%]
else
  goto <bb 12>; [67.00%]

<bb 11> [local count: 72751929]:
_Num.12_51 = (long unsigned int) _Num_50;
__builtin_memcpy (_48, _137, _Num.12_51);

so the value-range of that is known > 0.  There's a missed optimization:

<bb 2> [local count: 1073741824]:
_5 = bytes.D.25336._M_impl.D.24643._M_start;
_6 = bytes.D.25336._M_impl.D.24643._M_finish;
pretmp_66 = bytes.D.25336._M_impl.D.24643._M_end_of_storage;
if (_5 != _6)
  goto <bb 3>; [70.00%]
else
  goto <bb 4>; [30.00%]

...

<bb 6> [local count: 329045359]:
# _137 = PHI <_6(4), _5(3)>
_89 = operator new (4);
_43 = bytes.D.25336._M_impl.D.24643._M_start;
_Num_44 = _137 - _43;
if (_Num_44 != 0)
  goto <bb 7>; [33.00%]
else
  goto <bb 9>; [67.00%]

here we could see that _137 is equal to _5 and thus _Num_44 is zero (not
sure why _43 was not yet CSEd to _5 here, supposedly this is exposed too
late for the last FRE?).  Knowing this would make the memcpy unreachable.
So there's some std::__throw_length_error unresolved still which
clobbers 'bytes'.

I have a patch to elide the PHI node but the issue with
bytes.D.25336._M_impl.D.24643._M_start not CSEd remains, it's in this
case possibly clobbered by operator new (4).

There's the possible option to avoid re-loading from 'this' in the
involved(?) std::vector implementation (vector.tcc around line 800
which is where eventually the operator new originates from).  IIRC we
removed an optimization that 'new' cannot clobber global memory because
of correctness issues recently.

Reply via email to