[RFC/hppa] Change handling of stubs in the return path

[RFC/hppa] Change handling of stubs in the return path

[Randolph Chung]

The following patch changes the behavior of the unwinder for hpux so
that export stubs are not visible when unwinding the stack. this is
desired for several reasons as discussed on this list recently:

1. it is less confusing to the user (and more consistent with how 
   gdb used to work)
2. it improves the behavior for commands like "up" and "finish", 
   because the user expects to return to the caller of the current
   function, not to a stub that was synthesized by the toolchain

This fixes all the FAILs in funcargs.exp and some of the ones in
callfuncs.exp (and possibly others, i haven't ran the whole test)

Note that the stub unwinder is still there for the cases when we single
step into a stub.  There are some issues with the stub unwinder for HPUX
as well, but I'll fix that separately.

comments? is this ok?

randolph




2004-12-06  Randolph Chung  <tausq@debian.org>

    * hppa-tdep.h (gdbarch_tdep): Add unwind_adjust_stub method.
    * hppa-hpux-tdep.c (hppa_hpux_unwind_adjust_stub): New function.
    (hppa_hpux_init_abi) Set unwind_adjust_stub method.
    * hppa-tdep.c (hppa_frame_cache): Call unwind_adjust_stub method
    if defined.

Index: hppa-hpux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-hpux-tdep.c,v
retrieving revision 1.27
diff -u -p -r1.27 hppa-hpux-tdep.c
--- hppa-hpux-tdep.c	5 Dec 2004 15:48:14 -0000	1.27
+++ hppa-hpux-tdep.c	6 Dec 2004 22:26:25 -0000
@@ -1425,6 +1425,44 @@ hppa_hpux_inferior_created (struct targe
   hp_cxx_exception_support_initialized = 0;
 }
 
+/* Given the current value of the pc, check to see if it is inside a stub, and
+   if so, change the value of the pc to point to the caller of the stub.
+   NEXT_FRAME is the next frame in the current list of frames.
+   BASE contains to stack frame base of the current frame. 
+   SAVE_REGS is the register file stored in the frame cache. */
+static void
+hppa_hpux_unwind_adjust_stub(struct frame_info *next_frame, CORE_ADDR base,
+			     struct trad_frame_saved_reg *saved_regs)
+{
+  int optimized, realreg;
+  enum lval_type lval;
+  CORE_ADDR addr;
+  char buffer[sizeof(ULONGEST)];
+  ULONGEST val;
+  CORE_ADDR stubpc;
+  struct unwind_table_entry *u;
+
+  trad_frame_get_prev_register (next_frame, saved_regs, 
+				HPPA_PCOQ_HEAD_REGNUM, 
+				&optimized, &lval, &addr, &realreg, buffer);
+  val = extract_unsigned_integer (buffer, 
+				  register_size (get_frame_arch (next_frame), 
+      				  		 HPPA_PCOQ_HEAD_REGNUM));
+
+  u = find_unwind_entry (val);
+  if (u && u->stub_unwind.stub_type == EXPORT)
+    {
+      stubpc = read_memory_integer (base - 24, TARGET_PTR_BIT / 8);
+      trad_frame_set_value (saved_regs, HPPA_PCOQ_HEAD_REGNUM, stubpc);
+    }
+  else if (hppa_symbol_address ("__gcc_plt_call") 
+           == get_pc_function_start (val))
+    {
+      stubpc = read_memory_integer (base - 8, TARGET_PTR_BIT / 8);
+      trad_frame_set_value (saved_regs, HPPA_PCOQ_HEAD_REGNUM, stubpc);
+    }
+}
+
 static void
 hppa_hpux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -1435,6 +1473,8 @@ hppa_hpux_init_abi (struct gdbarch_info 
   else
     tdep->in_solib_call_trampoline = hppa64_hpux_in_solib_call_trampoline;
 
+  tdep->unwind_adjust_stub = hppa_hpux_unwind_adjust_stub;
+
   set_gdbarch_in_solib_return_trampoline (gdbarch,
 					  hppa_hpux_in_solib_return_trampoline);
   set_gdbarch_skip_trampoline_code (gdbarch, hppa_hpux_skip_trampoline_code);
Index: hppa-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.c,v
retrieving revision 1.185
diff -u -p -r1.185 hppa-tdep.c
--- hppa-tdep.c	6 Dec 2004 03:32:26 -0000	1.185
+++ hppa-tdep.c	6 Dec 2004 22:26:26 -0000
@@ -1928,6 +1928,19 @@ hppa_frame_cache (struct frame_info *nex
       }
   }
 
+  {
+    struct gdbarch *gdbarch;
+    struct gdbarch_tdep *tdep;
+
+    gdbarch = get_frame_arch (next_frame);
+    tdep = gdbarch_tdep (gdbarch);
+
+    if (tdep->unwind_adjust_stub)
+      {
+        tdep->unwind_adjust_stub (next_frame, cache->base, cache->saved_regs);
+      }
+  }
+
   if (hppa_debug)
     fprintf_unfiltered (gdb_stdlog, "base=0x%s }", 
       paddr_nz (((struct hppa_frame_cache *)*this_cache)->base));
Index: hppa-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.h,v
retrieving revision 1.14
diff -u -p -r1.14 hppa-tdep.h
--- hppa-tdep.h	31 Oct 2004 21:09:28 -0000	1.14
+++ hppa-tdep.h	6 Dec 2004 22:26:26 -0000
@@ -88,6 +88,15 @@ struct gdbarch_tdep
      IN_SOLIB_CALL_TRAMPOLINE evaluates to nonzero if we are currently
      stopped in one of these.  */
   int (*in_solib_call_trampoline) (CORE_ADDR pc, char *name);
+
+  /* For targets that support multiple spaces, we may have additional stubs
+     in the return path.  These stubs are internal to the ABI, and users are
+     not interested in them.  If we detect that we are returning to a stub,
+     adjust the pc to the real caller.  This improves the behavior of commands
+     that traverse frames such as "up" and "finish".  */
+  void (*unwind_adjust_stub) (struct frame_info *next_frame, CORE_ADDR base,
+  			      struct trad_frame_saved_reg *saved_regs);
+
 };
 
 /*

-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [RFC/hppa] Change handling of stubs in the return path

[Daniel Jacobowitz]

On Mon, Dec 06, 2004 at 02:37:12PM -0800, Randolph Chung wrote:
> The following patch changes the behavior of the unwinder for hpux so
> that export stubs are not visible when unwinding the stack. this is
> desired for several reasons as discussed on this list recently:
> 
> 1. it is less confusing to the user (and more consistent with how 
>    gdb used to work)
> 2. it improves the behavior for commands like "up" and "finish", 
>    because the user expects to return to the caller of the current
>    function, not to a stub that was synthesized by the toolchain
> 
> This fixes all the FAILs in funcargs.exp and some of the ones in
> callfuncs.exp (and possibly others, i haven't ran the whole test)
> 
> Note that the stub unwinder is still there for the cases when we single
> step into a stub.  There are some issues with the stub unwinder for HPUX
> as well, but I'll fix that separately.
> 
> comments? is this ok?

Does this work OK when single stepping out of something, i.e. back into
a stub?

> +static void
> +hppa_hpux_unwind_adjust_stub(struct frame_info *next_frame, CORE_ADDR base,
> +			     struct trad_frame_saved_reg *saved_regs)

Formatting ;-)


-- 
Daniel Jacobowitz

Re: [RFC/hppa] Change handling of stubs in the return path

[Randolph Chung]

> Does this work OK when single stepping out of something, i.e. back into
> a stub?

when you step out of a function that was called with a stub, you end up 
back at the caller, not the stub. is that what you mean? (when we step
into the stub, gdb detects that it is in a solib return trampoline and
runs the inferior until it is out of the trampoline). that part is not
really affected by this patch though.

> > +static void
> > +hppa_hpux_unwind_adjust_stub(struct frame_info *next_frame, CORE_ADDR base,
> > +			     struct trad_frame_saved_reg *saved_regs)
> 
> Formatting ;-)

sorry, the original version is ok, but i did some manual editting of the
file with vim that had expandtab set...

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [RFC/hppa] Change handling of stubs in the return path

[Daniel Jacobowitz]

On Mon, Dec 06, 2004 at 03:27:23PM -0800, Randolph Chung wrote:
> > Does this work OK when single stepping out of something, i.e. back into
> > a stub?
> 
> when you step out of a function that was called with a stub, you end up 
> back at the caller, not the stub. is that what you mean? (when we step
> into the stub, gdb detects that it is in a solib return trampoline and
> runs the inferior until it is out of the trampoline). that part is not
> really affected by this patch though.

When you step one instruction out of the function, you go from "frame
A, called by frame B" to "frame C, called by frame B".  Does GDB get
confused by this?  Try using "step"; it may decide that frame C was
called by frame A, and resume.

I guess that doesn't matter.  What usually happens in this case is GDB
loses control; but if it does, it will just hit the dummy frame
breakpoint, so who cares?

> 
> > > +static void
> > > +hppa_hpux_unwind_adjust_stub(struct frame_info *next_frame, CORE_ADDR base,
> > > +			     struct trad_frame_saved_reg *saved_regs)
> > 
> > Formatting ;-)
> 
> sorry, the original version is ok, but i did some manual editting of the
> file with vim that had expandtab set...

I meant the missing space after _stub.

-- 
Daniel Jacobowitz

Re: [RFC/hppa] Change handling of stubs in the return path

[Randolph Chung]

> When you step one instruction out of the function, you go from "frame
> A, called by frame B" to "frame C, called by frame B".  Does GDB get
> confused by this?  Try using "step"; it may decide that frame C was
> called by frame A, and resume.
> 
> I guess that doesn't matter.  What usually happens in this case is GDB
> loses control; but if it does, it will just hit the dummy frame
> breakpoint, so who cares?

are you talking about the function called by gdb case [1]? or the normal
function-called-shared-library-function case [2]?

but it doesn't matter, it works :) although in the case of [1], when
stepping out of A, you will step through C (this is arguably a bug,
because gdb doesn't consider __gcc_plt_stub a solib return trampoline)

in the case of [2], we step out of A, into C, gdb automatically skips C
because it is a solib return trampoline, so we end up directly back in B
without stopping in C.

> I meant the missing space after _stub.

ah, ok. fixed.

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/