Re: Make full use of context-sensitive ranges in access warnings

[Jeff Law <jeffreyalaw@gmail.com>]

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