[PUSHED] Fix off-by-one storage problem in irange_allocator.

[Martin Sebor]

On 10/6/20 1:52 AM, Jakub Jelinek via Gcc-patches wrote:
> On Tue, Oct 06, 2020 at 09:37:21AM +0200, Aldy Hernandez via Gcc-patches wrote:
>> Pushed as obvious.
>>
>> gcc/ChangeLog:
>>
>> 	* value-range.h (irange_allocator::allocate): Increase
>> 	newir storage by one.
>> ---
>>   gcc/value-range.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gcc/value-range.h b/gcc/value-range.h
>> index 94b48e55e77..7031a823138 100644
>> --- a/gcc/value-range.h
>> +++ b/gcc/value-range.h
>> @@ -670,7 +670,7 @@ irange_allocator::allocate (unsigned num_pairs)
>>
>>     struct newir {
>>       irange range;
>> -    tree mem[1];
>> +    tree mem[2];
>>     };
>>     size_t nbytes = (sizeof (newir) + sizeof (tree) * 2 * (num_pairs - 1));
>>     struct newir *r = (newir *) obstack_alloc (&m_obstack, nbytes);
> 
> So, we essentially want a flexible array member, which C++ without extension
> doesn't have, and thus need to rely on the compiler handling the trailing
> array as a poor men's flexible array member (again, GCC does for any size,
> but not 100% sure about other compilers, if they e.g. don't handle that way
> just size of 1).

The manual documents the [0] extension and mentions but discourages
using [1].  Nothing is said about other sizes and the warnings such
as -Warray-bounds have been increasingly complaining about accesses
past the declared constant bound (it won't complain about past-
the-end accesses to a mem[1], but will about those to mem[2]).

It would be nice if existing GCC code could eventually be converted
to avoid relying on the [1] hack.  I would hope we would avoid making
use of it in new code (and certainly avoid extending its uses to other
sizes).

If it's difficult to write efficient C++ code without relying on
these hacks we are in the perfect position to propose a solution
to C++.  Otherwise, if a portable solution already exists, we
should be able to adopt it.

Martin

> Is there any reason why the code is written that way?
> I mean, we could just use:
>    size_t nbytes = sizeof (irange) + sizeof (tree) * 2 * num_pairs;
>    irange *r = (irange *) obstack_alloc (&m_obstack, nbytes);
>    return new (r) irange ((tree *) (r + 1), num_pairs);
> without any new type.
> 
> 	Jakub
>