This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PE-POST] Adjust Bit-region in expand_assignment


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. 		 	   		  

Attachment: changelog-pr57748-3.txt
Description: Text document

Attachment: patch-pr57748-3.diff
Description: Binary data


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]