diff options
author | Pierre Langlois <pierre.langlois@embecosm.com> | 2014-05-20 15:13:20 +0100 |
---|---|---|
committer | Pierre Langlois <pierre.langlois@embecosm.com> | 2014-06-13 10:57:12 +0100 |
commit | b94ade42840d1b0fbf818db49d98da9ba80c1d53 (patch) | |
tree | 2e8640b65349e0c08442fbb1e94f08381b92109a /gdb/regcache.c | |
parent | d495ab0d843def702a6641fa4fc31708d7fc97b1 (diff) | |
download | binutils-b94ade42840d1b0fbf818db49d98da9ba80c1d53.zip binutils-b94ade42840d1b0fbf818db49d98da9ba80c1d53.tar.gz binutils-b94ade42840d1b0fbf818db49d98da9ba80c1d53.tar.bz2 |
Invalidate a register in cache when a remote target failed to write it.
As shown by the bug report, GDB crashes when the remote target was unable to
write to a register (the program counter) with the 'P' packet. This was reported
for AVR but can be reproduced on any architecture with a gdbserver that fails to
handle a 'P' packet.
Issue
=====
This GDB session was done with a custom gdbserver patched to send an error
packet when trying to set the program counter with a 'P' packet:
~~~
(gdb) file Debug/ATMega2560-simple-program.elf
Reading symbols from Debug/ATMega2560-simple-program.elf...done.
(gdb) target remote :51000
Remote debugging using :51000
0x00000000 in __vectors ()
(gdb) load
Loading section .text, size 0x1fc lma 0x0
Start address 0x0, load size 508
Transfer rate: 248 KB/sec, 169 bytes/write.
(gdb) b main
Breakpoint 1 at 0x164: file .././ATMega2560-simple-program.c, line 39.
(gdb) c
Continuing.
Program received signal SIGTRAP, Trace/breakpoint trap.
main () at .././ATMega2560-simple-program.c:42
42 DDRD |= LED0_MASK;// | LED1_MASK;
(gdb) info line 43
Line 43 of ".././ATMega2560-simple-program.c" is at address 0x178 <main+40> but contains no code.
(gdb) set $pc=0x178
Could not write register "PC2"; remote failure reply 'E00'
(gdb) info registers pc
pc 0x178 0x178 <main+40>
(gdb) s
../../unisrc-mainline/gdb/infrun.c:1978: internal-error: resume: Assertion `pc_in_thread_step_range (pc, tp)' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n)
../../unisrc-mainline/gdb/infrun.c:1978: internal-error: resume: Assertion `pc_in_thread_step_range (pc, tp)' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Create a core file of GDB? (y or n)
~~~
We can see that even though GDB reports that writing to the register failed, the
register cache was updated:
~~~
(gdb) set $pc=0x178
Could not write register "PC2"; remote failure reply 'E00'
(gdb) info registers pc
pc 0x178 0x178 <main+40>
~~~
The root of the problem is of course in the gdbserver but I thought GDB should
keep a register cache consistent with the hardware even in case of a failure.
Changes
=======
This patch adds routines to add a regcache_invalidate cleanup to the current
chain.
We can then register one before calling target_store_registers. This way if the
target throws an error, the register we wanted to write to will be invalidated
in cache. If target_store_registers succeeds, we can discard the new cleanup.
2014-06-12 Pierre Langlois <pierre.langlois@embecosm.com>
* regcache.c (struct register_to_invalidate): New structure.
(do_register_invalidate, make_cleanup_regcache_invalidate): New
functions.
(regcache_raw_write): Call make_cleanup_regcache_invalidate.
Diffstat (limited to 'gdb/regcache.c')
-rw-r--r-- | gdb/regcache.c | 43 |
1 files changed, 40 insertions, 3 deletions
diff --git a/gdb/regcache.c b/gdb/regcache.c index 8b588c6..5ee90b0 100644 --- a/gdb/regcache.c +++ b/gdb/regcache.c @@ -267,6 +267,32 @@ make_cleanup_regcache_xfree (struct regcache *regcache) return make_cleanup (do_regcache_xfree, regcache); } +/* Cleanup routines for invalidating a register. */ + +struct register_to_invalidate +{ + struct regcache *regcache; + int regnum; +}; + +static void +do_regcache_invalidate (void *data) +{ + struct register_to_invalidate *reg = data; + + regcache_invalidate (reg->regcache, reg->regnum); +} + +static struct cleanup * +make_cleanup_regcache_invalidate (struct regcache *regcache, int regnum) +{ + struct register_to_invalidate* reg = XNEW (struct register_to_invalidate); + + reg->regcache = regcache; + reg->regnum = regnum; + return make_cleanup_dtor (do_regcache_invalidate, (void *) reg, xfree); +} + /* Return REGCACHE's architecture. */ struct gdbarch * @@ -846,7 +872,8 @@ void regcache_raw_write (struct regcache *regcache, int regnum, const gdb_byte *buf) { - struct cleanup *old_chain; + struct cleanup *chain_before_save_inferior; + struct cleanup *chain_before_invalidate_register; gdb_assert (regcache != NULL && buf != NULL); gdb_assert (regnum >= 0 && regnum < regcache->descr->nr_raw_registers); @@ -864,16 +891,26 @@ regcache_raw_write (struct regcache *regcache, int regnum, regcache->descr->sizeof_register[regnum]) == 0)) return; - old_chain = save_inferior_ptid (); + chain_before_save_inferior = save_inferior_ptid (); inferior_ptid = regcache->ptid; target_prepare_to_store (regcache); memcpy (register_buffer (regcache, regnum), buf, regcache->descr->sizeof_register[regnum]); regcache->register_status[regnum] = REG_VALID; + + /* Register a cleanup function for invalidating the register after it is + written, in case of a failure. */ + chain_before_invalidate_register + = make_cleanup_regcache_invalidate (regcache, regnum); + target_store_registers (regcache, regnum); - do_cleanups (old_chain); + /* The target did not throw an error so we can discard invalidating the + register and restore the cleanup chain to what it was. */ + discard_cleanups (chain_before_invalidate_register); + + do_cleanups (chain_before_save_inferior); } void |