[PATCH] Fix gdb1476.exp on s390*

[PATCH] Fix gdb1476.exp on s390*

[Ulrich Weigand]

Hello,

this patch fixes the gdb.base/gdb1476.exp test case (backtrace from NULL
function pointer call).

When calling via an invalid function pointer, what we should do is simply
unwind the PC from the return address register and continue.  This is
exactly what is done to unwind from a PLT stub as well, so the patch 
simply uses the PLT frame unwinder to handle this case as well.

Tested with no regressions on s390-ibm-linux and s390x-ibm-linux.

Bye,
Ulrich

ChangeLog:

	* s390-tdep.c (s390_pltstub_frame_sniffer): Handle invalid function
	pointer calls like PLT calls.

Index: gdb/s390-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/s390-tdep.c,v
retrieving revision 1.129
diff -c -p -r1.129 s390-tdep.c
*** gdb/s390-tdep.c	26 Feb 2004 23:48:01 -0000	1.129
--- gdb/s390-tdep.c	15 Mar 2004 19:36:47 -0000
*************** static const struct frame_unwind s390_pl
*** 2103,2109 ****
  static const struct frame_unwind *
  s390_pltstub_frame_sniffer (struct frame_info *next_frame)
  {
!   if (!in_plt_section (frame_pc_unwind (next_frame), NULL))
      return NULL;
  
    return &s390_pltstub_frame_unwind;
--- 2103,2116 ----
  static const struct frame_unwind *
  s390_pltstub_frame_sniffer (struct frame_info *next_frame)
  {
!   CORE_ADDR pc = frame_pc_unwind (next_frame);
!   bfd_byte insn[S390_MAX_INSTR_SIZE];
! 
!   /* If the current PC points to non-readable memory, we assume we have
!      trapped due to an invalid function pointer call.  We handle the
!      non-existing current function like a PLT stub.  */
!   if (!in_plt_section (pc, NULL)
!       && s390_readinstruction (insn, pc) > 0)
      return NULL;
  
    return &s390_pltstub_frame_unwind;
-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de

[PATCH] Fix gdb1476.exp on s390*

[Ulrich Weigand]

Hello,

this patch fixes the gdb.base/gdb1476.exp test case (backtrace from NULL
function pointer call).

When calling via an invalid function pointer, what we should do is simply
unwind the PC from the return address register and continue.  This is
exactly what is done to unwind from a PLT stub as well, so the patch 
simply uses the PLT frame unwinder to handle this case as well.

Tested with no regressions on s390-ibm-linux and s390x-ibm-linux.

Bye,
Ulrich

ChangeLog:

	* s390-tdep.c (s390_pltstub_frame_sniffer): Handle invalid function
	pointer calls like PLT calls.

Index: gdb/s390-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/s390-tdep.c,v
retrieving revision 1.129
diff -c -p -r1.129 s390-tdep.c
*** gdb/s390-tdep.c	26 Feb 2004 23:48:01 -0000	1.129
--- gdb/s390-tdep.c	15 Mar 2004 19:36:47 -0000
*************** static const struct frame_unwind s390_pl
*** 2103,2109 ****
  static const struct frame_unwind *
  s390_pltstub_frame_sniffer (struct frame_info *next_frame)
  {
!   if (!in_plt_section (frame_pc_unwind (next_frame), NULL))
      return NULL;
  
    return &s390_pltstub_frame_unwind;
--- 2103,2116 ----
  static const struct frame_unwind *
  s390_pltstub_frame_sniffer (struct frame_info *next_frame)
  {
!   CORE_ADDR pc = frame_pc_unwind (next_frame);
!   bfd_byte insn[S390_MAX_INSTR_SIZE];
! 
!   /* If the current PC points to non-readable memory, we assume we have
!      trapped due to an invalid function pointer call.  We handle the
!      non-existing current function like a PLT stub.  */
!   if (!in_plt_section (pc, NULL)
!       && s390_readinstruction (insn, pc) > 0)
      return NULL;
  
    return &s390_pltstub_frame_unwind;
-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de

Re: [PATCH] Fix gdb1476.exp on s390*

[Andrew Cagney]

> Hello,
> 
> this patch fixes the gdb.base/gdb1476.exp test case (backtrace from NULL
> function pointer call).
> 
> When calling via an invalid function pointer, what we should do is simply
> unwind the PC from the return address register and continue.  This is
> exactly what is done to unwind from a PLT stub as well, so the patch 
> simply uses the PLT frame unwinder to handle this case as well.

While that might be true, I don't know that its the job of something 
called "PLTSTUB".  I can think of either:
- renaming pltstub to something that matches its new purpose (just "stub"?)
- using the default unwinder
thoughts?

Andrew

Re: [PATCH] Fix gdb1476.exp on s390*

[Ulrich Weigand]

Andrew Cagney wrote:

> > Hello,
> > 
> > this patch fixes the gdb.base/gdb1476.exp test case (backtrace from NULL
> > function pointer call).
> > 
> > When calling via an invalid function pointer, what we should do is simply
> > unwind the PC from the return address register and continue.  This is
> > exactly what is done to unwind from a PLT stub as well, so the patch 
> > simply uses the PLT frame unwinder to handle this case as well.
> 
> While that might be true, I don't know that its the job of something 
> called "PLTSTUB".  

Yes, the name's a bit unfortunate.  What this unwinder does is basically
unwind from any situation where we don't have a proper function prolog.
It could in theory handle all kinds of stubs, trampolines etc.

> I can think of either:
> - renaming pltstub to something that matches its new purpose (just "stub"?)

This would be my preferred solution.

> - using the default unwinder

The default unwinder as it is now would handle the situation incorrectly.
I could build the stub detection logic into the default unwinder, but that
would make it even more complex than it already is; I would think with the
new frame logic it is preferable to have multiple unwinders rather than a
single one that handles everything.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de

Re: [PATCH] Fix gdb1476.exp on s390*

[Andrew Cagney]

[-- Attachment #1: Type: text/plain, Size: 1391 bytes --]

> Andrew Cagney wrote:
> 
> 
>>>> > Hello,
>>>> > 
>>>> > this patch fixes the gdb.base/gdb1476.exp test case (backtrace from NULL
>>>> > function pointer call).
>>>> > 
>>>> > When calling via an invalid function pointer, what we should do is simply
>>>> > unwind the PC from the return address register and continue.  This is
>>>> > exactly what is done to unwind from a PLT stub as well, so the patch 
>>>> > simply uses the PLT frame unwinder to handle this case as well.
>>
>>> 
>>> While that might be true, I don't know that its the job of something 
>>> called "PLTSTUB".  
> 
> 
> Yes, the name's a bit unfortunate.  What this unwinder does is basically
> unwind from any situation where we don't have a proper function prolog.
> It could in theory handle all kinds of stubs, trampolines etc.
> 
> 
>>> I can think of either:
>>> - renaming pltstub to something that matches its new purpose (just "stub"?)
> 
> 
> This would be my preferred solution.

Try the attached.

Andrew

>>> - using the default unwinder
> 
> 
> The default unwinder as it is now would handle the situation incorrectly.
> I could build the stub detection logic into the default unwinder, but that
> would make it even more complex than it already is; I would think with the
> new frame logic it is preferable to have multiple unwinders rather than a
> single one that handles everything.
> 
> Bye,
> Ulrich


[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 5018 bytes --]

2004-03-22  Andrew Cagney  <cagney@redhat.com>

	* s390-tdep.c (struct s390_stub_unwind_cache): Rename
	s390_pltstub_unwind_cache.
	(s390_stub_frame_unwind_cache): Rename
	s390_pltstub_frame_unwind_cache.
	(s390_stub_frame_this_id): Rename s390_pltstub_frame_this_id.
	(s390_stub_frame_prev_register): Rename
	s390_pltstub_frame_prev_register.
	(s390_stub_frame_unwind): Rename s390_pltstub_frame_unwind.
	(s390_stub_frame_sniffer): Rename s390_stub_frame_sniffer.

	From Ulrich Weigand:
	* s390-tdep.c (s390_pltstub_frame_sniffer): Handle invalid function
	pointer calls like PLT calls.

Index: s390-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/s390-tdep.c,v
retrieving revision 1.129
diff -u -r1.129 s390-tdep.c
--- s390-tdep.c	26 Feb 2004 23:48:01 -0000	1.129
+++ s390-tdep.c	22 Mar 2004 21:55:33 -0000
@@ -2037,27 +2037,29 @@
 }
 
 
-/* PLT stub stack frames.  */
-
-struct s390_pltstub_unwind_cache {
+/* Code stubs and their stack frames.  For things like PLTs and NULL
+   function calls (where there is no true frame and the return address
+   is in the RETADDR register).  */
 
+struct s390_stub_unwind_cache
+{
   CORE_ADDR frame_base;
   struct trad_frame_saved_reg *saved_regs;
 };
 
-static struct s390_pltstub_unwind_cache *
-s390_pltstub_frame_unwind_cache (struct frame_info *next_frame,
-				 void **this_prologue_cache)
+static struct s390_stub_unwind_cache *
+s390_stub_frame_unwind_cache (struct frame_info *next_frame,
+			      void **this_prologue_cache)
 {
   struct gdbarch *gdbarch = get_frame_arch (next_frame);
   int word_size = gdbarch_ptr_bit (gdbarch) / 8;
-  struct s390_pltstub_unwind_cache *info;
+  struct s390_stub_unwind_cache *info;
   ULONGEST reg;
 
   if (*this_prologue_cache)
     return *this_prologue_cache;
 
-  info = FRAME_OBSTACK_ZALLOC (struct s390_pltstub_unwind_cache);
+  info = FRAME_OBSTACK_ZALLOC (struct s390_stub_unwind_cache);
   *this_prologue_cache = info;
   info->saved_regs = trad_frame_alloc_saved_regs (next_frame);
 
@@ -2072,41 +2074,47 @@
 }
 
 static void
-s390_pltstub_frame_this_id (struct frame_info *next_frame,
-			    void **this_prologue_cache,
-			    struct frame_id *this_id)
+s390_stub_frame_this_id (struct frame_info *next_frame,
+			 void **this_prologue_cache,
+			 struct frame_id *this_id)
 {
-  struct s390_pltstub_unwind_cache *info
-    = s390_pltstub_frame_unwind_cache (next_frame, this_prologue_cache);
+  struct s390_stub_unwind_cache *info
+    = s390_stub_frame_unwind_cache (next_frame, this_prologue_cache);
   *this_id = frame_id_build (info->frame_base, frame_pc_unwind (next_frame));
 }
 
 static void
-s390_pltstub_frame_prev_register (struct frame_info *next_frame,
-				  void **this_prologue_cache,
-				  int regnum, int *optimizedp,
-				  enum lval_type *lvalp, CORE_ADDR *addrp,
-				  int *realnump, void *bufferp)
+s390_stub_frame_prev_register (struct frame_info *next_frame,
+			       void **this_prologue_cache,
+			       int regnum, int *optimizedp,
+			       enum lval_type *lvalp, CORE_ADDR *addrp,
+			       int *realnump, void *bufferp)
 {
-  struct s390_pltstub_unwind_cache *info
-    = s390_pltstub_frame_unwind_cache (next_frame, this_prologue_cache);
+  struct s390_stub_unwind_cache *info
+    = s390_stub_frame_unwind_cache (next_frame, this_prologue_cache);
   trad_frame_prev_register (next_frame, info->saved_regs, regnum,
                             optimizedp, lvalp, addrp, realnump, bufferp);
 }
 
-static const struct frame_unwind s390_pltstub_frame_unwind = {
+static const struct frame_unwind s390_stub_frame_unwind = {
   NORMAL_FRAME,
-  s390_pltstub_frame_this_id,
-  s390_pltstub_frame_prev_register
+  s390_stub_frame_this_id,
+  s390_stub_frame_prev_register
 };
 
 static const struct frame_unwind *
-s390_pltstub_frame_sniffer (struct frame_info *next_frame)
+s390_stub_frame_sniffer (struct frame_info *next_frame)
 {
-  if (!in_plt_section (frame_pc_unwind (next_frame), NULL))
-    return NULL;
+  CORE_ADDR pc = frame_pc_unwind (next_frame);
+  bfd_byte insn[S390_MAX_INSTR_SIZE];
 
-  return &s390_pltstub_frame_unwind;
+  /* If the current PC points to non-readable memory, we assume we
+     have trapped due to an invalid function pointer call.  We handle
+     the non-existing current function like a PLT stub.  */
+  if (in_plt_section (pc, NULL)
+      || s390_readinstruction (insn, pc) < 0)
+    return &s390_stub_frame_unwind;
+  return NULL;
 }
 
 
@@ -3028,7 +3036,7 @@
   dwarf2_frame_set_init_reg (gdbarch, s390_dwarf2_frame_init_reg);
   frame_unwind_append_sniffer (gdbarch, dwarf2_frame_sniffer);
   frame_base_append_sniffer (gdbarch, dwarf2_frame_base_sniffer);
-  frame_unwind_append_sniffer (gdbarch, s390_pltstub_frame_sniffer);
+  frame_unwind_append_sniffer (gdbarch, s390_stub_frame_sniffer);
   frame_unwind_append_sniffer (gdbarch, s390_sigtramp_frame_sniffer);
   frame_unwind_append_sniffer (gdbarch, s390_frame_sniffer);
   frame_base_set_default (gdbarch, &s390_frame_base);

Re: [PATCH] Fix gdb1476.exp on s390*

[Ulrich Weigand]

Andrew Cagney wrote:

> Try the attached.

Works fine on s390-ibm-linux and s390x-ibm-linux, no test suite
regression, and fixes the gdb1476 NULL pointer test.

Thanks for taking care of this!

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  weigand@informatik.uni-erlangen.de

Re: [PATCH] Fix gdb1476.exp on s390*

[Andrew Cagney]

> Andrew Cagney wrote:
> 
> 
>>> Try the attached.
> 
> 
> Works fine on s390-ibm-linux and s390x-ibm-linux, no test suite
> regression, and fixes the gdb1476 NULL pointer test.
> 
> Thanks for taking care of this!

committed to mainline and 6.1 branch

Andrew