RE: [PATCH, PR 57748] Check for out of bounds access

[Bernd Edlinger <bernd dot edlinger at hotmail dot de>]

Just for completeness, these were the test examples on
this private communication:

On Fri, 6 Sep 2013 11:19:18, Richard Biener wrote:
> On Fri, Sep 6, 2013 at 10:35 AM, Bernd Edlinger
> <bernd.edlinger@hotmail.de> wrote:
>> Richard,
>>
>>> But the movmisalign path skips all this code and with the
>>> current code thinks the actual access is in the mode of the
>>> whole structure. (and also misses the address adjustment
>>> as shown in the other testcase for the bug)
>>>
>>> The movmisalign handling in this path is just broken. And
>>> it's _not_ just for optimization! If you have
>>>
>>> struct __attribute__((packed)) {
>>> double x;
>>> v2df y;
>>> } *p;
>>>
>>> and do
>>>
>>> p = malloc (48); // gets you 8-byte aligned memory
>>> p->y = (v2df) { 1., 2. };
>>>
>>> then you cannot skip the movmisaling handling because
>>> we'd generate an aligned move (based on the mode) then.
>>>
>>
>> Ok, test examples are really helpful here.
>>
>> This time the structure is BLKmode, unaligned,
>> movmisalign = false anyway.
>>
>> I tried to make a test case out of your example,
>> and as I expected, the code is also correct:
>>
>> foo:
>> .cfi_startproc
>> movdqa .LC1(%rip), %xmm0
>> movq $-1, (%rdi)
>> movl $0x4048f5c3, 8(%rdi)
>> movdqu %xmm0, 12(%rdi)
>> ret
>>
>> movdqu.
>>
>> The test executes without trap.
>> And I did everything to make the object unaligned.
>
> Yeah, well - for the expand path with BLKmode it's working fine.
> We're doing all
> the dances because of correctness for non-BLKmode expand paths.
>
>> I am sure we could completely remove the
>> movmisalign path, and nothing would happen.
>> probably. except maybe for a performance regression.
>
> In this place probably yes. But we can also fix it to use the proper mode and
> properly do the offset adjustment. But just adding a bounds check looks
> broken to me.
>
> Note that there may have been a correctness reason for this code in the
> light of the IPA-SRA code. Maybe Martin remembers.
>
> If removing the movmisalign handling inside the handled-component
> case in expand_assignment works out (make sure to also test a
> strict-alignment target) then that is probably fine.
>
> Richard.
>
>>
>> Bernd.

Attachment: x.c
Description: Text document

Attachment: y.c
Description: Text document