[PATCH] enhance -Warray-bounds to handle strings and excessive indices

[Martin Sebor]

On 11/15/2017 03:51 AM, Richard Biener wrote:
> On Tue, Nov 14, 2017 at 6:45 PM, Martin Sebor <msebor@gmail.com> wrote:
>> On 11/14/2017 05:28 AM, Richard Biener wrote:
>>>
>>> On Mon, Nov 13, 2017 at 6:37 PM, Martin Sebor <msebor@gmail.com> wrote:
>>>>
>>>> Richard, this thread may have been conflated with the one Re:
>>>> [PATCH] enhance -Warray-bounds to detect out-of-bounds offsets
>>>> (PR 82455) They are about different things.
>>>>
>>>> I'm still looking for approval of:
>>>>
>>>>   https://gcc.gnu.org/ml/gcc-patches/2017-10/msg01208.html
>>
>>
>> Sorry, I pointed to an outdated version.  This is the latest
>> version:
>>
>>   https://gcc.gnu.org/ml/gcc-patches/2017-10/msg01304.html
>>
>> My bad...
>>
>>
>>>
>>> +      tree maxbound
>>> + = build_int_cst (sizetype, ~(1LLU << (TYPE_PRECISION (sizetype) - 1)));
>>>
>>> this looks possibly bogus.  Can you instead use
>>>
>>>   up_bound_p1
>>>     = wide_int_to_tree (sizetype, wi::div_trunc (wi::max_value
>>> (TYPE_PRECISION (sizetype), SIGNED), wi::to_wide (eltsize)));
>>>
>>> please?  Note you are _not_ computing the proper upper bound here because
>>> that
>>> is what you compute plus low_bound.
>>>
>>> +      up_bound_p1 = int_const_binop (TRUNC_DIV_EXPR, maxbound, eltsize);
>>>
>>> +
>>> +      tree arg = TREE_OPERAND (ref, 0);
>>> +      tree_code code = TREE_CODE (arg);
>>> +      if (code == COMPONENT_REF)
>>> + {
>>> +  HOST_WIDE_INT off;
>>> +  if (tree base = get_addr_base_and_unit_offset (ref, &off))
>>> +    {
>>> +      tree size = TYPE_SIZE_UNIT (TREE_TYPE (base));
>>> +      if (TREE_CODE (size) == INTEGER_CST)
>>> + up_bound_p1 = int_const_binop (MINUS_EXPR, up_bound_p1, size);
>>>
>>> I think I asked this multiple times now but given 'ref' is the
>>> variable array-ref
>>> a.b.c[i] when you call get_addr_base_and_unit_offset (ref, &off) you
>>> always
>>> get a NULL_TREE return value.
>>>
>>> So I asked you to pass it 'arg' instead ... which gets you the offset of
>>> a.b.c, which looks like what you intended to get anyway.
>>>
>>> I also wonder what you compute here - you are looking at the size of
>>> 'base'
>>> but that is the size of 'a'.  You don't even use the computed offset!
>>> Which
>>> means you could have used get_base_address instead!?  Also the type
>>> of 'base' may be completely off given MEM[&blk + 8].b.c[i] would return
>>> blk
>>> as base which might be an array of chars and not in any way related to
>>> the type of the innermost structure we access with COMPONENT_REFs.
>>>
>>> Why are you only looking at COMPONENT_REF args anyways?  You
>>> don't want to handle a.b[3][i]?
>>>
>>> That is, I'd have expected you do
>>>
>>>    if (get_addr_base_and_unit_offset (ref, &off))
>>>      up_bound_p1 = wide_int_to_tree (sizetype, wi::sub (wi::to_wide
>>> (up_bound_p1), off));
>
> ^^^^^^^^^