Re: [PATCH 2/6] gdb: make it possible to restore selected user-created frames

[Simon Marchi <simon.marchi@polymtl.ca>]

>> 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.

>> @@ -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.

Simon