Make full use of context-sensitive ranges in access warnings

[Andrew MacLeod]

On 10/25/21 4:24 PM, Jeff Law via Gcc-patches wrote:
>
>
> 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.
Absolutely not on its own, unless its globally set earlier by loop 
analysis or someone else.