Hi, I almost forgot about this (rather simple) one. It falls in the category "Mostly Harmless", but it would be nice to have the bitregion_start/end consistent in all cases, even when it does currently not cause any mal-function.
this was:
> Subject: RE: [PATCH, PR 57748] Check for out of bounds access, Part 3
> Date: Wed, 6 Nov 2013 15:40:23 +0100
>
>> Meanwhile I am able to give an example where that code is executed
>> with bitpos = 64, bitsize=32, bitregion_start = 32, bitregion_end = 95.
>>
>> Afterwards bitpos=0, bitsize=32, which is completely outside
>> bitregion_start=32, bitregion_end=95.
>>
>> However this can only be seen in the debugger, as the store_field
>> goes thru a code path that does not look at bitregion_start/end.
>>
>> Well that is at least extremely ugly, and I would not be sure, that
>> I cannot come up with a sample that crashes or creates wrong code.
>>
>> Currently I think that maybe the best way to fix that would be this:
>>
>> --- gcc/expr.c 2013-10-21 08:27:09.546035668 +0200
>> +++ gcc/expr.c 2013-10-22 15:19:56.749476525 +0200
>> @@ -4762,6 +4762,9 @@ expand_assignment (tree to, tree from, b
>> && MEM_ALIGN (to_rtx) == GET_MODE_ALIGNMENT (mode1))
>> {
>> to_rtx = adjust_address (to_rtx, mode1, bitpos / BITS_PER_UNIT);
>> + bitregion_start = 0;
>> + if (bitregion_end>= (unsigned HOST_WIDE_INT) bitpos)
>> + bitregion_end -= bitpos;
>> bitpos = 0;
>> }
>>
>
> well, as already discussed, the following example executes the above memory
> block,
> and leaves bitregion_start/end in an undefined state:
>
extern void abort (void);
struct x{
int a;
int :32;
volatile int b:32;
};
struct s
{
int a,b,c,d;
struct x xx[1];
};
struct s ss;
volatile int k;
int main()
{
ss.xx[k].b = 1;
// asm volatile("":::"memory");
if ( ss.xx[k].b != 1)
abort ();
return 0;
}
> Although this does not cause malfunction at the time, I'd propose to play
> safe,
> and update the bitregion_start/bitregion_end. Additionally I'd propose to
> remove
> this comment in expand_assignment and expand_expr_real_1:
>
> "A constant address in TO_RTX can have VOIDmode, we must not try
> to call force_reg for that case. Avoid that case."
>
> This comment is completely out of sync: There is no longer any force_reg in
> that if-block,
> and a constant address in TO_RTX has SImode or DImode in GET_MODE (XEXP
> (to_rtx, 0))
> I do not know how to make it a VOIDmode, therefore the comment does not help
> either.
>
>
Boot-strapped and regression-tested on x86_64-linux-gnu.
OK for trunk?
Regards
Bernd. 2013-11-06 Bernd Edlinger <bernd.edlin...@hotmail.de>
PR middle-end/57748
* expr.c (expand_assignment): Remove bogus comment.
Update bitregion_start/bitregion_end.
(expand_expr_real_1): Remove bogus comment.
patch-pr57748-3.diff
Description: Binary data
