[1/1] gdb: fix assert after register write failure
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
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/ChangeLog:
2021-03-18 Eduard Sargsyan <eduard.sargsyan@intel.com>
* regcache.c (regcache::raw_write): Add check for pointer validity.
---
gdb/regcache.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
Comments
>>>>> Eduard Sargsyan <eduard.sargsyan@intel.com> writes:
> 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).
Could you say how to reproduce this? If it's an existing test case,
then that's fine; but I am wondering if it needs a new test.
Tom
On 2023-12-19 10:14, Eduard Sargsyan wrote:
> 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/ChangeLog:
> 2021-03-18 Eduard Sargsyan <eduard.sargsyan@intel.com>
>
> * regcache.c (regcache::raw_write): Add check for pointer validity.
> ---
> gdb/regcache.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/regcache.c b/gdb/regcache.c
> index 6140a05f02b..94ca89a58de 100644
> --- a/gdb/regcache.c
> +++ b/gdb/regcache.c
> @@ -882,8 +882,11 @@ regcache::raw_write (int regnum, gdb::array_view<const gdb_byte> src)
>
> /* Invalidate the register after it is written, in case of a
> failure. */
> - auto invalidator
> - = make_scope_exit ([&] { this->invalidate (regnum); });
> + auto invalidator = make_scope_exit ([&]
> + {
> + if (this->m_register_status != nullptr)
> + this->invalidate (regnum);
> + });
>
> target_store_registers (this, regnum);
>
So, there's no place I can see where the regcache will reset its
m_register_status field to nullptr. So my guess of what happens is that
when something wrong happens with the network connection inside
target_store_registers, the remote target closes, and somewhere in the
process, it destroys all regcaches bound to it (including the regcache
we are "in" right now). This calls m_register_status' destructor, which
resets the internal pointer to nullptr. When you reach the invalidator
code, the regcache has been deleted, so even accessing
this->m_register_status would be invalid, *this has been deleted.
If you run this with an ASan-enabled build, do you get a use-after-free
error?
Simon
@@ -882,8 +882,11 @@ regcache::raw_write (int regnum, gdb::array_view<const gdb_byte> src)
/* Invalidate the register after it is written, in case of a
failure. */
- auto invalidator
- = make_scope_exit ([&] { this->invalidate (regnum); });
+ auto invalidator = make_scope_exit ([&]
+ {
+ if (this->m_register_status != nullptr)
+ this->invalidate (regnum);
+ });
target_store_registers (this, regnum);