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

[Alexander Cherepanov <ch3root at openwall dot com>]

On 2015-10-05 21:45, Mark Wielaard 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).

Great.

> Secondly there are people who want to use clang to build elfutils.

Yes, this nice to have.

> clang
> doesn't support various GNU extensions used in the code like VLAs. So
> for those people any VLA type seems problematic.

VLAs are in C99 (but optional in C11) and are supported by clang[1]. But 
VLAs inside structs/unions are prohibited by the standards, are a gcc 
extension and are not supported by clang.

[1] http://clang.llvm.org/compatibility.html#vla

>> Why not just use VLAs of unions? Cold memory?
>
> I am not sure clang supports such VLA types.

Yes, it should work.

> If it does, then I would say
> VLAs of unions are preferred above a type that doesn't include bounds.

In comparison with a pointer-to-array (classical Pascal) approach it has 
pros and cons.

Pros:
- shorter declaration (no malloc, no calculations).
Cons:
- bounded arrays only;
- requires more memory (for 32-bit case);
- non-continuous memory (for 32-bit case).

Not sure if these cons are important.

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

I believe it was not working in gcc 4.9. It works in newer gcc?

> You can build and run elfutils
> and the tests with configure --enable-sanitize-undefined to use ubsan
> checking.

Nice.

-- 
Alexander Cherepanov