> Hi,
>
> On Tue, 17 Sep 2013 01:09:45, Martin Jambor wrote:
>>> @@ -4773,6 +4738,8 @@ expand_assignment (tree to, tree from, b
>>> if (MEM_P (to_rtx)
>>> && GET_MODE (to_rtx) == BLKmode
>>> && GET_MODE (XEXP (to_rtx, 0)) != VOIDmode
>>> + && bitregion_start == 0
>>> + && bitregion_end == 0
>>> && bitsize> 0
>>> && (bitpos % bitsize) == 0
>>> && (bitsize % GET_MODE_ALIGNMENT (mode1)) == 0
>>>
>> ...
>>
>> I'm not sure to what extent the hunk adding tests for bitregion_start
>> and bitregion_end being zero is connected to this issue. I do not see
>> any of the testcases exercising that path. If it is indeed another
>> problem, I think it should be submitted (and potentially committed) as
>> a separate patch, preferably with a testcase.
>>
>
> 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;
> }
>
> Any suggestions?
>
>
>
> Regards
> Bernd.
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
