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] |
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] |