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

[Tom Tromey <tom@tromey.com>]

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