In non-stop mode, or rather, breakpoints always-inserted mode, the
code cache can easily end up with stale breakpoint instructions:
All it takes is filling a cache line when breakpoints already exist in
that memory region, and then delete the breakpoint.
Vis. (from the new test):
(gdb) set breakpoint always-inserted on
(gdb) b 23
Breakpoint 2 at 0x400540: file ../../../src/gdb/testsuite/gdb.base/breakpoint-shadow.c, line 23.
(gdb) b 24
Breakpoint 3 at 0x400547: file ../../../src/gdb/testsuite/gdb.base/breakpoint-shadow.c, line 24.
disass main
Dump of assembler code for function main:
0x000000000040053c <+0>: push %rbp
0x000000000040053d <+1>: mov %rsp,%rbp
=> 0x0000000000400540 <+4>: movl $0x1,-0x4(%rbp)
0x0000000000400547 <+11>: movl $0x2,-0x4(%rbp)
0x000000000040054e <+18>: mov $0x0,%eax
0x0000000000400553 <+23>: pop %rbp
0x0000000000400554 <+24>: retq
End of assembler dump.
So far so good. Now flush the code cache:
(gdb) set code-cache off
(gdb) set code-cache on
Requesting a disassembly works as expected, breakpoint shadowing is
applied:
(gdb) disass main
Dump of assembler code for function main:
0x000000000040053c <+0>: push %rbp
0x000000000040053d <+1>: mov %rsp,%rbp
=> 0x0000000000400540 <+4>: movl $0x1,-0x4(%rbp)
0x0000000000400547 <+11>: movl $0x2,-0x4(%rbp)
0x000000000040054e <+18>: mov $0x0,%eax
0x0000000000400553 <+23>: pop %rbp
0x0000000000400554 <+24>: retq
End of assembler dump.
However, now delete the breakpoints:
(gdb) delete
Delete all breakpoints? (y or n) y
And disassembly shows the old breakpoint instructions:
(gdb) disass main
Dump of assembler code for function main:
0x000000000040053c <+0>: push %rbp
0x000000000040053d <+1>: mov %rsp,%rbp
=> 0x0000000000400540 <+4>: int3
0x0000000000400541 <+5>: rex.RB cld
0x0000000000400543 <+7>: add %eax,(%rax)
0x0000000000400545 <+9>: add %al,(%rax)
0x0000000000400547 <+11>: int3
0x0000000000400548 <+12>: rex.RB cld
0x000000000040054a <+14>: add (%rax),%al
0x000000000040054c <+16>: add %al,(%rax)
0x000000000040054e <+18>: mov $0x0,%eax
0x0000000000400553 <+23>: pop %rbp
0x0000000000400554 <+24>: retq
End of assembler dump.
Those breakpoint instructions are no longer installed in target memory
they're stale in the code cache. Easily confirmed by just disabling
the code cache:
(gdb) set code-cache off
(gdb) disass main
Dump of assembler code for function main:
0x000000000040053c <+0>: push %rbp
0x000000000040053d <+1>: mov %rsp,%rbp
=> 0x0000000000400540 <+4>: movl $0x1,-0x4(%rbp)
0x0000000000400547 <+11>: movl $0x2,-0x4(%rbp)
0x000000000040054e <+18>: mov $0x0,%eax
0x0000000000400553 <+23>: pop %rbp
0x0000000000400554 <+24>: retq
End of assembler dump.
I stumbled upon this when writing a patch to infrun.c, that made
handle_inferior_event & co fill in the cache before breakpoints were
removed from the target. Recall that wait_for_inferior flushes the
dcache for every event. So in that case, always-inserted mode was not
necessary to trigger this. It's just a convenient way to expose the
issue.
The dcache works at the raw memory level. We need to update it
whenever memory is written, no matter what kind of target memory
object was originally passed down by the caller. The issue is that
the dcache update code isn't reached when a caller explicitly writes
raw memory. Breakpoint insertion/removal is one such case --
mem-break.c uses target_write_read_memory/target_write_raw_memory.
The fix is to move the dcache update code from memory_xfer_partial_1
to raw_memory_xfer_partial so that it's always reachable.
When we do that, we can actually simplify a series of things.
memory_xfer_partial_1 no longer needs to handle writes for any kind of
memory object, and therefore dcache_xfer_memory no longer needs to
handle writes either. So the latter (dcache_xfer_memory) and its
callees can be simplified to only care about reads. While we're
touching dcache_xfer_memory's prototype, might as well rename it to
reflect that fact that it only handles reads, and make it follow the
new target_xfer_status/xfered_len style. This made me notice that
dcache_xfer_memory loses the real error status if a memory read fails:
we could have failed to read due to TARGET_XFER_E_UNAVAILABLE, for
instance, but we always return TARGET_XFER_E_IO, hence the FIXME note.
I felt that fixing that fell out of the scope of this patch.
Currently dcache_xfer_memory handles the case of a write failing. The
whole cache line is invalidated when that happens. However,
dcache_update, the sole mechanism for handling writes that will remain
after the patch, does not presently handle that scenario. That's a
bug. The patch makes it handle that, by passing down the
target_xfer_status status from the caller, so that it can better
decide what to do itself. While I was changing the function's
prototype, I constified the myaddr parameter, getting rid of the need
for the cast as seen in its existing caller.
Tested on x86_64 Fedora 17, native and gdbserver.
gdb/
2014-03-05 Pedro Alves <palves@redhat.com>
PR gdb/16575
* dcache.c (dcache_poke_byte): Constify ptr parameter. Return
void. Update comment.
(dcache_xfer_memory): Delete.
(dcache_read_memory_partial): New, based on the read bits of
dcache_xfer_memory.
(dcache_update): Add status parameter. Use ULONGEST for len, and
adjust. Discard cache lines if the reason for the update was
error.
* dcache.h (dcache_xfer_memory): Delete declaration.
(dcache_read_memory_partial): New declaration.
(dcache_update): Update prototype.
* target.c (raw_memory_xfer_partial): Update the dcache here.
(memory_xfer_partial_1): Don't handle dcache writes here.
gdb/testsuite/
2014-03-05 Pedro Alves <palves@redhat.com>
PR gdb/16575
* gdb.base/breakpoint-shadow.exp (compare_disassembly): New
procedure.
(top level): Adjust to use it. Add tests that exercise breakpoint
interaction with the code-cache.
Two modifications:
1. The addition of 2013 to the copyright year range for every file;
2. The use of a single year range, instead of potentially multiple
year ranges, as approved by the FSF.
* NEWS: Add note on new "set stack-cache" option.
* corefile.c (read_stack): New function.
* dcache.c (dcache_struct): New member ptid.
(dcache_enable_p): Mark as obsolete.
(show_dcache_enabled_p): Flag option as deprecated.
(dcache_invalidate): Update ptid.
(dcache_invalidate_line): New function.
(dcache_read_line): No longer check cacheable attribute, stack
accesses get cached despite attribute.
(dcache_init): Set ptid.
(dcache_xfer_memory): Flush cache if from different ptid than before.
Update cache after write.
(dcache_update): New function.
(dcache_info): Report ptid.
(_initialize_dcache): Update text for `remotecache' to indicate it
is obsolete.
* dcache.h (dcache_update): Declare.
* dwarf2loc.c (dwarf2_evaluate_loc_desc): Mark values on stack with
set_value_stack.
* frame-unwind.c (frame_unwind_got_memory): Ditto.
* gdbcore.h (read_stack): Declare.
* memattr.c (mem_enable_command): Call target_dcache_invalidate
instead of dcache_invalidate.
(mem_disable_command, mem_delete_command): Ditto.
* target.c (stack_cache_enabled_p_1): New static global.
(stack_cache_enabled_p): New static global.
(set_stack_cache_enabled_p): New function.
(show_stack_cache_enabled_p): New function.
(target_dcache): Make static.
(target_dcache_invalidate): New function.
(target_load, target_resume): Call target_dcache_invalidate
instead of dcache_invalidate.
(memory_xfer_partial): New arg object, all callers updated.
Check for existing inferior before calling dcache routines.
When writing non-TARGET_OBJECT_STACK_MEMORY, notify dcache.
(target_xfer_partial): Call memory_xfer_partial for
TARGET_OBJECT_STACK_MEMORY.
(target_read_stack): New function.
(initialize_targets): Install new option `stack-cache'.
* target.h: Remove #include of dcache.h.
(enum target_object): New value TARGET_OBJECT_STACK_MEMORY.
(target_dcache): Delete.
(target_dcache_invalidate): Declare.
(target_read_stack): Declare.
* top.c (prepare_execute_command): New function.
(execute_command): Call prepare_execute_command
instead of free_all_values.
* top.h (prepare_execute_command): Declare.
* valops.c (get_value_at): New function.
(value_at): Guts moved to get_value_at.
(value_at_lazy): Similarly.
(value_fetch_lazy): Call read_stack for stack values.
* value.c (struct value): New member `stack'.
(value_stack, set_value_stack): New functions.
* value.h (value_stack, set_value_stack): Declare.
* mi/mi-main.c (mi_cmd_execute): Call prepare_execute_command
instead of free_all_values.
doc/
* gdb.texinfo (Caching Data of Remote Targets): Update text.
Mark `set/show remotecache' options as obsolete.
Document new `set/show stack-cache' option.
Update text for `info dcache'.
Remove partially implemented writeback support.
* dcache.c: Include splay-tree.h.
(LINE_SIZE_POWER): Change from 5 to 6.
(DCACHE_SIZE): Change from 64 to 4096.
(ENTRY_INVALID, ENTRY_VALID, ENTRY_DIRTY): Delete.
(state_chars): Delete.
(struct dcache_block): Clean up; remove state and anydirty fields.
(struct dcache_struct): Redefine as a splay tree and linked list.
(last_cache): Make static.
(dcache_invalidate, dcache_hit): Rewrite for new cache structure.
(dcache_read_line, dcache_alloc): Rewrite for new cache structure.
(dcache_write_line): Delete.
(dcache_writeback): Delete.
(dcache_peek_byte): Clean up; remove "invalid" state check.
(dcache_poke_byte): Rewrite for new cache structure; clarify comment.
(dcache_splay_tree_compare): New function.
(dcache_init, dcache_free): Rewrite for new cache structure.
(dcache_xfer_memory): Rewrite for new write-through cache structure.
(dcache_print_line): New function.
(dcache_info): Rewrite for new cache structure.
(_initialize_dcache): Update "info dcache" help text.
* dcache.h (dcache_xfer_memory): Update declaration.
* target.c (memory_xfer_partial): Update calls to dcache_xfer_memory.
* infptrace.c (child_xfer_memory): Likewise.
* monitor.c (monitor_xfer_memory): Likewise.
* remote-adapt.c (adapt_xfer_inferior_memory): Likewise.
* remote-array.c (array_xfer_memory): Likewise.
* remote-bug.c (bug_xfer_memory): Likewise.
* remote-e7000.c (e7000_xfer_inferior_memory): Likewise.
* remote-eb.c (eb_xfer_inferior_memory): Likewise.
* remote-es.c (es1800_xfer_inferior_memory): Likewise.
* remote-mips.c (mips_xfer_memory): Likewise.
* remote-mm.c (mm_xfer_inferior_memory): Likewise.
* remote-nindy.c (nindy_xfer_inferior_memory): Likewise.
* remote-os9k.c (rombug_xfer_inferior_memory): Likewise.
* remote-rdi.c (arm_rdi_xfer_memory): Likewise.
* remote-rdp.c (remote_rdp_xfer_inferior_memory): Likewise.
* remote-sds.c (sds_xfer_memory): Likewise.
* remote-sim.c (gdbsim_xfer_inferior_memory): Likewise.
* remote-st.c (st2000_xfer_inferior_memory): Likewise.
* remote-udi.c (udi_xfer_inferior_memory): Likewise.
* remote-vx.c (vx_xfer_memory): Likewise.
* remote.c (remote_xfer_memory): Likewise.
* target.c (debug_to_xfer_memory, do_xfer_memory): Likewise.
* target.h (child_xfer_memory, do_xfer_memory, xfer_memory): Likewise.
* target.h (#include "memattr.h"): Added.
(target_ops.to_xfer_memory): Add attrib argument.
* wince.c (_initialize_inftarg): Removed call to set_dcache_state.
* dcache.h (set_dcache_state): Removed declaration.
* dcache.c (set_dcache_state): Removed definition
* dcache.c: Update module comment, as dcache is now enabled and
disabled with memory region attributes instead of by the global
variable "remotecache". Add comment describing the interaction
between dcache and memory region attributes.
(dcache_xfer_memory): Add comment describing benefits of moving
cache writeback to a higher level.
(dcache_struct): Removed cache_has_stuff field. This was used to
record whether the cache had been accessed in order to invalidate
it when it was disabled. However, this is not needed because the
cache is write through and the code that enables, disables, and
deletes memory regions invalidate the cache. Add comment which
suggests that we could be more selective and only invalidate those
cache lines containing data from those memory regions.
(dcache_invalidate): Updated.
(dcache_xfer_memory): Updated.
(dcache_alloc): Don't abort() if dcache_enabled_p is clear.
(dcache_xfer_memory): Removed code that called do_xfer_memory() to
perform a uncached transfer if dcache_enabled_p was clear. This
function is now only called if caching is enabled for the memory
region.
(dcache_info): Always print cache info.
* target.c (do_xfer_memory): Add attrib argument.
(target_xfer_memory, target_xfer_memory_partial): Break transfer
into chunks defined by memory regions, pass region attributes to
do_xfer_memory().
* dcache.c (dcache_read_line, dcache_write_line): Likewise.
* Makefile.in (SFILES): Add memattr.c.
(COMMON_OBS): Add memattr.o.
(dcache.o): Add target.h to dependencies.
* memattr.c: New file.
* memattr.h: Likewise.
(nindy_xfer_inferior_memory): Use dcache_xfer_memory() instead of
breaking transfer into chunks and using nindy_fetch_word() and
nindy_store_word().
* remote-bug.c (bug_xfer_memory): Use dcache_xfer_memory() instead
of breaking transfer into chunks and using gr_fetch_word() and
gr_store_word().
* remote.c (remote_fetch_word, remote_store_word): Removed.
* remote-utils.h (gr_fetch_word, gr_store_word): Removed.
* remote-utils.c (gr_fetch_word, gr_store_word): Removed.
* dcache.h (dcache_fetch, dcache_poke, dcache_poke_block): Removed.
* dcache.c (dcache_fetch, dcache_poke): Removed.
should be char *, not unsigned char *.
* dcache.h (memxferfunc): Ditto.
* monitor.c (monitor_write_memory, monitor_read_memory_single):
Ditto.
(monitor_make_srec): Let compiler figure size of hextab.
* monitor.c: Turn on caching.
(monitor_printf): If a ^C was sent, don't expect to see its
echo.
(monitor_open): Enable caching.
(monitor_resume, monitor_load): Flush cache.
(monitor_xfer_memory): Call cache routine.
(monitor_dump_regs): New.
(monitor_fetch_registers): If monitor_dump_regs available
then use it.
(monitor_load): Don't ref exec_bfd if it's NULL.
(monitor_load_srec): Use new monitor_make_srec calling convention.
(monitor_make_srec): Rewrite to cope with two, three and four byte
addresses.
* remote-hms.c (hms_cmds): Initialze end-of-command delim.
* dcache.h, dcache.h: Rewritten.
* remote.c: Reenable caching.
(getpkt): Reduce MAX_TRIES to 3.
(remote_xfer_memory): Use dcache_xfer_memory.
* defs.h (error_hook): New.
* top.c (error_hook): New definition.
* utils.c (error): Use error_hook if initialized.
* sparcl-tdep.c (HAVE_SOCKETS): Don't define if GO32 or WIN32. Use
HAVE_SOCKETS in place of #ifndef GO32.
Joel Brobecker