[PATCH/expand] PR64011 Adjust bitsize when partial overflow happen for big-endian

[Jiong Wang]

On 13/01/15 21:45, Jeff Law wrote:
> On 01/09/15 06:39, Jiong Wang wrote:
>
>> the bug testcase is
>> ===================
>>
>> typedef short U __attribute__((may_alias, aligned (1)));
>> struct S
>> {
>>     _Complex float d __attribute__((aligned (8)));
>> };
>> void bar(struct S);
>> void f5 (int x)
>> {
>>     struct S s;
>>     ((U *)((char *) &s.d + 1))[3] = x;
>>     bar (s);
>> }
> So I'm going to focus on that assignment statement.  Doesn't that write
> outside the bounds of "s"?    If I'm reading everything correctly we
> have an object that is 8 bytes (a complex float).  The assignment is a 2
> byte write starting at byte 7 in the object.  ISTM that writes one byte
> beyond the underlying object, at which point we're in undefined
> behaviour territory.
>
> In many ways having the compiler or assembler spitting out an error here
> is preferable to silently compiling the code.  That would also help
> explain why we haven't seen this on other big endian targets with rich
> bitfield support (PA and H8 come to mind) -- it only arises in cases of
> undefined behaviour AFAICT.
>
> What I do not like is the ICE or unrecognizable insn error.

currently, if a backend define "insv" with strict operand constraints to reject
negative imm, for example ARM, will ICE as unrecognizable error.

> I'm really tempted here to use the conditional you want to add to
> store_bit_field_using_insv and when it triggers issue an error/warning
> instead of or in addition to truncating the size of the assignment.

agree, and I think the truncation is needed otherwise there may have ICE on some target.

and I found current gcc LOCATION info is very good !
have done an experimental hack on at "expand_assignment": 4931 where the tree is expanded,
gcc could give quite useful & accurate warning based on tree LOCATION info.

./cc1 -O2 -mbig-endian pr48335-2.c

pr48335-2.c: In function ‘f5’:
pr48335-2.c:19:29: warning: overflow here !
    ((U *)((char *) &s.d + 1))[3] = x;
                              ^

while we need to add warning at store_bit_field_using_insv where
there is no accurate LOCATION info. but looks like it's acceptable?

pr48335-2.c:19:33: warning: overflow here !
    ((U *)((char *) &s.d + 1))[3] = x;
                                  ^

>
> Thoughts?
>
> jeff
>
>
>
>
>
>