On 10/23/2021 5:49 PM, Martin Sebor via Gcc-patches wrote:
Somewhat belatedly following Aldy's lead on finishing
the conversion to Ranger, the attached patch modifies
gimple-ssa-warn-access and other passes that use
the pointer_query machinery to provide Ranger with
the statement it's being called to determine ranges for.
The changes are almost completely mechanical, involving
passing a GIMPLE statement around (and a range_query
pointer) all the way into the bowels of the pointer_query
class to make them available when range info is being
determined.

There might be some overlap with Aldy's tree-ssa-strlen.c
changes to do the same there.  I'll deal with any conflicts
when it comes time to commit the work.

The changes trigger a couple of -Wstringop-overread instances
in libstdc++ tests.  The warnings look valid for the IL but
the code they're in is unreachable.  One of the tests already
suppresses -Wstringop-overflow so also suppressing
-Wstringop-overread doesn't seem out of line.

Tested on x86_64-linux.

Martin

PS The warning for the u8path-char8_t.cc test is this:

/ssd/test/build/gcc-test/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/char_traits.h:355: warning: 'void* __builtin_memcpy(void*, const void*, long unsigned int)' reading between 16 and 4611686018427387903 bytes from a region of size 10 [-Wstringop-overread]

The IL for it is below.  The loop iN BB 3 exits with __i_22 equal
to 10 so BBs 5, 6 and 7 are unreachable.  It's surprising to me
that the loop isn't optimized into something better (like a MEM
array assignment or memcpy).

  <bb 2> [local count: 1073741824]:
  MEM[(struct basic_string *)&s1] ={v} {CLOBBER};
  MEM[(struct _Alloc_hider *)&s1] ={v} {CLOBBER};
  MEM[(struct _Alloc_hider *)&s1]._M_p = &s1.D.30357._M_local_buf;

  <bb 3> [local count: 8687547547]:
  # __i_109 = PHI <__i_22(3), 0(2)>
  __i_22 = __i_109 + 1;
  _24 = MEM[(const char_type &)"filename2" + __i_22 * 1];
  if (_24 != 0)
    goto <bb 3>; [89.00%]
  else
    goto <bb 4>; [11.00%]

  <bb 4> [local count: 1073741824]:   <<< __i_22 == 10 here
  if (__i_22 > 15)
    goto <bb 5>; [33.00%]
  else
    goto <bb 8>; [67.00%]

  <bb 5> [local count: 354334802]:
  if (__i_22 > 4611686018427387903)
    goto <bb 6>; [0.04%]
  else
    goto <bb 7>; [99.96%]   >>> __i_22 in [16, 4611686018427387903]

  <bb 6> [local count: 141736]:
  std::__throw_length_error ("basic_string::_M_create");

  <bb 7> [local count: 354193066]:
  _85 = __i_109 + 2;
  _42 = operator new (_85);
  s1._M_dataplus._M_p = _42;
  s1.D.30357._M_allocated_capacity = __i_22;
  __builtin_memcpy (_42, "filename2", __i_22);   << -Wstringop-overread

Do you mean __i_22 == 16 earlier?  I don't see how it's restricted to 10.

I would have expected to have a global range for i_22 of [0,16] which in turn should have allowed the optimizers to remove bb5 and bb6.  Not sure if that'd fix your overread though.

OK.  I'll let you and Aldy coordinate since y'all may be hitting some of the same bits.

jeff

Reply via email to