RISC-V: Don't clobber retval when __builtin_eh_return called.

Message ID 20180604234537.20010-1-jimw@sifive.com
State New
Headers show
Series
  • RISC-V: Don't clobber retval when __builtin_eh_return called.
Related show

Commit Message

Jim Wilson June 4, 2018, 11:45 p.m.
This fixes a problem reported on the RISC-V foundation sw-dev mailing list.  In
a function that calls __builtin_eh_return, such as Unwind_RaiseException, the
return value gets clobbered when we restore the EH_RETURN_DATA_REGNO registers.
The RISC-V port is using arg registers for EH_RETURN_DATA_REGNO that are also
used for the return value.  The implementation was copied from the MIPS port,
but the MIPS port has separate registers for return values and arguments.  This
causes return values to be clobbered for RISC-V code when __builtin_eh_return
is called.

The solution is pulled from the x86 port which also uses return value registers
for EH_RETURN_DATA_REGNO.  A function that calls __builtin_eh_return now has
two epilogues.  On the normal return path, we don't restore the eh data regs
as we don't need them, which avoid clobbering the return value.  On the EH
return path, we do restore the eh data regs as we need them, and we don't need
the return value.

Unfortunately, I don't have a testcase for this, it was found while porting
the D compiler.  But the problem can be seem by disassembling
Unwind_RaiseException.  The code is obviously wrong without the patch, and
looks correct with the patch.

This was tested cross for riscv32-elf and riscv64-linux, and native for
riscv64-linux.  There were no regressions.

Committed.

Jim

	gcc/
	* config/riscv/riscv-protos.h (riscv_expand_epilogue): Change bool arg
	to int.
	* config/riscv/riscv.c (riscv_for_each_saved_reg): New args epilogue
	and maybe_eh_return.  Change regno to unsigned int.  Use new args to
	handle EH_RETURN_DATA_REGNO registers properly.
	(riscv_expand_prologue): Pass new args to riscv_for_each_saved_reg.
	(riscv_expand_epilogue): Update comment.  Change argument name and
	type.  Update code to use new name and type.  Pass new args to
	riscv_for_each_saved_reg.  Only use EH_RETURN_STACKADJ_RTX when
	EXCEPTION_RETURN.
	* config/riscv/riscv.md (NORMAL_RETURN): New.
	(SIBCALL_RETURN, EXCEPTION_RETURN): New.
	(epilogue, sibcall_epilogue): Update riscv_expand_epilogue arg.
	(eh_return): Call gen_eh_return_internal and emit barrier.
	(eh_return_internal): Call riscv_expand_epilogue.
---
 gcc/config/riscv/riscv-protos.h |  2 +-
 gcc/config/riscv/riscv.c        | 52 ++++++++++++++++++++++++---------
 gcc/config/riscv/riscv.md       | 19 ++++++++++--
 3 files changed, 56 insertions(+), 17 deletions(-)

-- 
2.17.0

Patch

diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
index a194b192a2b..f158ed007dd 100644
--- a/gcc/config/riscv/riscv-protos.h
+++ b/gcc/config/riscv/riscv-protos.h
@@ -66,7 +66,7 @@  extern bool riscv_expand_block_move (rtx, rtx, rtx);
 extern rtx riscv_return_addr (int, rtx);
 extern HOST_WIDE_INT riscv_initial_elimination_offset (int, int);
 extern void riscv_expand_prologue (void);
-extern void riscv_expand_epilogue (bool);
+extern void riscv_expand_epilogue (int);
 extern bool riscv_epilogue_uses (unsigned int);
 extern bool riscv_can_use_return_insn (void);
 extern rtx riscv_function_value (const_tree, const_tree, enum machine_mode);
diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index 6e389fa0102..c418dc1ec2e 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -3502,23 +3502,45 @@  riscv_save_restore_reg (machine_mode mode, int regno,
    of the frame.  */
 
 static void
-riscv_for_each_saved_reg (HOST_WIDE_INT sp_offset, riscv_save_restore_fn fn)
+riscv_for_each_saved_reg (HOST_WIDE_INT sp_offset, riscv_save_restore_fn fn,
+			  bool epilogue, bool maybe_eh_return)
 {
   HOST_WIDE_INT offset;
 
   /* Save the link register and s-registers. */
   offset = cfun->machine->frame.gp_sp_offset - sp_offset;
-  for (int regno = GP_REG_FIRST; regno <= GP_REG_LAST; regno++)
+  for (unsigned int regno = GP_REG_FIRST; regno <= GP_REG_LAST; regno++)
     if (BITSET_P (cfun->machine->frame.mask, regno - GP_REG_FIRST))
       {
-	riscv_save_restore_reg (word_mode, regno, offset, fn);
+	bool handle_reg = TRUE;
+
+	/* If this is a normal return in a function that calls the eh_return
+	   builtin, then do not restore the eh return data registers as that
+	   would clobber the return value.  But we do still need to save them
+	   in the prologue, and restore them for an exception return, so we
+	   need special handling here.  */
+	if (epilogue && !maybe_eh_return && crtl->calls_eh_return)
+	  {
+	    unsigned int i, regnum;
+
+	    for (i = 0; (regnum = EH_RETURN_DATA_REGNO (i)) != INVALID_REGNUM;
+		 i++)
+	      if (regno == regnum)
+		{
+		  handle_reg = FALSE;
+		  break;
+		}
+	  }
+
+	if (handle_reg)
+	  riscv_save_restore_reg (word_mode, regno, offset, fn);
 	offset -= UNITS_PER_WORD;
       }
 
   /* This loop must iterate over the same space as its companion in
      riscv_compute_frame_info.  */
   offset = cfun->machine->frame.fp_sp_offset - sp_offset;
-  for (int regno = FP_REG_FIRST; regno <= FP_REG_LAST; regno++)
+  for (unsigned int regno = FP_REG_FIRST; regno <= FP_REG_LAST; regno++)
     if (BITSET_P (cfun->machine->frame.fmask, regno - FP_REG_FIRST))
       {
 	machine_mode mode = TARGET_DOUBLE_FLOAT ? DFmode : SFmode;
@@ -3694,7 +3716,7 @@  riscv_expand_prologue (void)
 			    GEN_INT (-step1));
       RTX_FRAME_RELATED_P (emit_insn (insn)) = 1;
       size -= step1;
-      riscv_for_each_saved_reg (size, riscv_save_reg);
+      riscv_for_each_saved_reg (size, riscv_save_reg, false, false);
     }
 
   frame->mask = mask; /* Undo the above fib.  */
@@ -3756,11 +3778,11 @@  riscv_adjust_libcall_cfi_epilogue ()
   return dwarf;
 }
 
-/* Expand an "epilogue" or "sibcall_epilogue" pattern; SIBCALL_P
-   says which.  */
+/* Expand an "epilogue", "sibcall_epilogue", or "eh_return_internal" pattern;
+   style says which.  */
 
 void
-riscv_expand_epilogue (bool sibcall_p)
+riscv_expand_epilogue (int style)
 {
   /* Split the frame into two.  STEP1 is the amount of stack we should
      deallocate before restoring the registers.  STEP2 is the amount we
@@ -3771,7 +3793,8 @@  riscv_expand_epilogue (bool sibcall_p)
   unsigned mask = frame->mask;
   HOST_WIDE_INT step1 = frame->total_size;
   HOST_WIDE_INT step2 = 0;
-  bool use_restore_libcall = !sibcall_p && riscv_use_save_libcall (frame);
+  bool use_restore_libcall = ((style == NORMAL_RETURN)
+			      && riscv_use_save_libcall (frame));
   rtx ra = gen_rtx_REG (Pmode, RETURN_ADDR_REGNUM);
   rtx insn;
 
@@ -3781,14 +3804,14 @@  riscv_expand_epilogue (bool sibcall_p)
 
   if (cfun->machine->naked_p)
     {
-      gcc_assert (!sibcall_p);
+      gcc_assert (style == NORMAL_RETURN);
 
       emit_jump_insn (gen_return ());
 
       return;
     }
 
-  if (!sibcall_p && riscv_can_use_return_insn ())
+  if ((style == NORMAL_RETURN) && riscv_can_use_return_insn ())
     {
       emit_jump_insn (gen_return ());
       return;
@@ -3863,7 +3886,8 @@  riscv_expand_epilogue (bool sibcall_p)
     frame->mask = 0; /* Temporarily fib that we need not save GPRs.  */
 
   /* Restore the registers.  */
-  riscv_for_each_saved_reg (frame->total_size - step2, riscv_restore_reg);
+  riscv_for_each_saved_reg (frame->total_size - step2, riscv_restore_reg,
+			    true, style == EXCEPTION_RETURN);
 
   if (use_restore_libcall)
     {
@@ -3902,14 +3926,14 @@  riscv_expand_epilogue (bool sibcall_p)
     }
 
   /* Add in the __builtin_eh_return stack adjustment. */
-  if (crtl->calls_eh_return)
+  if ((style == EXCEPTION_RETURN) && crtl->calls_eh_return)
     emit_insn (gen_add3_insn (stack_pointer_rtx, stack_pointer_rtx,
 			      EH_RETURN_STACKADJ_RTX));
 
   /* Return from interrupt.  */
   if (cfun->machine->interrupt_handler_p)
     emit_insn (gen_riscv_mret ());
-  else if (!sibcall_p)
+  else if (style != SIBCALL_RETURN)
     emit_jump_insn (gen_simple_return_internal (ra));
 }
 
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index fa681971c4c..b9faf00d076 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -73,6 +73,10 @@ 
    (S0_REGNUM			8)
    (S1_REGNUM			9)
    (S2_REGNUM			18)
+
+   (NORMAL_RETURN		0)
+   (SIBCALL_RETURN		1)
+   (EXCEPTION_RETURN		2)
 ])
 
 (include "predicates.md")
@@ -2036,7 +2040,7 @@ 
   [(const_int 2)]
   ""
 {
-  riscv_expand_epilogue (false);
+  riscv_expand_epilogue (NORMAL_RETURN);
   DONE;
 })
 
@@ -2044,7 +2048,7 @@ 
   [(const_int 2)]
   ""
 {
-  riscv_expand_epilogue (true);
+  riscv_expand_epilogue (SIBCALL_RETURN);
   DONE;
 })
 
@@ -2086,6 +2090,9 @@ 
     emit_insn (gen_eh_set_lr_di (operands[0]));
   else
     emit_insn (gen_eh_set_lr_si (operands[0]));
+
+  emit_jump_insn (gen_eh_return_internal ());
+  emit_barrier ();
   DONE;
 })
 
@@ -2114,6 +2121,14 @@ 
   DONE;
 })
 
+(define_insn_and_split "eh_return_internal"
+  [(eh_return)]
+  ""
+  "#"
+  "epilogue_completed"
+  [(const_int 0)]
+  "riscv_expand_epilogue (EXCEPTION_RETURN); DONE;")
+
 ;;
 ;;  ....................
 ;;