[PATCH] gdb: invalidate register cache after monitor commands

[PATCH] gdb: invalidate register cache after monitor commands

[Jerry Zhang Jian]

A monitor command is opaque to GDB: the stub can halt, resume, or
reset the target behind GDB's back, even if it later reports an
error, and the remote protocol has no way to tell GDB that happened.
For example, "monitor reset halt" was leaving GDB reporting the
pre-reset $pc until a later step/continue forced a refetch.

Invalidate the register cache after every monitor command via
SCOPE_EXIT, so it still runs on the error path.  Scope it to the
inferior's own process_stratum_target, matching registers_changed_thread()
and the target_wait()/target_stop() lookup pattern elsewhere in this
file, rather than wiping every inferior's cache with registers_changed().
Hold a strong reference to the target across the call in case it gets
unpushed/detached, the same idiom used in target_detach().

Signed-off-by: Jerry Zhang Jian <jerry.zhangjian@sifive.com>
---
 gdb/target.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/gdb/target.c b/gdb/target.c
index 5d937f3ae85..5c4684d81b5 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -4262,6 +4262,18 @@ default_rcmd (struct target_ops *self, const char *command,
 static void
 do_monitor_command (const char *cmd, int from_tty)
 {
+  process_target_ops_ref proc_target_ref;
+  if (process_stratum_target *proc_target
+	= current_inferior ()->process_target ())
+    proc_target_ref = process_target_ops_ref::new_reference (proc_target);
+
+  /* Monitor commands may change target state behind GDB's back.  */
+  SCOPE_EXIT
+    {
+      if (proc_target_ref != nullptr)
+	registers_changed_ptid (proc_target_ref.get (), minus_one_ptid);
+    };
+
   target_rcmd (cmd, gdb_stdtarg);
 }
 
-- 
2.53.0

Re: [PATCH] gdb: invalidate register cache after monitor commands

[Andrew Burgess]

Jerry Zhang Jian <jerry.zhangjian@sifive.com> writes:

> A monitor command is opaque to GDB: the stub can halt, resume, or
> reset the target behind GDB's back, even if it later reports an
> error, and the remote protocol has no way to tell GDB that happened.

I don't find any of these example particularly clear.  They all kind of
hint towards a problem, but it would be nice to have at least one fully
explained case.

Take "halt".  Do you mean the target is running in async mode, but GDB's
"interrupt" command doesn't do what you need?  Or does "halt" mean
something different in this context?

Or "resume".  Why would GDB's normal resumption commands not be
sufficient?  If you resume the inferior via a monitor command that's
going to leave GDB thinking the inferior is stopped when it's actually
running, that's going to break you debug session, right?

The "reset" example is by far the most obvious.  The target is stopped
an you want to restore it to some initial state.  It's still stopped,
but the register state has changed.

> For example, "monitor reset halt" was leaving GDB reporting the
> pre-reset $pc until a later step/continue forced a refetch.

Do you mean "monitor reset halt" here?  Even if you do due to some
detail of your specific setup, is this really needed in the commit
message?  Wouldn't it be clearer just to pretend that the command was
"monitor reset" as it feels (to me) like it is more obvious what this
means.

>
> Invalidate the register cache after every monitor command via
> SCOPE_EXIT, so it still runs on the error path.  Scope it to the
> inferior's own process_stratum_target, matching registers_changed_thread()
> and the target_wait()/target_stop() lookup pattern elsewhere in this
> file, rather than wiping every inferior's cache with registers_changed().
> Hold a strong reference to the target across the call in case it gets
> unpushed/detached, the same idiom used in target_detach().

That all makes sense.

>
> Signed-off-by: Jerry Zhang Jian <jerry.zhangjian@sifive.com>

Please remove the 'Signed-off-by' tag, these are not used by GDB right
now, but might be in the future.

> ---
>  gdb/target.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/gdb/target.c b/gdb/target.c
> index 5d937f3ae85..5c4684d81b5 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -4262,6 +4262,18 @@ default_rcmd (struct target_ops *self, const char *command,
>  static void
>  do_monitor_command (const char *cmd, int from_tty)
>  {
> +  process_target_ops_ref proc_target_ref;
> +  if (process_stratum_target *proc_target
> +	= current_inferior ()->process_target ())

This treats a pointer as a bool.  It would be clearer to just split the
assignment out from the `if` and just have the `if` condition be
`proc_target != nullptr`.

Thanks,
Andrew

> +    proc_target_ref = process_target_ops_ref::new_reference (proc_target);
> +
> +  /* Monitor commands may change target state behind GDB's back.  */
> +  SCOPE_EXIT
> +    {
> +      if (proc_target_ref != nullptr)
> +	registers_changed_ptid (proc_target_ref.get (), minus_one_ptid);
> +    };
> +
>    target_rcmd (cmd, gdb_stdtarg);
>  }
>  
> -- 
> 2.53.0

Re: [PATCH] gdb: invalidate register cache after monitor commands

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> I don't find any of these example particularly clear.  They all kind of
Andrew> hint towards a problem, but it would be nice to have at least one fully
Andrew> explained case.

I wonder also if this conceptually conflicts with your patch "avoid
switching threads for send_packet where possible".  In that patch, you
mention an unwinder sending a remote packet during unwinding.  If that
packet happens to be a 'monitor' command, then presumably something bad
will happen due to flushing the register cache while unwinding.

Tom

Re: [PATCH] gdb: invalidate register cache after monitor commands

[Jerry Zhang Jian]

[-- Attachment #1: Type: text/plain, Size: 2205 bytes --]

Hi Andrew and Tom,

Thanks for the review and for raising the interaction with the related
remote-packet change.

Andrew, you are right that the examples in the commit message were too
broad. The concrete case behind this change is monitor reset halt: after
GDB has cached a pre-reset PC, the target is reset and halted, but an
immediate read of $pc can still return the old pre-reset value. A
subsequent step or continue causes GDB to refetch the register and reveals
the reset-vector PC, showing that the target reset succeeded and only GDB's
register cache was stale.

I will simplify the commit message in v2 to focus on this reset case and
explain that the cache invalidation is needed because an opaque monitor
command can change target state without GDB receiving a protocol-level
notification.

Tom, I checked the interaction with the related remote-packet change. This
patch invalidates the register cache only after the CLI monitor command
completes, so it does not run in the middle of an unrelated packet send or
unwinding operation. Based on the current call paths, I do not think the
two patches directly conflict, but I will make this scope explicit in v2
and double-check whether monitor packets sent through other paths need
separate handling.

I will also make the other requested cleanup changes:

remove the Signed-off-by line;
split the assignment out of the if and explicitly check proc_target !=
nullptr.

Thanks,
Jerry

Tom Tromey <tom@tromey.com> 於 2026年8月13日週四 上午4:34寫道：

> >>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> I don't find any of these example particularly clear.  They all
> kind of
> Andrew> hint towards a problem, but it would be nice to have at least one
> fully
> Andrew> explained case.
>
> I wonder also if this conceptually conflicts with your patch "avoid
> switching threads for send_packet where possible".  In that patch, you
> mention an unwinder sending a remote packet during unwinding.  If that
> packet happens to be a 'monitor' command, then presumably something bad
> will happen due to flushing the register cache while unwinding.
>
> Tom
>
>

[-- Attachment #2: Type: text/html, Size: 2666 bytes --]

Re: [PATCH] gdb: invalidate register cache after monitor commands

[Tom Tromey]

>>>>> "Jerry" == Jerry Zhang Jian <jerry.zhangjian@sifive.com> writes:

Jerry> Tom, I checked the interaction with the related remote-packet
Jerry> change. This patch invalidates the register cache only after the
Jerry> CLI monitor command completes, so it does not run in the middle
Jerry> of an unrelated packet send or unwinding operation.

I mean I suppose the unwinder in theory could invoke a 'monitor'
command.

Though I suppose we could tell people they have to send a raw packet
instead; which isn't so bad since anything like this is pretty
weird/extreme anyway.

Tom