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.