Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: scott@scottlinder.com
To: Gdb Patches <gdb-patches@sourceware.org>
Subject: [RFC][PATCH] Support frames inlined into the outer frame
Date: Wed, 18 Mar 2020 16:43:55 -0400	[thread overview]
Message-ID: <237802b049d5e89a6d556559815b6f20@scottlinder.com> (raw)

AMD is working on a port of GDB for our GPUs based on the ROCm stack
(https://rocm.github.io/). We recently open-sourced the fork at
https://github.com/ROCm-Developer-Tools/ROCgdb. We hope to begin 
upstreaming
patches where possible,
and https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/767 is the 
first
such patch.

We frequently have functions inlined into the outermost frame. The 
current
implementation which introduced `outer_frame_id` as a "valid" ID 
distinct from
`null_frame_id` does not support this, asserting the need for both a 
valid
and non-`outer_frame_id` ID to base the ID of the inlined frame on.

This patch changes the definition of `outer_frame_id` slightly to 
effectively
represent a class of IDs which identify a frame inlined into the outer 
frame.
These differ in `artificial_depth`, but otherwise behave just as
`outer_frame_id` in that they are `FID_STACK_INVALID`, yet `frame_id_p` 
returns
`true` and they compare equal to each other.

Running the testsuite both with and without the patch doesn't yield any 
obvious
regressions, although I have not come up with a test case to prove this 
out on
e.g. x86.

Does this seem reasonable? It is a bit of a hack on a hack, considering 
the
existing issues with `outer_frame_id` and the obvious desire to remove 
it
completely. At the same time, there is a fair amount of thought and 
effort
involved in making that change, and I think it can/should be done 
independently
of fixing this bug. My feeling is this patch is a pretty non-invasive 
change
that doesn't make the situation fundamentally worse, but any feedback is
appreciated.

Cheers,
Scott


             reply	other threads:[~2020-03-18 20:43 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-18 20:43 scott [this message]
2020-03-18 21:17 ` scott
2020-03-18 21:27   ` Simon Marchi
2020-03-18 21:42     ` scott
2020-03-18 21:45       ` Simon Marchi
2020-03-18 22:06         ` Scott Linder
2020-03-18 22:11         ` [PATCH] [gdb] " Scott Linder
2020-03-24 10:22           ` Andrew Burgess
2020-03-30 22:22             ` scott
2020-03-31 19:18               ` [PATCH v2] " Scott Linder
2020-04-03 17:00                 ` Andrew Burgess
2020-04-17 20:41                   ` Scott Linder
2020-04-03 19:37                 ` Luis Machado
2020-04-17 20:51                   ` Scott Linder
2020-06-04 16:11                 ` Simon Marchi
2020-06-04 19:23                   ` Simon Marchi
2020-06-08 12:00                     ` Luis Machado
2020-06-08 16:01                       ` Simon Marchi
2020-06-08 16:10                         ` Luis Machado
2020-04-02 19:30               ` [PATCH] " Pedro Alves
2020-04-17 20:35                 ` Scott Linder

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=237802b049d5e89a6d556559815b6f20@scottlinder.com \
    --to=scott@scottlinder.com \
    --cc=gdb-patches@sourceware.org \
    /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