[review] [gdb] Support frames inlined into the outer frame

[review] [gdb] Support frames inlined into the outer frame

[Scott Linder (Code Review)]

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/767
......................................................................

[gdb] Support frames inlined into the outer frame

Broaden the definition of `outer_frame_id` to effectively create a new
class of "invalid" IDs to represent frames inlined into the outer frame.
These new IDs behave like the outer frame, in that they are "invalid",
yet return true from `frame_id_p` and compare equal to themselves.

2020-03-18  Scott Linder  <scott@scottlinder.com>

	* frame.c (frame_id_p): Consider functions inlined into outer frame
	as valid.
	(frame_id_eq): Consider functions inlined into outer frame with same
	artificial_depth as equal.

	* frame.h (outer_frame_id): Update comment.
	(frame_id_p): Update comment.

	* inline-frame.c (inline_frame_this_id): Remove assert that prevents
	inline frame ids in outer frame.

Change-Id: I8aa129c667dccc31590ffdf426586418493a6ebe
---
M gdb/frame.c
M gdb/frame.h
M gdb/inline-frame.c
3 files changed, 10 insertions(+), 12 deletions(-)



diff --git a/gdb/frame.c b/gdb/frame.c
index d74d1d5..b62d68f 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -694,8 +694,8 @@
 
   /* The frame is valid iff it has a valid stack address.  */
   p = l.stack_status != FID_STACK_INVALID;
-  /* outer_frame_id is also valid.  */
-  if (!p && memcmp (&l, &outer_frame_id, sizeof (l)) == 0)
+  /* outer_frame_id and functions inlined into it are also valid.  */
+  if (!p && l.special_addr_p)
     p = 1;
   if (frame_debug)
     {
@@ -722,12 +722,13 @@
 
   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
+    /* The outermost frame marker, and any inline frame markers
+       derived from it, are equal to themselves.  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 = 1;
+    eq = l.artificial_depth == r.artificial_depth;
   else 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.
diff --git a/gdb/frame.h b/gdb/frame.h
index cfc1502..d394382 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -195,7 +195,8 @@
 
 /* This means "there is no frame ID, but there is a frame".  It should be
    replaced by best-effort frame IDs for the outermost frame, somehow.
-   The implementation is only special_addr_p set.  */
+   The implementation is only special_addr_p, and possibly
+   artificial_depth, set.  */
 extern const struct frame_id outer_frame_id;
 
 /* Flag to control debugging.  */
@@ -237,8 +238,8 @@
 extern struct frame_id frame_id_build_wild (CORE_ADDR stack_addr);
 
 /* Returns non-zero when L is a valid frame (a valid frame has a
-   non-zero .base).  The outermost frame is valid even without an
-   ID.  */
+   non-zero .base).  The outermost frame and any frames inlined into it
+   are valid even without an ID.  */
 extern int frame_id_p (struct frame_id l);
 
 /* Returns non-zero when L is a valid frame representing a frame made up by GDB
diff --git a/gdb/inline-frame.c b/gdb/inline-frame.c
index c650195..a187630 100644
--- a/gdb/inline-frame.c
+++ b/gdb/inline-frame.c
@@ -171,10 +171,6 @@
      frame").  This will take work.  */
   gdb_assert (frame_id_p (*this_id));
 
-  /* For now, require we don't match outer_frame_id either (see
-     comment above).  */
-  gdb_assert (!frame_id_eq (*this_id, outer_frame_id));
-
   /* Future work NOTE: Alexandre Oliva applied a patch to GCC 4.3
      which generates DW_AT_entry_pc for inlined functions when
      possible.  If this attribute is available, we should use it

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I8aa129c667dccc31590ffdf426586418493a6ebe
Gerrit-Change-Number: 767
Gerrit-PatchSet: 1
Gerrit-Owner: Scott Linder <scott@scottlinder.com>
Gerrit-MessageType: newchange

[review] [gdb] Support frames inlined into the outer frame

[Simon Marchi (Code Review)]

Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/767
......................................................................


Patch Set 1:

Hi Scott,

Thanks for the patch.  However, we don't really use Gerrit anymore.  We gave it a try and it didn't work so well for us, so we decided to keep the mailing list process for now.  Could you please send your patch to the gdb-patches mailing list?

Thanks!


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I8aa129c667dccc31590ffdf426586418493a6ebe
Gerrit-Change-Number: 767
Gerrit-PatchSet: 1
Gerrit-Owner: Scott Linder <scott@scottlinder.com>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Wed, 18 Mar 2020 20:33:16 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[review] [gdb] Support frames inlined into the outer frame

[Scott Linder (Code Review)]

Scott Linder has abandoned this change. ( https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/767 )

Change subject: [gdb] Support frames inlined into the outer frame
......................................................................


Abandoned

Moving to gdb-patches
-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I8aa129c667dccc31590ffdf426586418493a6ebe
Gerrit-Change-Number: 767
Gerrit-PatchSet: 1
Gerrit-Owner: Scott Linder <scott@scottlinder.com>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: abandon

[review] [gdb] Support frames inlined into the outer frame

[Scott Linder (Code Review)]

Scott Linder has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/767
......................................................................


Patch Set 1:

> Patch Set 1:
> 
> Hi Scott,
> 
> Thanks for the patch.  However, we don't really use Gerrit anymore.  We gave it a try and it didn't work so well for us, so we decided to keep the mailing list process for now.  Could you please send your patch to the gdb-patches mailing list?
> 
> Thanks!

Hi Simon,

My mistake, I haven't been following the list as closely recently. I already sent an email referencing this review to gdb-patches, so I will just reply there with the formatted patch.


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I8aa129c667dccc31590ffdf426586418493a6ebe
Gerrit-Change-Number: 767
Gerrit-PatchSet: 1
Gerrit-Owner: Scott Linder <scott@scottlinder.com>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Wed, 18 Mar 2020 21:10:24 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[review] [gdb] Support frames inlined into the outer frame

[Simon Marchi (Code Review)]

Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/767
......................................................................


Patch Set 1:

> Patch Set 1:
> 
> > Patch Set 1:
> > 
> > Hi Scott,
> > 
> > Thanks for the patch.  However, we don't really use Gerrit anymore.  We gave it a try and it didn't work so well for us, so we decided to keep the mailing list process for now.  Could you please send your patch to the gdb-patches mailing list?
> > 
> > Thanks!
> 
> Hi Simon,
> 
> My mistake, I haven't been following the list as closely recently. I already sent an email referencing this review to gdb-patches, so I will just reply there with the formatted patch.

No problem.  In fact, I had tried to disable creation of new patches on Gerrit, but clearly it didn't work.


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I8aa129c667dccc31590ffdf426586418493a6ebe
Gerrit-Change-Number: 767
Gerrit-PatchSet: 1
Gerrit-Owner: Scott Linder <scott@scottlinder.com>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Wed, 18 Mar 2020 21:23:28 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment