diff mbox

Fix PR rtl-optimization/60452

Message ID 41050782.FO0TJjpIUH@polaris
State New
Headers show

Commit Message

Eric Botcazou March 23, 2014, 12:02 p.m. UTC 
This is a regression present on mainline and 4.8 branch: ifcvt generates a 
conditional move from an invalid location on the stack, resulting in a 
segfault at runtime.  The testcase is pathological and exploits the very old 
RTL semantics (now embodied in may_trap_or_fault_p) of considering that stack 
references cannot trap, which is of course wrong for nonsensical offsets.

This is an old issue (the attached testcase distilled by Jakub already fails 
with GCC 4.3) and the original testcase is clearly machine-generated, so I 
don't think that we need to pessimize the common case for it; instead fixing 
this kind of very minor issues on a case-by-case basis is good enough I think.

The attached patch only adds a check in rtx_addr_can_trap_p_1 for nonsensical 
offsets against the frame pointer; this is sufficient for both testcases.  The 
check is supposed to be exact (e.g. it never triggers during a bootstrap) so 
it won't pessimize anything.  This might be different if the ??? comment is 
addressed later but, again, I don't think that we should care at this point.

Tested on x86_64-suse-linux, any objections?


2014-03-23  Eric Botcazou  <ebotcazou@adacore.com>

	PR rtl-optimization/60452
	* rtlanal.c (rtx_addr_can_trap_p_1): Fix head comment.
	<case REG>: Return 1 for nonsensical offsets from the frame pointer.


2014-03-23  Eric Botcazou  <ebotcazou@adacore.com>

	* gcc.c-torture/execute/20140323-1.c: New test.

Comments

Richard Biener March 24, 2014, 9:29 a.m. UTC | #1 
On Sun, Mar 23, 2014 at 1:02 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> This is a regression present on mainline and 4.8 branch: ifcvt generates a
> conditional move from an invalid location on the stack, resulting in a
> segfault at runtime.  The testcase is pathological and exploits the very old
> RTL semantics (now embodied in may_trap_or_fault_p) of considering that stack
> references cannot trap, which is of course wrong for nonsensical offsets.
>
> This is an old issue (the attached testcase distilled by Jakub already fails
> with GCC 4.3) and the original testcase is clearly machine-generated, so I
> don't think that we need to pessimize the common case for it; instead fixing
> this kind of very minor issues on a case-by-case basis is good enough I think.
>
> The attached patch only adds a check in rtx_addr_can_trap_p_1 for nonsensical
> offsets against the frame pointer; this is sufficient for both testcases.  The
> check is supposed to be exact (e.g. it never triggers during a bootstrap) so
> it won't pessimize anything.  This might be different if the ??? comment is
> addressed later but, again, I don't think that we should care at this point.
>
> Tested on x86_64-suse-linux, any objections?

Looks reasonable to me.

Richard.

>
> 2014-03-23  Eric Botcazou  <ebotcazou@adacore.com>
>
>         PR rtl-optimization/60452
>         * rtlanal.c (rtx_addr_can_trap_p_1): Fix head comment.
>         <case REG>: Return 1 for nonsensical offsets from the frame pointer.
>
>
> 2014-03-23  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * gcc.c-torture/execute/20140323-1.c: New test.
>
>
> --
> Eric Botcazou
diff mbox

Patch

Index: rtlanal.c
===================================================================
--- rtlanal.c	(revision 208763)
+++ rtlanal.c	(working copy)
@@ -224,10 +224,10 @@  rtx_varies_p (const_rtx x, bool for_alia
   return 0;
 }
 
-/* Return nonzero if the use of X as an address in a MEM can cause a trap.
-   MODE is the mode of the MEM (not that of X) and UNALIGNED_MEMS controls
-   whether nonzero is returned for unaligned memory accesses on strict
-   alignment machines.  */
+/* Return nonzero if the use of X+OFFSET as an address in a MEM with SIZE
+   bytes can cause a trap.  MODE is the mode of the MEM (not that of X) and
+   UNALIGNED_MEMS controls whether nonzero is returned for unaligned memory
+   references on strict alignment machines.  */
 
 static int
 rtx_addr_can_trap_p_1 (const_rtx x, HOST_WIDE_INT offset, HOST_WIDE_INT size,
@@ -235,11 +235,12 @@  rtx_addr_can_trap_p_1 (const_rtx x, HOST
 {
   enum rtx_code code = GET_CODE (x);
 
-  if (STRICT_ALIGNMENT
-      && unaligned_mems
-      && GET_MODE_SIZE (mode) != 0)
+  /* The offset must be a multiple of the mode size if we are considering
+     unaligned memory references on strict alignment machines.  */
+  if (STRICT_ALIGNMENT && unaligned_mems && GET_MODE_SIZE (mode) != 0)
     {
       HOST_WIDE_INT actual_offset = offset;
+
 #ifdef SPARC_STACK_BOUNDARY_HACK
       /* ??? The SPARC port may claim a STACK_BOUNDARY higher than
 	     the real alignment of %sp.  However, when it does this, the
@@ -298,8 +299,27 @@  rtx_addr_can_trap_p_1 (const_rtx x, HOST
       return 0;
 
     case REG:
-      /* As in rtx_varies_p, we have to use the actual rtx, not reg number.  */
-      if (x == frame_pointer_rtx || x == hard_frame_pointer_rtx
+      /* Stack references are assumed not to trap, but we need to deal with
+	 nonsensical offsets.  */
+      if (x == frame_pointer_rtx)
+	{
+	  HOST_WIDE_INT adj_offset = offset - STARTING_FRAME_OFFSET;
+	  if (size == 0)
+	    size = GET_MODE_SIZE (mode);
+	  if (FRAME_GROWS_DOWNWARD)
+	    {
+	      if (adj_offset < frame_offset || adj_offset + size - 1 >= 0)
+		return 1;
+	    }
+	  else
+	    {
+	      if (adj_offset < 0 || adj_offset + size - 1 >= frame_offset)
+		return 1;
+	    }
+	  return 0;
+	}
+      /* ??? Need to add a similar guard for nonsensical offsets.  */
+      if (x == hard_frame_pointer_rtx
 	  || x == stack_pointer_rtx
 	  /* The arg pointer varies if it is not a fixed register.  */
 	  || (x == arg_pointer_rtx && fixed_regs[ARG_POINTER_REGNUM]))
@@ -320,9 +340,7 @@  rtx_addr_can_trap_p_1 (const_rtx x, HOST
       if (XEXP (x, 0) == pic_offset_table_rtx && CONSTANT_P (XEXP (x, 1)))
 	return 0;
 
-      /* - or it is an address that can't trap plus a constant integer,
-	   with the proper remainder modulo the mode size if we are
-	   considering unaligned memory references.  */
+      /* - or it is an address that can't trap plus a constant integer.  */
       if (CONST_INT_P (XEXP (x, 1))
 	  && !rtx_addr_can_trap_p_1 (XEXP (x, 0), offset + INTVAL (XEXP (x, 1)),
 				     size, mode, unaligned_mems))