diff mbox

[RS6000] asynch exceptions and unwind info

Message ID 20110729012748.GR1081@bubble.grove.modra.org
State New
Headers show

Commit Message

Alan Modra July 29, 2011, 1:27 a.m. UTC 
On Thu, Jul 28, 2011 at 11:49:16AM -0700, Richard Henderson wrote:
> On 07/28/2011 12:27 AM, Alan Modra wrote:
> > On Wed, Jul 27, 2011 at 03:00:45PM +0930, Alan Modra wrote:
> >> Ideally what I'd like to
> >> do is have ld and gcc emit accurate r2 tracking unwind info and
> >> dispense with hacks like frob_update_context.  If ld did emit accurate
> >> unwind info for .glink, then the justification for frob_update_context
> >> disappears.
> > 
> > For the record, this statement of mine doesn't make sense.  A .glink
> > stub doesn't make a frame, so a backtrace won't normally pass through a
> > stub, thus having accurate unwind info for .glink doesn't help at all.
> 
> It does, for the duration of the stub.

Right, but I was talking about the normal case, where the unwinder
won't even look at .glink unwind info.

> The whole problem is that toc pointer copy in 40(1) is only valid
> during indirect call sequences, and iff ld inserted a stub?  I.e.
> direct calls between functions that share toc pointers never save
> the copy?

Yes.

> Would it make sense, if a function has any indirect call, to move
> the toc pointer save into the prologue?  You'd get to avoid that
> store all the time.  Of course you'd not be able to sink the load
> after the call, but it might still be a win.  And in that special
> case you can annotate the r2 save slot just once, correctly.

Except that any info about r2 in an indirect call sequence really
belongs to the *called* function frame, not the callee.  I woke up
this morning with the realization that what I'd done in
frob_update_context for indirect call sequences was wrong.  Ditto for
the r2 store that Michael moved into the prologue.  The only time we
want the unwinder to restore from that particular save is if r2 isn't
saved in the current frame.

Untested patch follows.

libgcc/
	* config/rs6000/linux-unwind.h (frob_update_context <__powerpc64__>):
	Restore for indirect call bcrtl from correct stack slot, and only
	if cfa+40 isn't valid.
gcc/
	* config/rs6000/rs6000-protos.h (rs6000_save_toc_in_prologue_p): Delete.
	* config/rs6000/rs6000.c (rs6000_save_toc_in_prologue_p): Make static.
	(rs6000_emit_prologue): Don't emit eh_frame info in
	save_toc_in_prologue case.
	(rs6000_call_indirect_aix): Formatting.

Comments

David Edelsohn July 29, 2011, 1:16 p.m. UTC | #1 
On Thu, Jul 28, 2011 at 9:27 PM, Alan Modra <amodra@gmail.com> wrote:

> Right, but I was talking about the normal case, where the unwinder
> won't even look at .glink unwind info.
>
>> The whole problem is that toc pointer copy in 40(1) is only valid
>> during indirect call sequences, and iff ld inserted a stub?  I.e.
>> direct calls between functions that share toc pointers never save
>> the copy?
>
> Yes.
>
>> Would it make sense, if a function has any indirect call, to move
>> the toc pointer save into the prologue?  You'd get to avoid that
>> store all the time.  Of course you'd not be able to sink the load
>> after the call, but it might still be a win.  And in that special
>> case you can annotate the r2 save slot just once, correctly.
>
> Except that any info about r2 in an indirect call sequence really
> belongs to the *called* function frame, not the callee.  I woke up
> this morning with the realization that what I'd done in
> frob_update_context for indirect call sequences was wrong.  Ditto for
> the r2 store that Michael moved into the prologue.  The only time we
> want the unwinder to restore from that particular save is if r2 isn't
> saved in the current frame.

This discussion seems to be referencing both PLT stubs and pointer
glue.  Indirect calls through a function pointer create a frame, save
R2, and the unwinder can visit that frame.  PLT stub calls are tail
calls, save R2, and the unwinder only would visit the frame if an
exception occurs in the middle of a call.  One also can add lazy
resolution using the glink code, which performs additional work in the
dynamic linker on the first call.

Which has the problem?  Which are you trying to solve?  And how is
your change solving it?

Thanks, David
Alan Modra July 29, 2011, 1:29 p.m. UTC | #2 
On Fri, Jul 29, 2011 at 09:16:09AM -0400, David Edelsohn wrote:
> Which has the problem?  Which are you trying to solve?  And how is
> your change solving it?

Michael's save_toc_in_prologue emit_frame_save writes unwind info for
the wrong frame.  That r2 save is the current r2.  What we need is
info about the previous r2, so we can restore when unwinding.

I made a similar mistake in frob_update_context in that the value
saved by an indirect function call sequence is the r2 for the current
function.  I also restored from the wrong location.
diff mbox

Patch

Index: libgcc/config/rs6000/linux-unwind.h
===================================================================
--- libgcc/config/rs6000/linux-unwind.h	(revision 176905)
+++ libgcc/config/rs6000/linux-unwind.h	(working copy)
@@ -354,20 +354,22 @@  frob_update_context (struct _Unwind_Cont
 	  /* We are in a plt call stub or r2 adjusting long branch stub,
 	     before r2 has been saved.  Keep REG_UNSAVED.  */
 	}
-      else if (pc[0] == 0x4E800421
-	       && pc[1] == 0xE8410028)
-	{
-	  /* We are at the bctrl instruction in a call via function
-	     pointer.  gcc always emits the load of the new r2 just
-	     before the bctrl.  */
-	  _Unwind_SetGRPtr (context, 2, context->cfa + 40);
-	}
       else
 	{
 	  unsigned int *insn
 	    = (unsigned int *) _Unwind_GetGR (context, R_LR);
 	  if (insn && *insn == 0xE8410028)
 	    _Unwind_SetGRPtr (context, 2, context->cfa + 40);
+	  else if (pc[0] == 0x4E800421
+		   && pc[1] == 0xE8410028)
+	    {
+	      /* We are at the bctrl instruction in a call via function
+		 pointer.  gcc always emits the load of the new R2 just
+		 before the bctrl so this is the first and only place
+		 we need to use the stored R2.  */
+	      _Unwind_Word sp = _Unwind_GetGR (context, 1);
+	      _Unwind_SetGRPtr (context, 2, sp + 40);
+	    }
 	}
     }
 #endif
Index: gcc/config/rs6000/rs6000-protos.h
===================================================================
--- gcc/config/rs6000/rs6000-protos.h	(revision 176905)
+++ gcc/config/rs6000/rs6000-protos.h	(working copy)
@@ -172,8 +172,6 @@  extern void rs6000_emit_epilogue (int);
 extern void rs6000_emit_eh_reg_restore (rtx, rtx);
 extern const char * output_isel (rtx *);
 extern void rs6000_call_indirect_aix (rtx, rtx, rtx);
-extern bool rs6000_save_toc_in_prologue_p (void);
-
 extern void rs6000_aix_asm_output_dwarf_table_ref (char *);
 
 /* Declare functions in rs6000-c.c */
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 176905)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -1178,6 +1178,7 @@  static void rs6000_conditional_register_
 static void rs6000_trampoline_init (rtx, tree, rtx);
 static bool rs6000_cannot_force_const_mem (enum machine_mode, rtx);
 static bool rs6000_legitimate_constant_p (enum machine_mode, rtx);
+static bool rs6000_save_toc_in_prologue_p (void);
 
 /* Hash table stuff for keeping track of TOC entries.  */
 
@@ -20536,8 +20562,11 @@  rs6000_emit_prologue (void)
 
   /* If we need to, save the TOC register after doing the stack setup.  */
   if (rs6000_save_toc_in_prologue_p ())
-    emit_frame_save (sp_reg_rtx, sp_reg_rtx, reg_mode, TOC_REGNUM,
-		     5 * reg_size, info->total_size);
+    {
+      rtx addr = gen_rtx_PLUS (Pmode, sp_reg_rtx, GEN_INT (5 * reg_size));
+      rtx mem = gen_frame_mem (reg_mode, addr);
+      emit_move_insn (mem, gen_rtx_REG (reg_mode, TOC_REGNUM));
+    }
 }
 
 /* Write function prologue.  */
@@ -27798,7 +27829,7 @@  rs6000_call_indirect_aix (rtx value, rtx
   if (TARGET_SAVE_TOC_INDIRECT && !cfun->calls_alloca
       && !cfun->calls_setjmp && !cfun->has_nonlocal_label
       && !cfun->can_throw_non_call_exceptions
-      && ((flags_from_decl_or_type (cfun->decl) & ECF_NOTHROW) == ECF_NOTHROW))
+      && (flags_from_decl_or_type (cfun->decl) & ECF_NOTHROW) == ECF_NOTHROW)
     cfun->machine->save_toc_in_prologue = true;
 
   else
@@ -27834,13 +27865,12 @@  rs6000_call_indirect_aix (rtx value, rtx
     insn = call_func (func_addr, flag, func_toc_mem, stack_toc_mem);
 
   emit_call_insn (insn);
-  return;
 }
 
 /* Return whether we need to always update the saved TOC pointer when we update
    the stack pointer.  */
 
-bool
+static bool
 rs6000_save_toc_in_prologue_p (void)
 {
   return (cfun && cfun->machine && cfun->machine->save_toc_in_prologue);