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

On Tue, Dec 10, 2013 at 4:02 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Tue, Dec 10, 2013 at 11:53 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>> What we support is out of bounds accesses for heap vars if the var's type
>>> has flexible array member or something we treat similarly and there is the
>>> possibility that there could be payload after the heap var that could be
>>> accessed from the flexible array members or similar arrays.
>>
>> My question was about the above similar arrays, i.e. whether we consider all
>> trailing arrays in structures as flexible-like or not.  No strong opinion.
>
> Yes we do, even for struct { struct { int a; char a[1] } }; (note the not really
> "trailing" as there is padding after the trailing array).  We do take
> size limitations from a DECL (if we see one) into account to limit the
> effect of this trailing-array-supporting, so it effectively only applies to
> indirect accesses (and the padding example above, you can use the whole
> padding if DECL_SIZE allows that).
>
>>> So, I don't see what is the big deal with BLKmode, because all the cases
>>> which actually could have flexible array member extra payloads (or similar)
>>> must necessarily live in memory, if it is the compiler that decides whether
>>> to put it into memory or keep in registers etc., then it can't be heap
>>> allocated.
>>
>> The invariant is that types for which objects can effectively have variable
>> size must have BLKmode, otherwise you need to add very ugly code in the RTL
>> expander to mask the lie.
>
> I wonder if we can make the expander more rely on the DECLs mode
> and optimize only the DECLs mode (if we can constrain its size via
> DECL_SIZE) to non-BLKmode instead of doing that for the TYPEs mode.
> Yes, you'd have DECL_MODE != TYPE_MODE that way.
>
> Or rather I wonder if the expander doesn't already work that way
> (looks at DECL_MODE).

To speak in patches (completely untested) - sth like the following ontop
of making more types have BLKmode:



> Richard.
>
>> --
>> Eric Botcazou

Hi,

On Tue, 10 Dec 2013 16:14:43, Richard Biener wrote:
>
> On Tue, Dec 10, 2013 at 4:02 PM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On Tue, Dec 10, 2013 at 11:53 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>>> What we support is out of bounds accesses for heap vars if the var's type
>>>> has flexible array member or something we treat similarly and there is the
>>>> possibility that there could be payload after the heap var that could be
>>>> accessed from the flexible array members or similar arrays.
>>>
>>> My question was about the above similar arrays, i.e. whether we consider all
>>> trailing arrays in structures as flexible-like or not. No strong opinion.
>>
>> Yes we do, even for struct { struct { int a; char a[1] } }; (note the not really
>> "trailing" as there is padding after the trailing array). We do take
>> size limitations from a DECL (if we see one) into account to limit the
>> effect of this trailing-array-supporting, so it effectively only applies to
>> indirect accesses (and the padding example above, you can use the whole
>> padding if DECL_SIZE allows that).
>>
>>>> So, I don't see what is the big deal with BLKmode, because all the cases
>>>> which actually could have flexible array member extra payloads (or similar)
>>>> must necessarily live in memory, if it is the compiler that decides whether
>>>> to put it into memory or keep in registers etc., then it can't be heap
>>>> allocated.
>>>
>>> The invariant is that types for which objects can effectively have variable
>>> size must have BLKmode, otherwise you need to add very ugly code in the RTL
>>> expander to mask the lie.
>>
>> I wonder if we can make the expander more rely on the DECLs mode
>> and optimize only the DECLs mode (if we can constrain its size via
>> DECL_SIZE) to non-BLKmode instead of doing that for the TYPEs mode.
>> Yes, you'd have DECL_MODE != TYPE_MODE that way.
>>
>> Or rather I wonder if the expander doesn't already work that way
>> (looks at DECL_MODE).
>
> To speak in patches (completely untested) - sth like the following ontop
> of making more types have BLKmode:
>
> Index: gcc/stor-layout.c
> ===================================================================
> --- gcc/stor-layout.c (revision 205857)
> +++ gcc/stor-layout.c (working copy)
> @@ -569,8 +569,17 @@ layout_decl (tree decl, unsigned int kno
> bitsize_unit_node));
>
> if (code != FIELD_DECL)
> - /* For non-fields, update the alignment from the type. */
> - do_type_align (type, decl);
> + {
> + /* For non-fields, update the alignment from the type. */
> + do_type_align (type, decl);
> + /* Optimize the mode of the decl.
> + ??? Ensure choosen mode alignment fits decl alignment. */
> + if (DECL_MODE (decl) == BLKmode
> + && DECL_SIZE (decl)
> + && compare_tree_int (DECL_SIZE (t), MAX_FIXED_MODE_SIZE) <= 0)
> + DECL_MODE (decl)
> + = mode_for_size (TREE_INT_CST_LOW (DECL_SIZE (decl)), MODE_INT, 1);
> + }
> else
> /* For fields, it's a bit more complicated... */
> {
>
>
>> Richard.
>>
>>> --
>>> Eric Botcazou

Whatever the fix will be, it should contain at least the two test cases from
my patch, and, maybe - if that is possible - it would be nice to test it with
with SLOW_UNALIGNED_ACCESS defined like this:

config/i386/i386.h:
#define SLOW_UNALIGNED_ACCESS(MODE, ALIGN) 1


Bernd.