From: Bruno Larsen <blarsen@redhat.com>
To: Simon Marchi <simon.marchi@polymtl.ca>, gdb-patches@sourceware.org
Cc: Simon Marchi <simon.marchi@efficios.com>
Subject: Re: [PATCH 2/6] gdb: make it possible to restore selected user-created frames
Date: Mon, 12 Dec 2022 12:14:06 +0100 [thread overview]
Message-ID: <4abe5d7b-db2f-1651-b500-13e51303178d@redhat.com> (raw)
In-Reply-To: <f1259ccc-0749-a467-ea84-bc16c2d01ce1@polymtl.ca>
On 08/12/2022 21:53, Simon Marchi wrote:
>>> Now, the question is, how to identify a user-created frame through the
>>> selected_frame_level/selected_frame_id pair. I initially added a
>>> separate "selected_frame_is_user_created" flag to that pair. Then it
>>> seemed simpler to make it so user-created frames would be identified by
>>> the selected_frame_level == 0. User-created frames already happen to
>>> have level 0. And remember that when the saved frame is the current
>>> target frame, selected_frame_level is -1. We need to modify
>>> select_frame in any case to make it so it will save the frame id and use
>>> the saved level 0 for user-created frames, despite them having level
>>> 0. So just with that, it's possible to distinguish user-created frames,
>>> no need to add a separate flag. It results in:
>> But this doesn't necessarily sound correct if we're supposed to be
>> able to go up and down on user created frames. My best guess for such
>> a feature would be improving the debugging experience of optimized
>> programs or reverse engineering something. If you were able to
>> identify that something is an inlined function, for instance, you
>> could make an artificial frame to keep track of this information, so
>> that future executions (or just moving up and down the stack) was
>> easier. If that guess is correct, it would be possible to have a user
>> created frame at a level that isn't level 0, and the detection method
>> fails.
> You are right, if it was possible to unwind from that user frame, I
> think we'd need to keep a separate flag to say "this frame was unwound
> from a user-created frame, not the target current frame". And we would
> probably need to keep that user-created frame id too (in addition to the
> selected frame id), to recreate it. But at the moment, since it's not
> possible to unwind go up / down from a user-created frame, I don't think
> we should account for that. If it ever becomes possible to unwind
> user-created frames, we can change the strategy then.
Makes sense. I agree with the patch, then
Reviewed-By: Bruno Larsen <blarsen@redhat.com>
>
>>> @@ -1669,6 +1673,11 @@ get_current_frame (void)
>>> never 0 and SELECTED_FRAME_ID is never the ID of the innermost
>>> frame.
>>> + An exception to that is if the selected frame is a used-created one
>>> + (created and selected through the "select-frame view" command). That
>>> + is encoded by having SELECTED_FRAME_LEVEL == 0 and SELECTED_FRAME_ID
>>> + the frame id derived from the user-provided addresses.
>>> +
>> I would slightly reword this comment so there isn't incorrect information if someone only reads part of it. Maybe something like:
>>
>> If SELECTED_FRAME_ID / SELECTED_FRAME_LEVEL are null_frame_id / -1,
>> and the target has stack and is stopped, the selected frame is the
>> current (innermost) frame. SELECTED_FRAME_ID is never the ID of the innermost
>> frame. SELECTED_FRAME_LEVEL may be 0 only if the selected frame is a
>> a user-created one (created and selected through the "select-frame view"
>> command), in which case SELECTED_FRAME_ID is the frame id derived from
>> the user-provided addresses.
>>
>> This way there is no way someone can be confused by thinking that 0 is never acceptable.
> Thanks, I used that.
>
>>> + # Verify that the "frame" command does not change the selected frame.
>>> + gdb_test "frame" "#0 baz \\(z1=.*, z2=.*\\).*" "frame"
>>> + gdb_test "frame" "#0 baz \\(z1=.*, z2=.*\\).*" "frame again"
>> I think it would be nice to have an explanation as to why frame is
>> being called twice. Just a mention that there was a bug where we'd
>> lose track of user-created frames when using the command "frame" would
>> be enough, I think.
> Hmm I think the current comment pretty much says that. By saying
> "Verify that the frame command does not change the selected frame", it
> implies there was a bug where the frame command did change the selected
> frame. But I can make it clearer if you'd like, comments are not
> expensive.
Yeah, I guess you're right, I must have been a bit too tired when I read
this to not notice the implication you mentioned. It's probably fine.
--
Cheers,
Bruno
>
> Simon
>
next prev parent reply other threads:[~2022-12-12 11:14 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-02 18:00 [PATCH 0/6] Make frame_info_ptr automatic Simon Marchi
2022-12-02 18:00 ` [PATCH 1/6] gdb: add invalidate_selected_frame function Simon Marchi
2022-12-07 14:09 ` Bruno Larsen
2022-12-07 16:54 ` Simon Marchi
2022-12-02 18:00 ` [PATCH 2/6] gdb: make it possible to restore selected user-created frames Simon Marchi
2022-12-08 9:25 ` Bruno Larsen
2022-12-08 20:53 ` Simon Marchi
2022-12-12 11:14 ` Bruno Larsen [this message]
2022-12-02 18:00 ` [PATCH 3/6] gdb: make user-created frames reinflatable Simon Marchi
2022-12-12 11:32 ` Bruno Larsen
2022-12-12 13:17 ` Simon Marchi
2022-12-12 13:33 ` Bruno Larsen
2022-12-12 13:45 ` Simon Marchi
2022-12-02 18:00 ` [PATCH 4/6] gdb: revert frame_unwind::this_id and callees to use `frame_info *` Simon Marchi
2022-12-05 21:41 ` Tom Tromey
2022-12-07 16:52 ` Simon Marchi
2022-12-12 13:17 ` Bruno Larsen
2022-12-12 13:26 ` Simon Marchi
2022-12-02 18:00 ` [PATCH 5/6] gdb: make frame_info_ptr grab frame level and id on construction Simon Marchi
2022-12-08 8:51 ` Bruno Larsen
2022-12-08 20:57 ` Simon Marchi
2022-12-02 18:00 ` [PATCH 6/6] gdb: make frame_info_ptr auto-reinflatable 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=4abe5d7b-db2f-1651-b500-13e51303178d@redhat.com \
--to=blarsen@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=simon.marchi@efficios.com \
--cc=simon.marchi@polymtl.ca \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).