Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Mark Kettenis <kettenis@chello.nl>
To: brobecker@gnat.com
Cc: cagney@gnu.org, ezannoni@redhat.com, gdb-patches@sources.redhat.com
Subject: Re: [RFA] use frame IDs to detect function calls while stepping
Date: Wed, 03 Mar 2004 21:12:00 -0000	[thread overview]
Message-ID: <200403032112.i23LC2G2001443@elgar.kettenis.dyndns.org> (raw)
In-Reply-To: <20040302061642.GW1051@gnat.com> (message from Joel Brobecker on Mon, 1 Mar 2004 22:16:42 -0800)

   Date: Mon, 1 Mar 2004 22:16:42 -0800
   From: Joel Brobecker <brobecker@gnat.com>

   > The expected behavior for the last "next" command is for GDB
   > to run until the inferior exits:
   > 
   >         (gdb) n
   >         Single stepping until exit from function _start, 
   >         which has no line number information.
   >         
   >         Program exited normally.
   > 
   > Unfortunately, here is what happens. At 0x000105ec, before we do
   > our second "next" command, we are about to execute the following
   > code:
   > 
   >         0x000105ec <_start+100>:        call  0x20950 <exit>
   >         0x000105f0 <_start+104>:        nop 
   > 
   > After two iterations (one for the call insn, and one for the delay
   > slot), GDB lands at the begining of function "exit" at 0x00020950,
   > which is:
   > 
   >         0x00020950 <exit+0>:    sethi  %hi(0xf000), %g1
   >         0x00020954 <exit+4>:    b,a   0x20914 <_PROCEDURE_LINKAGE_TABLE_>
   >         0x00020958 <exit+8>:    nop 
   > 
   > So at this point, the registers window has not been rotated.
   > I don't know if this is the cause for this problem, but at this
   > point GDB is unable to unwind the call stack:
   > 
   >         (gdb) bt
   >         #0  0x00020950 in _PROCEDURE_LINKAGE_TABLE_ ()
   > 
   > (And gets the wrong procedure name as well, but that's a separate
   > issue - although "x /i" does report what I believe is the correct
   > name, strange!).

Actually, it doesn't get the name wrong.  The function "exit" lives in
a shared library.  Since this is the first time you're calling exit(),
you don't end up in the function itself, but in the PLT entry for
"exit".  This PLT entry will invoke the dynamic linker to direct the
program to the real "exit" function [1].  The PLT entry function is
part of the Procedure Linkage Table (PLT) which is usally named
_PROCEDURE_LINKAGE_TABLE.

   > I am looking into the sparc unwinder code right now, to try to
   > understand a bit better the source of the problem.

   I think I found the source of the glitch. I may have the solution
   to fix it, but my little finger is telling that it might be a bit
   too extreme... Maybe MarkK has some comments about this?

It's not "extreme" at all.  Read on...

   What happens is that, at the point when we reach function "exit",
   the FP register is null:

	   (gdb) p /x $fp
	   $2 = 0x0

   The sparc unwinder in sparc_frame_cache() detects this, thinks there is
   something wrong, and aborts early. So, we never unwind the "_start"
   frame, and hence the following frame ID check doesn't notice the
   function call, as it should have in this case:

   +      if (frame_id_eq (get_frame_id (get_prev_frame (get_current_frame ())),
   +                       step_frame_id))
   +        {
   +          /* It's a subroutine call.  */
   +          handle_step_into_function (ecs);
   +          return;
   +        }

   With this example in mind, it seemed to me that the assertion that %fp
   register is not null is unfortunately incorrect. Given that the rest of
   the code in sparc_frame_cache() wasn't using the value of that register,
   I commented out the assertion, and retried.

OK.  Here %fp == 0 is marking the outermost frame.  So what we're
having is that we're effectively calling a frameless function (the PLT
entry) from the outermost frame.  Since this function is frameless, we
shouldn't be looking at %fp, but at %sp.  However, we bail out before
we do so.  So the check is bogus!  

I'm currently testing the attached patch.  If I see no regressions,
I'll check it in.  Is it needed on the branch too?

Mark


[1] It'll also patch things up such that any future calls to exit()
will go directly to the real function.  Pointless of course for
exit(), but this mechanism is used for all calls to functions living
in a shared library.


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* sparc-tdep.c (sparc_frame_cache): Don't bail out if %fp is zero.
	Reorganize code a bit.

Index: sparc-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc-tdep.c,v
retrieving revision 1.149
diff -u -p -r1.149 sparc-tdep.c
--- sparc-tdep.c 7 Feb 2004 20:40:35 -0000 1.149
+++ sparc-tdep.c 3 Mar 2004 21:11:29 -0000
@@ -615,14 +615,6 @@ sparc_frame_cache (struct frame_info *ne
   cache = sparc_alloc_frame_cache ();
   *this_cache = cache;
 
-  /* In priciple, for normal frames, %fp (%i6) holds the frame
-     pointer, which holds the base address for the current stack
-     frame.  */
-
-  cache->base = frame_unwind_register_unsigned (next_frame, SPARC_FP_REGNUM);
-  if (cache->base == 0)
-    return cache;
-
   cache->pc = frame_func_unwind (next_frame);
   if (cache->pc != 0)
     {
@@ -632,10 +624,18 @@ sparc_frame_cache (struct frame_info *ne
 
   if (cache->frameless_p)
     {
-      /* We didn't find a valid frame, which means that CACHE->base
-	 currently holds the frame pointer for our calling frame.  */
-      cache->base = frame_unwind_register_unsigned (next_frame,
-						    SPARC_SP_REGNUM);
+      /* This function is frameless, so %fp (%i6) holds the frame
+         pointer for our calling frame.  Use %sp (%o6) as this frame's
+         base address.  */
+      cache->base =
+	frame_unwind_register_unsigned (next_frame, SPARC_SP_REGNUM);
+    }
+  else
+    {
+      /* For normal frames, %fp (%i6) holds the frame pointer, the
+         base address for the current stack frame.  */
+      cache->base =
+	frame_unwind_register_unsigned (next_frame, SPARC_FP_REGNUM);
     }
 
   return cache;


WARNING: multiple messages have this Message-ID
From: Mark Kettenis <kettenis@chello.nl>
To: brobecker@gnat.com
Cc: cagney@gnu.org, ezannoni@redhat.com, gdb-patches@sources.redhat.com
Subject: Re: [RFA] use frame IDs to detect function calls while stepping
Date: Fri, 19 Mar 2004 00:09:00 -0000	[thread overview]
Message-ID: <200403032112.i23LC2G2001443@elgar.kettenis.dyndns.org> (raw)
Message-ID: <20040319000900.kijR1q3AatBB1xd_I5BJsrG0cVPs8NnLQ7YIGxAuOKM@z> (raw)
In-Reply-To: <20040302061642.GW1051@gnat.com> (message from Joel Brobecker on Mon, 1 Mar 2004 22:16:42 -0800)

   Date: Mon, 1 Mar 2004 22:16:42 -0800
   From: Joel Brobecker <brobecker@gnat.com>

   > The expected behavior for the last "next" command is for GDB
   > to run until the inferior exits:
   > 
   >         (gdb) n
   >         Single stepping until exit from function _start, 
   >         which has no line number information.
   >         
   >         Program exited normally.
   > 
   > Unfortunately, here is what happens. At 0x000105ec, before we do
   > our second "next" command, we are about to execute the following
   > code:
   > 
   >         0x000105ec <_start+100>:        call  0x20950 <exit>
   >         0x000105f0 <_start+104>:        nop 
   > 
   > After two iterations (one for the call insn, and one for the delay
   > slot), GDB lands at the begining of function "exit" at 0x00020950,
   > which is:
   > 
   >         0x00020950 <exit+0>:    sethi  %hi(0xf000), %g1
   >         0x00020954 <exit+4>:    b,a   0x20914 <_PROCEDURE_LINKAGE_TABLE_>
   >         0x00020958 <exit+8>:    nop 
   > 
   > So at this point, the registers window has not been rotated.
   > I don't know if this is the cause for this problem, but at this
   > point GDB is unable to unwind the call stack:
   > 
   >         (gdb) bt
   >         #0  0x00020950 in _PROCEDURE_LINKAGE_TABLE_ ()
   > 
   > (And gets the wrong procedure name as well, but that's a separate
   > issue - although "x /i" does report what I believe is the correct
   > name, strange!).

Actually, it doesn't get the name wrong.  The function "exit" lives in
a shared library.  Since this is the first time you're calling exit(),
you don't end up in the function itself, but in the PLT entry for
"exit".  This PLT entry will invoke the dynamic linker to direct the
program to the real "exit" function [1].  The PLT entry function is
part of the Procedure Linkage Table (PLT) which is usally named
_PROCEDURE_LINKAGE_TABLE.

   > I am looking into the sparc unwinder code right now, to try to
   > understand a bit better the source of the problem.

   I think I found the source of the glitch. I may have the solution
   to fix it, but my little finger is telling that it might be a bit
   too extreme... Maybe MarkK has some comments about this?

It's not "extreme" at all.  Read on...

   What happens is that, at the point when we reach function "exit",
   the FP register is null:

	   (gdb) p /x $fp
	   $2 = 0x0

   The sparc unwinder in sparc_frame_cache() detects this, thinks there is
   something wrong, and aborts early. So, we never unwind the "_start"
   frame, and hence the following frame ID check doesn't notice the
   function call, as it should have in this case:

   +      if (frame_id_eq (get_frame_id (get_prev_frame (get_current_frame ())),
   +                       step_frame_id))
   +        {
   +          /* It's a subroutine call.  */
   +          handle_step_into_function (ecs);
   +          return;
   +        }

   With this example in mind, it seemed to me that the assertion that %fp
   register is not null is unfortunately incorrect. Given that the rest of
   the code in sparc_frame_cache() wasn't using the value of that register,
   I commented out the assertion, and retried.

OK.  Here %fp == 0 is marking the outermost frame.  So what we're
having is that we're effectively calling a frameless function (the PLT
entry) from the outermost frame.  Since this function is frameless, we
shouldn't be looking at %fp, but at %sp.  However, we bail out before
we do so.  So the check is bogus!  

I'm currently testing the attached patch.  If I see no regressions,
I'll check it in.  Is it needed on the branch too?

Mark


[1] It'll also patch things up such that any future calls to exit()
will go directly to the real function.  Pointless of course for
exit(), but this mechanism is used for all calls to functions living
in a shared library.


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* sparc-tdep.c (sparc_frame_cache): Don't bail out if %fp is zero.
	Reorganize code a bit.

Index: sparc-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc-tdep.c,v
retrieving revision 1.149
diff -u -p -r1.149 sparc-tdep.c
--- sparc-tdep.c 7 Feb 2004 20:40:35 -0000 1.149
+++ sparc-tdep.c 3 Mar 2004 21:11:29 -0000
@@ -615,14 +615,6 @@ sparc_frame_cache (struct frame_info *ne
   cache = sparc_alloc_frame_cache ();
   *this_cache = cache;
 
-  /* In priciple, for normal frames, %fp (%i6) holds the frame
-     pointer, which holds the base address for the current stack
-     frame.  */
-
-  cache->base = frame_unwind_register_unsigned (next_frame, SPARC_FP_REGNUM);
-  if (cache->base == 0)
-    return cache;
-
   cache->pc = frame_func_unwind (next_frame);
   if (cache->pc != 0)
     {
@@ -632,10 +624,18 @@ sparc_frame_cache (struct frame_info *ne
 
   if (cache->frameless_p)
     {
-      /* We didn't find a valid frame, which means that CACHE->base
-	 currently holds the frame pointer for our calling frame.  */
-      cache->base = frame_unwind_register_unsigned (next_frame,
-						    SPARC_SP_REGNUM);
+      /* This function is frameless, so %fp (%i6) holds the frame
+         pointer for our calling frame.  Use %sp (%o6) as this frame's
+         base address.  */
+      cache->base =
+	frame_unwind_register_unsigned (next_frame, SPARC_SP_REGNUM);
+    }
+  else
+    {
+      /* For normal frames, %fp (%i6) holds the frame pointer, the
+         base address for the current stack frame.  */
+      cache->base =
+	frame_unwind_register_unsigned (next_frame, SPARC_FP_REGNUM);
     }
 
   return cache;


  reply	other threads:[~2004-03-03 21:12 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-02-05  4:41 Joel Brobecker
2004-02-05 17:13 ` Joel Brobecker
2004-02-05 18:54   ` Elena Zannoni
2004-02-07  4:01     ` Joel Brobecker
2004-02-27 15:23       ` Andrew Cagney
2004-03-19  0:09         ` Joel Brobecker
2004-03-01 19:48           ` Joel Brobecker
2004-03-19  0:09           ` Joel Brobecker
2004-03-01 23:52             ` Joel Brobecker
2004-03-02  6:16             ` Joel Brobecker
2004-03-03 21:12               ` Mark Kettenis [this message]
2004-03-19  0:09                 ` Mark Kettenis
2004-03-19  0:09               ` Joel Brobecker
2004-03-19  0:09               ` Andrew Cagney
2004-03-02 15:48                 ` Andrew Cagney
2004-03-19  0:09                 ` Joel Brobecker
2004-03-02 22:07                   ` Joel Brobecker
2004-03-06  0:15                   ` Andrew Cagney
2004-03-19  0:09                     ` Andrew Cagney
2004-02-05 19:01   ` Daniel Jacobowitz
2004-02-05 19:23     ` Elena Zannoni
2004-02-05 19:49       ` Daniel Jacobowitz
2004-02-09 19:21         ` Andrew Cagney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200403032112.i23LC2G2001443@elgar.kettenis.dyndns.org \
    --to=kettenis@chello.nl \
    --cc=brobecker@gnat.com \
    --cc=cagney@gnu.org \
    --cc=ezannoni@redhat.com \
    --cc=gdb-patches@sources.redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox