Bug 31224 - Hard to understand warning when reaching end of reverse-execution history
Summary: Hard to understand warning when reaching end of reverse-execution history
Status: RESOLVED FIXED
Alias: None
Product: gdb
Classification: Unclassified
Component: record (show other bugs)
Version: HEAD
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: good-first-bug
Depends on:
Blocks:
 
Reported: 2024-01-09 15:41 UTC by Guinevere Larsen
Modified: 2024-08-26 13:44 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Guinevere Larsen 2024-01-09 15:41:28 UTC
Currently, if a user is running a record session, moves backwards a bit and moves forward again, they may hit the following warning:

> No more reverse-execution history.

My very scientific testing of asking one end user highlighted that this message sounds like GDB will be unable to move the inferior forward, when all it means is that we will make the CPU actually execute the inferior, instead of simulating it.

This is made worse because when the inferior executes backwards to the end of the history, GDB really can't make the inferior continue backwards and the message is exactly the same.

It would be nice if the warning was different when moving forward, and was reworded to make clear that the inferior can continue executing. I would suggest changing it to:

> No more reverse-execution history. Switching to normal execution

However, I'm open to other ideas.
Comment 1 Alex Chronopoulos 2024-02-24 10:25:10 UTC
Hi, I could work on it, if that's ok. My understanding is that this bug is about changing the string in `print_no_history_reason()` method. Is there more than that?
Comment 2 Guinevere Larsen 2024-02-24 15:28:14 UTC
that is all there is to it, yes. instead of always printing the same thing, it should check if the inferior is moving forwards or backwards and change the message accordingly.

I'll be happy to review your changes, once you send them to the list, or give you guidance in private if you need it :) (though fair warning, I usually only look at that stuff during work hours for Europe)
Comment 3 Shobhit 2024-07-05 16:39:29 UTC
(In reply to Guinevere Larsen from comment #0)
> Currently, if a user is running a record session, moves backwards a bit and
> moves forward again, they may hit the following warning:
> 
> > No more reverse-execution history.
> 
> My very scientific testing of asking one end user highlighted that this
> message sounds like GDB will be unable to move the inferior forward, when
> all it means is that we will make the CPU actually execute the inferior,
> instead of simulating it.
> 
> This is made worse because when the inferior executes backwards to the end
> of the history, GDB really can't make the inferior continue backwards and
> the message is exactly the same.
> 
> It would be nice if the warning was different when moving forward, and was
> reworded to make clear that the inferior can continue executing. I would
> suggest changing it to:
> 
> > No more reverse-execution history. Switching to normal execution
> 
> However, I'm open to other ideas.

Hello, I would like to work on this if no one else is working on this yet. 

I think I just need to find a way to check if the inferior reached the end while going backwards (in which case current message should print) or if the inferior reached the end of history while moving forward (in which case a different and more clear message should print). 

I just have a doubt that from where the function 'print_no_history_reason()' is actually called to execute(which I need to find some way by which I can check if the inferior reached the end while going backwards or forwards), but I am unable to find it right now.
Comment 4 Guinevere Larsen 2024-07-05 17:39:44 UTC
(In reply to Shobhit from comment #3)
> (In reply to Guinevere Larsen from comment #0)
> > Currently, if a user is running a record session, moves backwards a bit and
> > moves forward again, they may hit the following warning:
> > 
> > > No more reverse-execution history.
> > 
> > My very scientific testing of asking one end user highlighted that this
> > message sounds like GDB will be unable to move the inferior forward, when
> > all it means is that we will make the CPU actually execute the inferior,
> > instead of simulating it.
> > 
> > This is made worse because when the inferior executes backwards to the end
> > of the history, GDB really can't make the inferior continue backwards and
> > the message is exactly the same.
> > 
> > It would be nice if the warning was different when moving forward, and was
> > reworded to make clear that the inferior can continue executing. I would
> > suggest changing it to:
> > 
> > > No more reverse-execution history. Switching to normal execution
> > 
> > However, I'm open to other ideas.
> 
> Hello, I would like to work on this if no one else is working on this yet. 

Alex is already working on this, and has a patch (with a couple of revisions) on the list already: https://inbox.sourceware.org/gdb-patches/20240705165820.220790-1-achronop@gmail.com/

I will explain the rest, though, in case you decide to work on some other issue that touches either subsystem.

> 
> I think I just need to find a way to check if the inferior reached the end
> while going backwards (in which case current message should print) or if the
> inferior reached the end of history while moving forward (in which case a
> different and more clear message should print). 

GDB can internally figure out the direction the inferior is going by checking the global variable execution_direction. It is defined in gdb/infrun.c and exported by the gdb/infrun.h header.

> 
> I just have a doubt that from where the function 'print_no_history_reason()'
> is actually called to execute(which I need to find some way by which I can
> check if the inferior reached the end while going backwards or forwards),
> but I am unable to find it right now.

GDB deals with notifying the user of anything by using interps (I think that's short for interpreters). For example, MI, CLI and TUI all have interps which will know how to notify the user.
When we reach the end of history, "interps_notify_no_history" is called, which gets the interpreters to generate their no_history warnings. For example, check the gdb/cli/cli-interp.c

However, to get that message to be called, all you need is to record a bit of execution, then use "reverse-continue" to reach the end backwards, and from there "continue" will reach the end forwards (assuming the inferior doesn't stop of any other reason).
Comment 5 Shobhit 2024-07-05 18:30:39 UTC

(In reply to Guinevere Larsen from comment #4)
> (In reply to Shobhit from comment #3)
> > (In reply to Guinevere Larsen from comment #0)
> > > Currently, if a user is running a record session, moves backwards a bit and
> > > moves forward again, they may hit the following warning:
> > > 
> > > > No more reverse-execution history.
> > > 
> > > My very scientific testing of asking one end user highlighted that this
> > > message sounds like GDB will be unable to move the inferior forward, when
> > > all it means is that we will make the CPU actually execute the inferior,
> > > instead of simulating it.
> > > 
> > > This is made worse because when the inferior executes backwards to the end
> > > of the history, GDB really can't make the inferior continue backwards and
> > > the message is exactly the same.
> > > 
> > > It would be nice if the warning was different when moving forward, and was
> > > reworded to make clear that the inferior can continue executing. I would
> > > suggest changing it to:
> > > 
> > > > No more reverse-execution history. Switching to normal execution
> > > 
> > > However, I'm open to other ideas.
> > 
> > Hello, I would like to work on this if no one else is working on this yet. 
> 
> Alex is already working on this, and has a patch (with a couple of
> revisions) on the list already:
> https://inbox.sourceware.org/gdb-patches/20240705165820.220790-1-
> achronop@gmail.com/

Oh alright, I was thinking it's still open because it was still showing not assigned to anyone.
> 
> I will explain the rest, though, in case you decide to work on some other
> issue that touches either subsystem.

Thanks a ton, I am trying to navigate the gdb codebase and will try to contribute in upcoming days.
  
> > I think I just need to find a way to check if the inferior reached the end
> > while going backwards (in which case current message should print) or if the
> > inferior reached the end of history while moving forward (in which case a
> > different and more clear message should print). 
> 
> GDB can internally figure out the direction the inferior is going by
> checking the global variable execution_direction. It is defined in
> gdb/infrun.c and exported by the gdb/infrun.h header.
>

Thanks for letting me about these variables. Helps me in navigating it better.
 
> > 
> > I just have a doubt that from where the function 'print_no_history_reason()'
> > is actually called to execute(which I need to find some way by which I can
> > check if the inferior reached the end while going backwards or forwards),
> > but I am unable to find it right now.
> 
> GDB deals with notifying the user of anything by using interps (I think
> that's short for interpreters). For example, MI, CLI and TUI all have
> interps which will know how to notify the user.
> When we reach the end of history, "interps_notify_no_history" is called,
> which gets the interpreters to generate their no_history warnings. For
> example, check the gdb/cli/cli-interp.c

Thanks a ton for letting me know about this.
Comment 6 Sourceware Commits 2024-08-26 13:36:05 UTC
The master branch has been updated by Guinevere Larsen <blarsen@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=089197010993b3a5dc50bf882470bab2de696d92

commit 089197010993b3a5dc50bf882470bab2de696d92
Author: Guinevere Larsen <blarsen@redhat.com>
Date:   Mon Aug 26 10:33:17 2024 -0300

    Change message when reaching end of reverse history.
    
    In a record session, when we move backward, GDB switches from normal
    execution to simulation. Moving forward again, the emulation continues
    until the end of the reverse history. When the end is reached, the
    execution stops, and a warning message is shown. This message has been
    modified to indicate that the forward emulation has reached the end, but
    the execution can continue as normal, and the recording will also continue.
    
    Before this patch, the warning message shown in that case was the same as
    in the reverse case. This meant that when the end of history was reached in
    either backward or forward emulation, the same message was displayed:
    
    "No more reverse-execution history."
    
    This message has changed for these two cases. Backward emulation:
    
    "Reached end of recorded history; stopping.
    Backward execution from here not possible."
    
    Forward emulation:
    
    "Reached end of recorded history; stopping.
    Following forward execution will be added to history."
    
    The reason for this change is that the initial message was deceiving, for
    the forward case, making the user believe that forward debugging could not
    continue.
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31224
    Reviewed-By: Markus T. Metzger <markus.t.metzger@intel.com> (btrace)
    Approved-By: Guinevere Larsen <blarsen@redhat.com>
Comment 7 Guinevere Larsen 2024-08-26 13:44:47 UTC
Fixed by the previously mentioned commit

The commit is authored by Alex Chronopoulos, not me, I just messed up some git commands when pushing for him.