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

[Jeff Law <jeffreyalaw@gmail.com>]

On 10/25/2021 1:31 PM, Martin Sebor wrote:
> On 10/25/21 12:57 PM, Jeff Law wrote:
>>
>>
>> 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.
>
> The loop computes the size of the "filename2" string so the result
> is 10, no?
Oh, duh.  I'm not sure that Ranger will pick that up though.

jeff