Re: [PATCH] Do without union of variable length arrays.

[Chih-Hung Hsieh <chh at google dot com>]

On Thu, Oct 1, 2015 at 2:10 PM, Alexander Cherepanov <ch3root@openwall.com>
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?
>
> Why not just use VLAs of unions? Cold memory?
>

I think this does not work because some part of elfutils walks through an
array of T32 (or an array of T64), not an array of union of T32 and T64.


>
> 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.


This looks fine to me. I don't mind either way.
Anyone else prefers a32[i]  or (*a32)[i]?

-- chh



>
>
> --
> Alexander Cherepanov
>

On Thu, Oct 1, 2015 at 2:10 PM, Alexander Cherepanov <ch3root@openwall.com> 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?

Why not just use VLAs of unions? Cold memory?

I think this does not work because some part of elfutils walks through an array of T32 (or an array of T64), not an array of union of T32 and T64.

 

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 = "" (...);
  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.

This looks fine to me. I don't mind either way.

Anyone else prefers a32[i]  or (*a32)[i]?

-- chh

 

--
Alexander Cherepanov