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
