[PATCH v2 0/1] gdb: fix assert after register write failure

[PATCH v2 0/1] gdb: fix assert after register write failure

[Stephan Rohr]

From: "Rohr, Stephan" <stephan.rohr@intel.com>

Hi all,

this is v2 of the patch.

v1 of the patch is available here:
https://sourceware.org/pipermail/gdb-patches/2026-October/230972.html

Guinevere provided feedback:
https://sourceware.org/pipermail/gdb-patches/2026-October/231104.html

Changes since v1:

  - Moved nullptr check to regcache::invalidate ().

Thanks
Stephan

Eduard Sargsyan (1):
  gdb: fix assert after register write failure

 gdb/regcache.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

-- 
2.43.0

________________________________________
Intel Deutschland GmbH 

Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany 

Tel: +49 (89) 99143-0 

www.intel.de 

Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman

Chairperson of the Supervisory Board: Sonja Pierer

Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

[PATCH vv2 1/1] gdb: fix assert after register write failure

[Stephan Rohr]

From: Eduard Sargsyan <eduard.sargsyan@intel.com>

GDB tries to invalidate register cache if write to register fails.  In case
when write was requested to be done for a remote inferior and connection with
remote is dead (so write request will fail) GDB cleans register caches and as
result setting register status fails with segmentation fault, because internal
pointers are not valid anymore and the unique_ptr with register status is
null.

This change adds check if register status cache is still valid before trying to
modify (set status to UNKNOWN).
---
 gdb/regcache.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gdb/regcache.c b/gdb/regcache.c
index 773e8ac37d0..723a2759d1c 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -329,7 +329,8 @@ void
 reg_buffer::invalidate (int regnum)
 {
   assert_regnum (regnum);
-  m_register_status[regnum] = REG_UNKNOWN;
+  if (m_register_status != nullptr)
+    m_register_status[regnum] = REG_UNKNOWN;
 }
 
 void
-- 
2.43.0

________________________________________
Intel Deutschland GmbH 

Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany 

Tel: +49 (89) 99143-0 

www.intel.de 

Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman

Chairperson of the Supervisory Board: Sonja Pierer

Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [PATCH vv2 1/1] gdb: fix assert after register write failure

[Tom Tromey]

>>>>> "Stephan" == Stephan Rohr <stephan.rohr@intel.com> writes:

Stephan> From: Eduard Sargsyan <eduard.sargsyan@intel.com>
Stephan> GDB tries to invalidate register cache if write to register fails.  In case
Stephan> when write was requested to be done for a remote inferior and connection with
Stephan> remote is dead (so write request will fail) GDB cleans register caches and as
Stephan> result setting register status fails with segmentation fault, because internal
Stephan> pointers are not valid anymore and the unique_ptr with register status is
Stephan> null.

Stephan> This change adds check if register status cache is still valid before trying to
Stephan> modify (set status to UNKNOWN).

I don't mind this patch but I do wonder about it a little.

I couldn't find where m_register_status is reset.  How does this part
work?  Like, what set this to 'nullptr'?

And I wonder about the contract of this object.  I understand wanting
this particular spot to do this check.  But there are many other things
that access m_register_status.  Do those need checks, in theory, to
preserve some invariant?  Or just invalidation?

thanks,
Tom

RE: [PATCH vv2 1/1] gdb: fix assert after register write failure

[Rohr, Stephan]

Hi Tom,

thanks for the feedback.

m_register_status is never reset in regcache.c itself.  The bug is exposed in
regcache::raw_write:

  1. An invalidator is setup in case of write failures:

      make_scope_exit ([&] { this->invalidate (regnum); })

  2. If the remote connection dies in "target_store_registers", the regcache is
      deleted.  This deletes the register buffers and the register status buffer.

  3. The "invalidate ()" call now sees the reset unique_ptr for the status array
       and crashes. 

I was able to manually reproduce the bug:

  $ gdbserver --once :2345 a.out
  $ gdb -q a.out
  (gdb) target remote :2345
  (gdb) break main
  (gdb) continue
  (gdb) info registers
  (gdb) info frame
  (gdb) shell kill -9 GDBSERVER_PID
  (gdb) set $rax = 1
  Fatal signal: Segmentation fault

I could reproduce on Ubuntu 24.04.  I think we could add a test for this.

An alternative for the nullptr check could be implemented by moving
the call of "target_store_registers ()" into a try block and invalidate
in "catch" if the error is not a "TARGET_CLOSE_ERROR".

Does this sound reasonable?

Thanks
Stephan

> -----Original Message-----
> From: Tom Tromey <tom@tromey.com>
> Sent: Monday, 19 October 2026 18:19
> To: Rohr, Stephan <stephan.rohr@intel.com>
> Cc: gdb-patches@sourceware.org; guinevere@redhat.com
> Subject: Re: [PATCH vv2 1/1] gdb: fix assert after register write failure
> 
> >>>>> "Stephan" == Stephan Rohr <stephan.rohr@intel.com> writes:
> 
> Stephan> From: Eduard Sargsyan <eduard.sargsyan@intel.com>
> Stephan> GDB tries to invalidate register cache if write to register fails.  In case
> Stephan> when write was requested to be done for a remote inferior and
> connection with
> Stephan> remote is dead (so write request will fail) GDB cleans register caches
> and as
> Stephan> result setting register status fails with segmentation fault, because
> internal
> Stephan> pointers are not valid anymore and the unique_ptr with register
> status is
> Stephan> null.
> 
> Stephan> This change adds check if register status cache is still valid before
> trying to
> Stephan> modify (set status to UNKNOWN).
> 
> I don't mind this patch but I do wonder about it a little.
> 
> I couldn't find where m_register_status is reset.  How does this part
> work?  Like, what set this to 'nullptr'?
> 
> And I wonder about the contract of this object.  I understand wanting
> this particular spot to do this check.  But there are many other things
> that access m_register_status.  Do those need checks, in theory, to
> preserve some invariant?  Or just invalidation?
> 
> thanks,
> Tom
________________________________________
Intel Deutschland GmbH 

Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany 

Tel: +49 (89) 99143-0 

www.intel.de 

Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman

Chairperson of the Supervisory Board: Sonja Pierer

Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [PATCH vv2 1/1] gdb: fix assert after register write failure

[Tom Tromey]

>>>>> Rohr, Stephan <stephan.rohr@intel.com> writes:

> m_register_status is never reset in regcache.c itself.  The bug is exposed in
> regcache::raw_write:

>   2. If the remote connection dies in "target_store_registers", the regcache is
>       deleted.  This deletes the register buffers and the register status buffer.

> I was able to manually reproduce the bug:

Thanks.  I reproduced it locally with your instructions.

I think the problem here is in step 2 -- the invalidator is accessing
'this' but this object has been destroyed.  That is, I set a breakpoint
where the invalidator is made and:

(top-gdb) p this
$1 = (regcache * const) 0x2f8f4d0

and then I set a breakpoint in ~regcache and "next"d over the call to
target_store_registers:

(top-gdb) break regcache::~regcache
Breakpoint 4 at 0xb9e722: regcache::~regcache. (2 locations)
(top-gdb) n
Thread 1 "gdb" hit Breakpoint 4.2, regcache::~regcache (this=0x2f8f4d0) at ../../binutils-gdb/gdb/regcache.h:370

... with the backtrace looking like:

(top-gdb) bt
#0  regcache::~regcache (this=0x2f8f4d0) at ../../binutils-gdb/gdb/regcache.h:370
[... lots of stuff ...]
#32 0x0000000000d75522 in target_store_registers (regcache=0x2f8f4d0, regno=0) at ../../binutils-gdb/gdb/target.c:3935
#33 0x0000000000b9818e in regcache::raw_write (this=0x2f8f4d0, regnum=0, src=...) at ../../binutils-gdb/gdb/regcache.c:889

I think what this means is that the invalidator here has a
use-after-free.  So I tend to think your patch also does this, and it
would be exposed by an ASAN build.

> An alternative for the nullptr check could be implemented by moving
> the call of "target_store_registers ()" into a try block and invalidate
> in "catch" if the error is not a "TARGET_CLOSE_ERROR".

> Does this sound reasonable?

I think so.  The key thing is that if 'this' is destroyed by the
operation, then it cannot be used again.  If TARGET_CLOSE_ERROR is the
only such case then that's fine.  It's possible the problem lies
elsewhere, I'm afraid I am not really certain.

thanks,
Tom