[PATCH] gdb/record: fix missing "no history" message in some situations

[PATCH] gdb/record: fix missing "no history" message in some situations

[Guinevere Larsen]

I recently noticed that the end of recoded history message wasn't
shown when replaying an inferior until end end for the first time, it
was only shown when trying to reverse past the start of
history. Executing forward past the end will not warn the user that
recording is going to start again, it just does so.

This was happening because the out-of-history was only set when it
caused the execution loop to stop, so using "next" to reach the end of
history, for example, wouldn't trigger it as I expected.  This commit
fixes that to make out-of-history check only happen after the loop is
finished.
---
 gdb/record-full.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/gdb/record-full.c b/gdb/record-full.c
index 26bd85d34f6..c02bc39f12c 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -1362,19 +1362,11 @@ record_full_wait_1 (struct target_ops *ops,
 	      /* Check for beginning and end of log.  */
 	      if (execution_direction == EXEC_REVERSE
 		  && record_full_next_insn < 0)
-		{
-		  /* Hit beginning of record log in reverse.  */
-		  status->set_no_history ();
-		  record_full_next_insn = 0;
 		  break;
-		}
-	      if (execution_direction != EXEC_REVERSE
+
+	      else if (execution_direction != EXEC_REVERSE
 		  && record_full_next_insn == record_full_log.size ())
-		{
-		  /* Hit end of record log going forward.  */
-		  status->set_no_history ();
 		  break;
-		}
 
 	      record_full_log[record_full_next_insn].exec_insn (regcache);
 
@@ -1424,11 +1416,13 @@ record_full_wait_1 (struct target_ops *ops,
 	    {
 	      gdb_assert (execution_direction == EXEC_REVERSE);
 	      record_full_next_insn = 0;
+	      status->set_no_history ();
 	    }
-	  else if (record_full_next_insn > record_full_log.size ())
+	  else if (record_full_next_insn >= record_full_log.size ())
 	    {
 	      gdb_assert (execution_direction == EXEC_FORWARD);
 	      record_full_next_insn = record_full_log.size ();
+	      status->set_no_history ();
 	    }
 	  /* Reset the current instruction to point to the one to be replayed
 	     moving forward.  */

base-commit: 0ca0b8de31ec76ff71f4a051db3dc1fcc9325154
-- 
2.55.0

Re: [PATCH] gdb/record: fix missing "no history" message in some situations

[Simon Marchi]

On 9/8/26 9:14 AM, Guinevere Larsen wrote:
> I recently noticed that the end of recoded history message wasn't

recoded -> recorded

> shown when replaying an inferior until end end for the first time, it

"until end end"?

> was only shown when trying to reverse past the start of
> history. Executing forward past the end will not warn the user that
> recording is going to start again, it just does so.
> 
> This was happening because the out-of-history was only set when it
> caused the execution loop to stop, so using "next" to reach the end of
> history, for example, wouldn't trigger it as I expected.  This commit
> fixes that to make out-of-history check only happen after the loop is
> finished.

Can you give examples of "before" and "after" GDB sessions so that it's
more obvious what the change is?

Could this use a test?

> ---
>  gdb/record-full.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/gdb/record-full.c b/gdb/record-full.c
> index 26bd85d34f6..c02bc39f12c 100644
> --- a/gdb/record-full.c
> +++ b/gdb/record-full.c
> @@ -1362,19 +1362,11 @@ record_full_wait_1 (struct target_ops *ops,
>  	      /* Check for beginning and end of log.  */
>  	      if (execution_direction == EXEC_REVERSE
>  		  && record_full_next_insn < 0)
> -		{
> -		  /* Hit beginning of record log in reverse.  */
> -		  status->set_no_history ();
> -		  record_full_next_insn = 0;
>  		  break;
> -		}
> -	      if (execution_direction != EXEC_REVERSE
> +
> +	      else if (execution_direction != EXEC_REVERSE
>  		  && record_full_next_insn == record_full_log.size ())

The indentation of this && line needs to be adjusted.

> -		{
> -		  /* Hit end of record log going forward.  */
> -		  status->set_no_history ();
>  		  break;
> -		}

The indentation of those "break" is wrong, you could fix it while at it.

Simon

Re: [PATCH] gdb/record: fix missing "no history" message in some situations

[Guinevere Larsen]

On 9/8/26 12:18 PM, Simon Marchi wrote:
> On 9/8/26 9:14 AM, Guinevere Larsen wrote:
>> I recently noticed that the end of recoded history message wasn't
> recoded -> recorded
>
>> shown when replaying an inferior until end end for the first time, it
> "until end end"?
>
>> was only shown when trying to reverse past the start of
>> history. Executing forward past the end will not warn the user that
>> recording is going to start again, it just does so.
>>
>> This was happening because the out-of-history was only set when it
>> caused the execution loop to stop, so using "next" to reach the end of
>> history, for example, wouldn't trigger it as I expected.  This commit
>> fixes that to make out-of-history check only happen after the loop is
>> finished.
> Can you give examples of "before" and "after" GDB sessions so that it's
> more obvious what the change is?
>
> Could this use a test?

Thanks for the quick review!

I fixed all the nits and added the test, which was easier than I 
thought. v2 is on the way

>
>> ---
>>   gdb/record-full.c | 16 +++++-----------
>>   1 file changed, 5 insertions(+), 11 deletions(-)
>>
>> diff --git a/gdb/record-full.c b/gdb/record-full.c
>> index 26bd85d34f6..c02bc39f12c 100644
>> --- a/gdb/record-full.c
>> +++ b/gdb/record-full.c
>> @@ -1362,19 +1362,11 @@ record_full_wait_1 (struct target_ops *ops,
>>   	      /* Check for beginning and end of log.  */
>>   	      if (execution_direction == EXEC_REVERSE
>>   		  && record_full_next_insn < 0)
>> -		{
>> -		  /* Hit beginning of record log in reverse.  */
>> -		  status->set_no_history ();
>> -		  record_full_next_insn = 0;
>>   		  break;
>> -		}
>> -	      if (execution_direction != EXEC_REVERSE
>> +
>> +	      else if (execution_direction != EXEC_REVERSE
>>   		  && record_full_next_insn == record_full_log.size ())
> The indentation of this && line needs to be adjusted.
>
>> -		{
>> -		  /* Hit end of record log going forward.  */
>> -		  status->set_no_history ();
>>   		  break;
>> -		}
> The indentation of those "break" is wrong, you could fix it while at it.
>
> Simon
>
-- 
Cheers,
Guinevere Larsen
it/its
she/her (deprecated)