On 01/15/15 09:27, Jiong Wang wrote:

On 15/01/15 03:51, Jeff Law wrote:
On 01/14/15 15:31, Jiong Wang wrote:
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;
                                   ^
Yes, I think we're on the right track now -- warn and truncate the the
insertion.

I just scanned our set of warning flags to see if this would fit nicely
under any of the existing flags, and it doesn't.  I guess putting it
under -Wextra is probably best for now.

I think the warning text should indicate that the statement will write
outside the bounds of the destination object or something along those
lines.

thanks for suggestions. patch updated

the warning message is as the following:

./cc1 -O2 pr48335-2.c -Wextra

pr48335-2.c: In function ‘f5’:
pr48335-2.c:19:33: warning: write of 16bit data outside the bound of
destination object, data truncated into 8bit [-Wextra]
    ((U *)((char *) &s.d + 1))[3] = x;
                                  ^
x86-64 bootstrap & regress ok.

ok for trunk?

one other thing is I found using of "insv" maybe sub-optimal in some
situation.
for example, even before my patch, modify type of U to char so there is
no overflow.

use BFI will cost one more fmov (cc1 -O2 test.c), maybe this is caused
by bfi only
support integer type, which may cause extra reg copy when there are both
float & int type.

// comment out the "if (maybe_expand_insn" in store_bit_field_using_insv
// to fall back to other code path
f5:
         uxtb    w0, w0
         stp     x29, x30, [sp, -16]!
         fmov    s0, wzr
         fmov    s1, w0
         add     x29, sp, 0
         bl      bar
         ldp     x29, x30, [sp], 16
         ret

// BFI version
f5:
         fmov    s0, wzr
         stp     x29, x30, [sp, -16]!
         add     x29, sp, 0
         fmov    w1, s0
         bfi     w1, w0, 0, 8
         fmov    s1, w1
         bl      bar
         ldp     x29, x30, [sp], 16
         ret


Thanks.

gcc/
     PR64011
     * expmed.c (store_bit_field_using_insv): Warn and truncate bitsize
when there is partial overflow.
OK for the trunk.   Please install.

Thanks,
Jeff

Reply via email to