Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Simon Marchi <simon.marchi@polymtl.ca>
To: gdb-patches@sourceware.org
Cc: Zoran.Zaric@amd.com, Tony.Tye@amd.com, Scott.Linder@amd.com,
	Simon Marchi <simon.marchi@efficios.com>
Subject: [PATCH v2 2/3] gdb: introduce explicit outer frame id kind
Date: Thu, 27 Aug 2020 16:57:23 -0400	[thread overview]
Message-ID: <20200827205724.409603-3-simon.marchi@polymtl.ca> (raw)
In-Reply-To: <20200827205724.409603-1-simon.marchi@polymtl.ca>

From: Simon Marchi <simon.marchi@efficios.com>

In the following patch, we'll need to easily differentiate the frame_id
of the outer frame (or the frame id of a frame inlined into the outer
frame) from a simply invalid frame id.

Currently, the frame id of the outer frame has `stack_status` set to
FID_STACK_INVALID plus special_addr_p set.  A frame inlined into the
outer frame would also have `artificial_depth` set to greater than one.
That makes the job of differntiating the frame id of the outer frame (or a
frame inlined into the outer frame) cumbersome.

To make it easier, give the outer frame id its own frame_id_stack_status
enum value.  outer_frame_id then becomes very similar to
sentinel_frame_id, another "special" frame id value.

In frame_id_p, we don't need a special case for the outer frame id, as
it's no long a special case of FID_STACK_INVALID.  Same goes for
frame_id_eq.

So in the end, FID_STACK_OUTER isn't even used (except in
fprint_frame_id).  But that's expected: all the times we wanted to
identify an outer frame was to differentiate it from an otherwise
invalid frame.  Since their frame_id_stack_status value is different
now, that is done naturally.

gdb/ChangeLog:

	* frame.h (enum frame_id_stack_status) <FID_STACK_OUTER>: New.
	* frame.c (fprint_frame_id): Handle FID_STACK_OUTER.
	(outer_frame_id): Use FID_STACK_OUTER instead of
	FID_STACK_INVALID.
	(frame_id_p): Don't check for outer_frame_id.

Change-Id: I654e7f936349debc4f04f7f684b15e71a0c37619
---
 gdb/frame.c | 19 +++++--------------
 gdb/frame.h |  9 +++++++--
 2 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/gdb/frame.c b/gdb/frame.c
index ccaf97dc7e91..54f4c613c9e8 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -396,8 +396,11 @@ fprint_frame_id (struct ui_file *file, struct frame_id id)
     fprintf_unfiltered (file, "stack=<unavailable>");
   else if (id.stack_status == FID_STACK_SENTINEL)
     fprintf_unfiltered (file, "stack=<sentinel>");
+  else if (id.stack_status == FID_STACK_OUTER)
+    fprintf_unfiltered (file, "stack=<outer>");
   else
     fprintf_unfiltered (file, "stack=%s", hex_string (id.stack_addr));
+
   fprintf_unfiltered (file, ",");
 
   fprint_field (file, "code", id.code_addr_p, id.code_addr);
@@ -672,7 +675,7 @@ frame_unwind_caller_id (struct frame_info *next_frame)
 
 const struct frame_id null_frame_id = { 0 }; /* All zeros.  */
 const struct frame_id sentinel_frame_id = { 0, 0, 0, FID_STACK_SENTINEL, 0, 1, 0 };
-const struct frame_id outer_frame_id = { 0, 0, 0, FID_STACK_INVALID, 0, 1, 0 };
+const struct frame_id outer_frame_id = { 0, 0, 0, FID_STACK_OUTER, 0, 1, 0 };
 
 struct frame_id
 frame_id_build_special (CORE_ADDR stack_addr, CORE_ADDR code_addr,
@@ -746,10 +749,6 @@ frame_id_p (frame_id l)
   /* The frame is valid iff it has a valid stack address.  */
   bool p = l.stack_status != FID_STACK_INVALID;
 
-  /* outer_frame_id is also valid.  */
-  if (!p && memcmp (&l, &outer_frame_id, sizeof (l)) == 0)
-    p = true;
-
   if (frame_debug)
     {
       fprintf_unfiltered (gdb_stdlog, "{ frame_id_p (l=");
@@ -774,15 +773,7 @@ frame_id_eq (frame_id l, frame_id r)
 {
   bool eq;
 
-  if (l.stack_status == FID_STACK_INVALID && l.special_addr_p
-      && r.stack_status == FID_STACK_INVALID && r.special_addr_p)
-    /* The outermost frame marker is equal to itself.  This is the
-       dodgy thing about outer_frame_id, since between execution steps
-       we might step into another function - from which we can't
-       unwind either.  More thought required to get rid of
-       outer_frame_id.  */
-    eq = true;
-  else if (l.stack_status == FID_STACK_INVALID
+  if (l.stack_status == FID_STACK_INVALID
 	   || r.stack_status == FID_STACK_INVALID)
     /* Like a NaN, if either ID is invalid, the result is false.
        Note that a frame ID is invalid iff it is the null frame ID.  */
diff --git a/gdb/frame.h b/gdb/frame.h
index 1c6afad1ae95..3ceb7b32effa 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -85,8 +85,7 @@ struct frame_print_options;
 
 enum frame_id_stack_status
 {
-  /* Stack address is invalid.  E.g., this frame is the outermost
-     (i.e., _start), and the stack hasn't been setup yet.  */
+  /* Stack address is invalid.  */
   FID_STACK_INVALID = 0,
 
   /* Stack address is valid, and is found in the stack_addr field.  */
@@ -95,6 +94,12 @@ enum frame_id_stack_status
   /* Sentinel frame.  */
   FID_STACK_SENTINEL = 2,
 
+  /* Outer frame.  Since a frame's stack address is typically defined as the
+     value the stack pointer had prior to the activation of the frame, an outer
+     frame doesn't have a stack address.  The frame ids of frames inlined in the
+     outer frame are also of this type.  */
+  FID_STACK_OUTER = 3,
+
   /* Stack address is unavailable.  I.e., there's a valid stack, but
      we don't know where it is (because memory or registers we'd
      compute it from were not collected).  */
-- 
2.28.0



  parent reply	other threads:[~2020-08-27 20:57 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-27 20:57 [PATCH v2 0/3] Support frames inlined in outer frames Simon Marchi
2020-08-27 20:57 ` [PATCH v2 1/3] gdb: make frame_unwind_got_optimized return a not_lval value Simon Marchi
2020-08-27 21:37   ` Pedro Alves
2020-08-27 20:57 ` Simon Marchi [this message]
2020-08-27 21:38   ` [PATCH v2 2/3] gdb: introduce explicit outer frame id kind Pedro Alves
2020-08-27 20:57 ` [PATCH v2 3/3] gdb: support frames inlined into the outer frame Simon Marchi
2020-09-08  9:55   ` [committed][gdb/testsuite] Fix gdb.dwarf2/frame-inlined-in-outer-frame.exp Tom de Vries
2020-08-27 21:44 ` [PATCH v2 0/3] Support frames inlined in outer frames Pedro Alves
2020-08-28  8:50 ` Andrew Burgess
2020-08-31 17:32   ` Simon Marchi

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=20200827205724.409603-3-simon.marchi@polymtl.ca \
    --to=simon.marchi@polymtl.ca \
    --cc=Scott.Linder@amd.com \
    --cc=Tony.Tye@amd.com \
    --cc=Zoran.Zaric@amd.com \
    --cc=gdb-patches@sourceware.org \
    --cc=simon.marchi@efficios.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