[REPOST] Invalid Code when reading from unaligned zero-sized array

Richard Biener richard.guenther@gmail.com
Fri Dec 13 11:17:00 GMT 2013


On Fri, Dec 13, 2013 at 12:08 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> Instead I'd suggest to keep a 'last_field_array_p' flag that you can
>> check at the end of the loop.
>
> OK, I can do that.
>
>> +      && TREE_CODE (TREE_TYPE (field)) == ARRAY_TYPE
>> +      && !integer_zerop (TYPE_SIZE (TREE_TYPE (TREE_TYPE (field)))))
>> +    return;
>> +
>>
>> Why does this exclude zero-sized element types?  That looks odd to me ;)
>
> Because the size cannot change if you add zero to it, even multiple times?

Of course - I wonder if you hit a testcase in the testsuite.

>> Btw, the loop already has
>>
>>       if (TREE_CODE (TREE_TYPE (field)) == ERROR_MARK
>>
>>           || (TYPE_MODE (TREE_TYPE (field)) == BLKmode
>>
>>               && ! TYPE_NO_FORCE_BLK (TREE_TYPE (field))
>>               && !(TYPE_SIZE (TREE_TYPE (field)) != 0
>>                    && integer_zerop (TYPE_SIZE (TREE_TYPE (field)))))
>>
>>           || ! tree_fits_uhwi_p (bit_position (field))
>>           || DECL_SIZE (field) == 0
>>           || ! tree_fits_uhwi_p (DECL_SIZE (field)))
>>
>>         return;
>>
>> which then is supposed to
>> handle struct { struct { char c[8]; } a; } - but it seems to
>> special-case zero-sized
>> members again, thus struct { struct { char c[0]; } a; } would still be
>> broken after your patch?
>
> Probably indeed, this test was added by the same patch that added the 0-sized
> bitfield test in the RTL expander
>    http://gcc.gnu.org/ml/gcc-patches/2003-10/msg00823.html
> and that I think is obsolete now.  I can remove it.

Ok with me.

>> The issue Bernd raises is real as well, though we probably should fix this
>> in a different way by using a different DECL_MODE based on the types
>> size for asm register vars?
>
> Really?  The number of open PRs for register variables not behaving properly
> at run time would make me think that this wouldn't be necessary bad...

Yeah ... we can wait and see if anybody notices.  (especially register
vars that you access out-of-bounds behave erratically ...)

>> Btw, do you think we can recover from some of the now BLKmodes by
>> having DECL_MODE != TYPE_MODE?
>
> Well, if you want to do that, the straightforward solution is to keep the non-
> BLKmode for the TYPE, put a flag on it and treat it as BLKmode when needed.

That sounds less conservative though.  Anyway, that was just some thought
to fix the eventual fallout of making more types have BLKmode.

Thanks,
Richard.

> --
> Eric Botcazou



More information about the Gcc-patches mailing list