Please review attached 0006*patch with the new changed I mentioned
previously. Thanks.



On Mon, Oct 5, 2015 at 12:04 PM, Chih-hung Hsieh <[email protected]> wrote:

> I will prepare a new patch with the suggested changes:
>
> (a) more efficient overflow check like
>         if (num > SIZE_MAX / elem_size)
>
> (b) replacing union of VLA with pointers to arrays like
>          void *data = malloc (...);
>          T32 (*a32)[n] = data;
>          T64 (*a64)[n] = data;
>
> Please feel free to suggest better solutions.
> Thanks.
>
>
> On Mon, Oct 5, 2015 at 11:45 AM, Mark Wielaard <[email protected]> wrote:
>
>> On Fri, Oct 02, 2015 at 12:10:47AM +0300, Alexander Cherepanov wrote:
>> > On 2015-09-16 18:25, Mark Wielaard wrote:
>> > >On Fri, 2015-09-11 at 12:22 -0700, Roland McGrath wrote:
>> > >>It looks fine to me from a quick skim, but Mark should review and
>> test it too.
>> > >
>> > >I am not super enthusiastic about this change, it seems to just take
>> > >away type/size information that the compiler/bounds checking tools can
>> > >use.
>> >
>> > I'm not sure I fully understand the problem. As I understand it unions
>> of
>> > VLAs are not Ok while VLAs are Ok and even desirable due to bounds
>> checking,
>> > right?
>>
>> There are two issues. First with GCC VLA types are OK and desired compared
>> to types not using bounds. But if such a VLAs might be unbounded then they
>> should not be stack allocated (this is enforced by building with
>> -Wstack-usage except for a couple of files that haven't been converted).
>> Secondly there are people who want to use clang to build elfutils. clang
>> doesn't support various GNU extensions used in the code like VLAs. So
>> for those people any VLA type seems problematic.
>>
>> > Why not just use VLAs of unions? Cold memory?
>>
>> I am not sure clang supports such VLA types. If it does, then I would say
>> VLAs of unions are preferred above a type that doesn't include bounds.
>>
>> > Given that the current approach (before the patch) already required to
>> write
>> > superfluous "->" perhaps an approach requiring a superfluous "*" will
>> fit?
>> > Like this:
>> >
>> >   void *data = malloc (...);
>> >   T32 (*a32)[n] = data;
>> >   T64 (*a64)[n] = data;
>> >
>> > Then the use looks like "(*a32)[i].member". Clang seems to be happy and
>> its
>> > UBSAN works fine.
>>
>> If that works that would probably be preferred since then ubsan can see
>> the array bounds and help catch issues. You can build and run elfutils
>> and the tests with configure --enable-sanitize-undefined to use ubsan
>> checking.
>>
>> Cheers,
>>
>> Mark
>>
>
>

Attachment: 0006-Do-without-union-of-variable-length-arrays.patch
Description: Binary data

Reply via email to