breakpoint shadowing, take single-step breakpoints into account.
authorPedro Alves <palves@redhat.com>
Thu, 10 Apr 2014 13:19:52 +0000 (14:19 +0100)
committerPedro Alves <palves@redhat.com>
Thu, 10 Apr 2014 13:19:52 +0000 (14:19 +0100)
Breakpoints are supposed to be transparent to memory accesses.  For
all kinds of breakpoints breakpoint_xfer_memory hides the breakpoint
instructions.  However, sss breakpoints aren't tracked like all other
breakpoints, and nothing is taking care of hiding them from memory
reads.

Say, as is, a background step + disassemble will see breakpoints
instructions on software step targets.  E.g., stepping over this line:

  while (1);

with s&

and then "disassemble" would show sss breakpoints.

Actually, that's still not be possible to see today, because:

 - in native Linux, you can't read memory while the program
   is running.
 - with Linux gdbserver, you can, but in the all-stop RSP you
   can't talk to the server while the program is running...
 - and with non-stop, on software step targets, we presently
   force the use of displaced-stepping for all single-steps,
   so no single-step breakpoints are used...

I've been working towards making non-stop not force displaced stepping
on sss targets, and I noticed the issue then.  With that, I indeed see
this:

(gdb) set remote Z-packet off
(gdb) s&
(gdb) disassemble main
Dump of assembler code for function main:
   0x000000000040049c <+0>:     push   %rbp
   0x000000000040049d <+1>:     mov    %rsp,%rbp
   0x00000000004004a0 <+4>:     int3
   0x00000000004004a1 <+5>:     (bad)
End of assembler dump.

Instead of the correct:

(gdb) disassemble main
Dump of assembler code for function main:
   0x000000000040049c <+0>:     push   %rbp
   0x000000000040049d <+1>:     mov    %rsp,%rbp
   0x00000000004004a0 <+4>:     jmp    0x4004a0 <main+4>

This is actually one thing that my v1 of the recent "fix a bunch of
run control bugs" series was fixing, because it made sss breakpoints
be regular breakpoints in the breakpoint chain.  But dropped it in the
version that landed in the tree, due to some problems.

So instead of making sss breakpoints regular breakpoints, go with a
simpler fix (at least for now) -- make breakpoint_xfer_memory take
software single-step breakpoints into account.  After the patch, I get
the correct disassemble output.

Tested on x86_64 Fedora 17, and also on top of my "use software
single-step on x86" series.

Also fixes the issue pointed out by Yao at
https://sourceware.org/ml/gdb-patches/2014-04/msg00045.html, where the
prologue analysis/frame sniffing manages to see software step
breakpoint instructions.

gdb/
2014-04-10  Pedro Alves  <palves@redhat.com>

* breakpoint.c (single_step_breakpoints)
(single_step_gdbarch): Move up in the file.
(one_breakpoint_xfer_memory): New function, factored out from ...
(breakpoint_xfer_memory): ... here.  Also process single-step
breakpoints.

gdb/ChangeLog
gdb/breakpoint.c

index 880c37a5ee56b6109782b9685d6f1313360aa055..45a94a87e9ff3ddf6af69e3227885499fe911997 100644 (file)
@@ -1,3 +1,11 @@
+2014-04-10  Pedro Alves  <palves@redhat.com>
+
+       * breakpoint.c (single_step_breakpoints)
+       (single_step_gdbarch): Move up in the file.
+       (one_breakpoint_xfer_memory): New function, factored out from ...
+       (breakpoint_xfer_memory): ... here.  Also process single-step
+       breakpoints.
+
 2014-04-09  Tristan Gingold  <gingold@adacore.com>
 
        * darwin-nat.c (darwin_check_new_threads): Fix port leak, add
index 19af9df6d4d13f062b4053ba56d8bbb8a58f2a2b..f777a4a62cf429038e4a404b9f089d89769ca227 100644 (file)
@@ -293,6 +293,12 @@ static struct breakpoint_ops bkpt_probe_breakpoint_ops;
 /* Dynamic printf class type.  */
 struct breakpoint_ops dprintf_breakpoint_ops;
 
+/* One (or perhaps two) breakpoints used for software single
+   stepping.  */
+
+static void *single_step_breakpoints[2];
+static struct gdbarch *single_step_gdbarch[2];
+
 /* The style in which to perform a dynamic printf.  This is a user
    option because different output options have different tradeoffs;
    if GDB does the printing, there is better error handling if there
@@ -1409,6 +1415,100 @@ bp_location_has_shadow (struct bp_location *bl)
   return 1;
 }
 
+/* Update BUF, which is LEN bytes read from the target address
+   MEMADDR, by replacing a memory breakpoint with its shadowed
+   contents.
+
+   If READBUF is not NULL, this buffer must not overlap with the of
+   the breakpoint location's shadow_contents buffer.  Otherwise, a
+   failed assertion internal error will be raised.  */
+
+static void
+one_breakpoint_xfer_memory (gdb_byte *readbuf, gdb_byte *writebuf,
+                           const gdb_byte *writebuf_org,
+                           ULONGEST memaddr, LONGEST len,
+                           struct bp_target_info *target_info,
+                           struct gdbarch *gdbarch)
+{
+  /* Now do full processing of the found relevant range of elements.  */
+  CORE_ADDR bp_addr = 0;
+  int bp_size = 0;
+  int bptoffset = 0;
+
+  if (!breakpoint_address_match (target_info->placed_address_space, 0,
+                                current_program_space->aspace, 0))
+    {
+      /* The breakpoint is inserted in a different address space.  */
+      return;
+    }
+
+  /* Addresses and length of the part of the breakpoint that
+     we need to copy.  */
+  bp_addr = target_info->placed_address;
+  bp_size = target_info->shadow_len;
+
+  if (bp_addr + bp_size <= memaddr)
+    {
+      /* The breakpoint is entirely before the chunk of memory we are
+        reading.  */
+      return;
+    }
+
+  if (bp_addr >= memaddr + len)
+    {
+      /* The breakpoint is entirely after the chunk of memory we are
+        reading.  */
+      return;
+    }
+
+  /* Offset within shadow_contents.  */
+  if (bp_addr < memaddr)
+    {
+      /* Only copy the second part of the breakpoint.  */
+      bp_size -= memaddr - bp_addr;
+      bptoffset = memaddr - bp_addr;
+      bp_addr = memaddr;
+    }
+
+  if (bp_addr + bp_size > memaddr + len)
+    {
+      /* Only copy the first part of the breakpoint.  */
+      bp_size -= (bp_addr + bp_size) - (memaddr + len);
+    }
+
+  if (readbuf != NULL)
+    {
+      /* Verify that the readbuf buffer does not overlap with the
+        shadow_contents buffer.  */
+      gdb_assert (target_info->shadow_contents >= readbuf + len
+                 || readbuf >= (target_info->shadow_contents
+                                + target_info->shadow_len));
+
+      /* Update the read buffer with this inserted breakpoint's
+        shadow.  */
+      memcpy (readbuf + bp_addr - memaddr,
+             target_info->shadow_contents + bptoffset, bp_size);
+    }
+  else
+    {
+      const unsigned char *bp;
+      CORE_ADDR placed_address = target_info->placed_address;
+      int placed_size = target_info->placed_size;
+
+      /* Update the shadow with what we want to write to memory.  */
+      memcpy (target_info->shadow_contents + bptoffset,
+             writebuf_org + bp_addr - memaddr, bp_size);
+
+      /* Determine appropriate breakpoint contents and size for this
+        address.  */
+      bp = gdbarch_breakpoint_from_pc (gdbarch, &placed_address, &placed_size);
+
+      /* Update the final write buffer with this inserted
+        breakpoint's INSN.  */
+      memcpy (writebuf + bp_addr - memaddr, bp + bptoffset, bp_size);
+    }
+}
+
 /* Update BUF, which is LEN bytes read from the target address MEMADDR,
    by replacing any memory breakpoints with their shadowed contents.
 
@@ -1435,6 +1535,7 @@ breakpoint_xfer_memory (gdb_byte *readbuf, gdb_byte *writebuf,
   /* Left boundary, right boundary and median element of our binary
      search.  */
   unsigned bc_l, bc_r, bc;
+  size_t i;
 
   /* Find BC_L which is a leftmost element which may affect BUF
      content.  It is safe to report lower value but a failure to
@@ -1508,74 +1609,27 @@ breakpoint_xfer_memory (gdb_byte *readbuf, gdb_byte *writebuf,
 
     if (!bp_location_has_shadow (bl))
       continue;
-    if (!breakpoint_address_match (bl->target_info.placed_address_space, 0,
-                                  current_program_space->aspace, 0))
-      continue;
-
-    /* Addresses and length of the part of the breakpoint that
-       we need to copy.  */
-    bp_addr = bl->target_info.placed_address;
-    bp_size = bl->target_info.shadow_len;
-
-    if (bp_addr + bp_size <= memaddr)
-      /* The breakpoint is entirely before the chunk of memory we
-         are reading.  */
-      continue;
 
-    if (bp_addr >= memaddr + len)
-      /* The breakpoint is entirely after the chunk of memory we are
-         reading.  */
-      continue;
+    one_breakpoint_xfer_memory (readbuf, writebuf, writebuf_org,
+                               memaddr, len, &bl->target_info, bl->gdbarch);
+  }
 
-    /* Offset within shadow_contents.  */
-    if (bp_addr < memaddr)
-      {
-       /* Only copy the second part of the breakpoint.  */
-       bp_size -= memaddr - bp_addr;
-       bptoffset = memaddr - bp_addr;
-       bp_addr = memaddr;
-      }
+  /* Now process single-step breakpoints.  These are not found in the
+     bp_location array.  */
+  for (i = 0; i < 2; i++)
+    {
+      struct bp_target_info *bp_tgt = single_step_breakpoints[i];
 
-    if (bp_addr + bp_size > memaddr + len)
-      {
-       /* Only copy the first part of the breakpoint.  */
-       bp_size -= (bp_addr + bp_size) - (memaddr + len);
-      }
+      if (bp_tgt != NULL)
+       {
+         struct gdbarch *gdbarch = single_step_gdbarch[i];
 
-    if (readbuf != NULL)
-      {
-       /* Verify that the readbuf buffer does not overlap with
-          the shadow_contents buffer.  */
-       gdb_assert (bl->target_info.shadow_contents >= readbuf + len
-                   || readbuf >= (bl->target_info.shadow_contents
-                                  + bl->target_info.shadow_len));
-
-       /* Update the read buffer with this inserted breakpoint's
-          shadow.  */
-       memcpy (readbuf + bp_addr - memaddr,
-               bl->target_info.shadow_contents + bptoffset, bp_size);
-      }
-    else
-      {
-       struct gdbarch *gdbarch = bl->gdbarch;
-       const unsigned char *bp;
-       CORE_ADDR placed_address = bl->target_info.placed_address;
-       int placed_size = bl->target_info.placed_size;
-
-       /* Update the shadow with what we want to write to memory.  */
-       memcpy (bl->target_info.shadow_contents + bptoffset,
-               writebuf_org + bp_addr - memaddr, bp_size);
-
-       /* Determine appropriate breakpoint contents and size for this
-          address.  */
-       bp = gdbarch_breakpoint_from_pc (gdbarch, &placed_address, &placed_size);
-
-       /* Update the final write buffer with this inserted
-          breakpoint's INSN.  */
-       memcpy (writebuf + bp_addr - memaddr, bp + bptoffset, bp_size);
-      }
-  }
+         one_breakpoint_xfer_memory (readbuf, writebuf, writebuf_org,
+                                     memaddr, len, bp_tgt, gdbarch);
+       }
+    }
 }
+
 \f
 
 /* Return true if BPT is either a software breakpoint or a hardware
@@ -15036,12 +15090,6 @@ deprecated_remove_raw_breakpoint (struct gdbarch *gdbarch, void *bp)
   return ret;
 }
 
-/* One (or perhaps two) breakpoints used for software single
-   stepping.  */
-
-static void *single_step_breakpoints[2];
-static struct gdbarch *single_step_gdbarch[2];
-
 /* Create and insert a breakpoint for software single step.  */
 
 void