Re: ToT glibc build problem with ToT GCC

[Carlos O'Donell <carlos at redhat dot com>]

On 8/29/19 7:03 PM, Jeff Law wrote:
> On 8/29/19 1:53 PM, Martin Sebor wrote:
>> On 8/29/19 1:17 PM, Carlos O'Donell wrote:
>>> On 8/29/19 2:36 PM, Martin Sebor wrote:
>>>> On 8/29/19 11:54 AM, Carlos O'Donell wrote:
>>>>> On 8/29/19 1:28 PM, Martin Sebor wrote:
>>>>>> A longer term solution is to replace the array with a suitably
>>>>>> aligned char buffer and write the data into it, e.g., like this:
>>>>>>
>>>>>>     union {
>>>>>>       struct charseq replace[2];
>>>>>>       char buf[sizeof (struct charseq) * 2];
>>>>>>     } u;
>>>>>>
>>>>>>     u.replace[0].nbytes = 1;
>>>>>>     const char s[] = "?";
>>>>>>     memcpy (u.buf + offsetof (struct charseq, bytes), "?", 2);
>>>>>>
>>>>>> This is still iffy but it won't trigger an out-of-bounds warning.
>>>>>> This code should still be flagged for the declaration of an object
>>>>>> with an internal zero-length array, so that will need to be dealt
>>>>>> with when such a warning is introduced (e.g., by suppressing
>>>>>> the warning).
>>>>>
>>>>> Why is this iffy? This is the kind of solution I was thinking of using.
>>>>>
>>>>> The alternative is an entirely new type that has a non-zero-length
>>>>> array that can be used for static object allocation like this, but then
>>>>> you need to cast between them in a safe way?
>>>>
>>>> It's iffy because of the interior zero-length array.
>>>>
>>>> GCC accepts zero-length array members anywhere but it doesn't
>>>> always avoid assuming their elements don't overlap other members.
>>>> Some other compilers don't allow zero-length arrays in definitions
>>>> where they're followed by another member or element of a larger
>>>> array.  I'd like to see GCC warn for such uses because of
>>>> the overlapping problem.
>>>>
>>>> Martin
>>>>
>>>> PS For example, GCC folds the test below to false.  Clang does
>>>> the same with slightly different test cases.
>>>>
>>>>    struct charseq replace[2];
>>>>
>>>>    void f (int i)
>>>>    {
>>>>      int x = *replace[1].name;
>>>>
>>>>      replace[0].bytes[i] = 0;
>>>>
>>>>      if (x != *replace[1].name)
>>>>        __builtin_abort ();
>>>>    }
>>>
>>> This static allocation is rare, it's the only one.
>>>
>>> Is there a viable solution for a static allocation of a structure
>>> that ends in a VLA?
>>
>> The static allocation isn't necessarily a problem.  Defining arrays
>> or members of structures with flexible array members (or zero-length
>> arrays) in a way that allows overlapping is.
>>
>> The only viable alternative I can think of is to define replace as
>> a char buffer:
>>
>>   static _Alignas (struct charseq)
>>   unsigned char replace[sizeof (struct charseq[2])];
>>
>>   memcpy (replace + offsetof (struct charseq, nbytes),
>>                     (int[]){ 1 }, sizeof (int));
>>
>>   memcpy (replace + offsetof (struct charseq, bytes), "?", 2);
>>
>> This isn't pretty but it's close to strictly conforming.  The only
>> non-conforming part other than the zero-length member is that (if)
>> the char array is later accessed as struct charseq.  But that's so
>> pervasive that it might as well be in the standard.
> Couldn't we just drop the whole static initialization and do something like
> 
>   static struct charseq *replace = NULL;
> 
>   if (!replace)
>     replace = malloc (sizeof (struct charseq) + 2);
> 
> It leaks the chunk of memory though.

There are cases where static allocation is enough. Yes, we could
switch to dynamic allocation, and for something this size we would
now go through tcache, take no locks, and get back a block in 
nanoseconds.

It is tempting to just switch to dynamic allocation.

The memory would have to get reclaimed via __libc_freeres such
that valgrind won't report it as a leak.

-- 
Cheers,
Carlos.