This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFA] Problem after hitting breakpoint on Windows (with GDBserver)


When debugging on Windows with GDBserver, the debugger starts
failing after hitting a breakpoint.  For instance:

    (gdb) b foo
    Breakpoint 1 at 0x40177e: file foo.adb, line 5.
    (gdb) cont
    Continuing.

    Breakpoint 1, foo () at foo.adb:5
    5          Put_Line ("Hello World.");  -- STOP
    (gdb) n

    Program received signal SIGSEGV, Segmentation fault.
    0x00401782 in foo () at foo.adb:5
    5          Put_Line ("Hello World.");  -- STOP

There are two issues:

  1. While trying to re-insert a breakpoint that is still inserted
     in memory, insert_bp_location wipes out the breakpoint location's
     shadow_contents.  As a consequence, we cannot restore the proper
     instruction when removing the breakpoint anymore.  That's why
     the inferior's behavior changes when trying to resume after
     the breakpoint was hit.

  2. mem-break.c:default_memory_insert_breakpoint passes a breakpoint
     location's shadow_contents as the buffer for a memory read.
     This reveals a limitation of the various memory-read target
     functions.  This patch documents this limitation and adjust
     the two calls that seem to hit that limitation.

gdb/ChangeLog:

2012-03-12  Joel Brobecker  <brobecker@adacore.com>
            Pedro Alves <palves@redhat.com>

        * breakpoint.c (insert_bp_location): Do not wipe bl->target_info out.

        * target.h (target_read): Document limitation.
        * target.c (memory_xfer_partial, target_xfer_partial)
        (target_read_memory): Document limitation.
        * mem-break.c: #include "gdb_string.h".
        (default_memory_insert_breakpoint): Do not call target_read_memory
        with a pointer to the breakpoint's shadow_contents buffer.  Use
        a local buffer instead.
        * m32r-tdep.c (m32r_memory_insert_breakpoint): Ditto.

Tested on x86-windows with AdaCore's testsuite (standalone and with
GDBserver). And tested on x86_64-linux, with the official testsuite,
in both standalone and native-gdbserver modes.  No regression detected.

The change in m32r-tdep.c could not be tested beyond simply compiling
the file. A careful code inspection would be appreciated.

OK to commit?

Thanks,
-- 
Joel

---
 gdb/breakpoint.c |   11 +++++++++--
 gdb/m32r-tdep.c  |    3 ++-
 gdb/mem-break.c  |    7 ++++++-
 gdb/target.c     |   19 ++++++++++++++++---
 gdb/target.h     |    7 ++++++-
 5 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 1158f0e..06d1c6f 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2082,8 +2082,15 @@ insert_bp_location (struct bp_location *bl,
   if (!should_be_inserted (bl) || (bl->inserted && !bl->needs_update))
     return 0;
 
-  /* Initialize the target-specific information.  */
-  memset (&bl->target_info, 0, sizeof (bl->target_info));
+  /* Note we don't initialize bl->target_info, as that wipes out
+     the breakpoint location's shadow_contents if the breakpoint
+     is still inserted at that location.  This in turn breaks
+     target_read_memory which depends on these buffers when
+     a memory read is requested at the breakpoint location:
+     Once the target_info has been wiped, we fail to see that
+     we have a breakpoint inserted at that address and thus
+     read the breakpoint instead of returning the data saved in
+     the breakpoint location's shadow contents.  */
   bl->target_info.placed_address = bl->address;
   bl->target_info.placed_address_space = bl->pspace->aspace;
   bl->target_info.length = bl->length;
diff --git a/gdb/m32r-tdep.c b/gdb/m32r-tdep.c
index 72872bd..d504eb3 100644
--- a/gdb/m32r-tdep.c
+++ b/gdb/m32r-tdep.c
@@ -85,7 +85,7 @@ m32r_memory_insert_breakpoint (struct gdbarch *gdbarch,
   CORE_ADDR addr = bp_tgt->placed_address;
   int val;
   gdb_byte buf[4];
-  gdb_byte *contents_cache = bp_tgt->shadow_contents;
+  gdb_byte contents_cache[4];
   gdb_byte bp_entry[] = { 0x10, 0xf1 };	/* dpt */
 
   /* Save the memory contents.  */
@@ -93,6 +93,7 @@ m32r_memory_insert_breakpoint (struct gdbarch *gdbarch,
   if (val != 0)
     return val;			/* return error */
 
+  memcpy (bp_tgt->shadow_contents, contents_cache, 4);
   bp_tgt->placed_size = bp_tgt->shadow_len = 4;
 
   /* Determine appropriate breakpoint contents and size for this address.  */
diff --git a/gdb/mem-break.c b/gdb/mem-break.c
index 7d0e3f1..41c1cd9 100644
--- a/gdb/mem-break.c
+++ b/gdb/mem-break.c
@@ -29,6 +29,7 @@
 #include "breakpoint.h"
 #include "inferior.h"
 #include "target.h"
+#include "gdb_string.h"
 
 
 /* Insert a breakpoint on targets that don't have any better
@@ -46,6 +47,7 @@ default_memory_insert_breakpoint (struct gdbarch *gdbarch,
 {
   int val;
   const unsigned char *bp;
+  gdb_byte *readbuf;
 
   /* Determine appropriate breakpoint contents and size for this address.  */
   bp = gdbarch_breakpoint_from_pc
@@ -55,8 +57,11 @@ default_memory_insert_breakpoint (struct gdbarch *gdbarch,
 
   /* Save the memory contents.  */
   bp_tgt->shadow_len = bp_tgt->placed_size;
-  val = target_read_memory (bp_tgt->placed_address, bp_tgt->shadow_contents,
+  readbuf = alloca (bp_tgt->placed_size);
+  val = target_read_memory (bp_tgt->placed_address, readbuf,
 			    bp_tgt->placed_size);
+  if (val == 0)
+    memcpy (bp_tgt->shadow_contents, readbuf, bp_tgt->placed_size);
 
   /* Write the breakpoint.  */
   if (val == 0)
diff --git a/gdb/target.c b/gdb/target.c
index cffea2c..6e13ae9 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1608,7 +1608,11 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
 }
 
 /* Perform a partial memory transfer.  For docs see target.h,
-   to_xfer_partial.  */
+   to_xfer_partial.
+
+   In addition, READBUF must not be the shadow_contents buffer of
+   one of the breakpoint locations.  Otherwise, this shadow_contents
+   buffer will become corrupted.  */
 
 static LONGEST
 memory_xfer_partial (struct target_ops *ops, enum target_object object,
@@ -1665,7 +1669,12 @@ make_show_memory_breakpoints_cleanup (int show)
 		       (void *) (uintptr_t) current);
 }
 
-/* For docs see target.h, to_xfer_partial.  */
+/* For docs see target.h, to_xfer_partial.
+
+   In addition, READBUF must not be the shadow_contents buffer of
+   one of the breakpoint locations when OBJECT is TARGET_OBJECT_MEMORY
+   or TARGET_OBJECT_STACK_MEMORY.  Otherwise, this shadow_contents
+   buffer will become corrupted.  */
 
 static LONGEST
 target_xfer_partial (struct target_ops *ops,
@@ -1754,7 +1763,11 @@ target_xfer_partial (struct target_ops *ops,
    filling the buffer with good data.  There is no way for the caller to know
    how much good data might have been transfered anyway.  Callers that can
    deal with partial reads should call target_read (which will retry until
-   it makes no progress, and then return how much was transferred).  */
+   it makes no progress, and then return how much was transferred).
+
+   As a limitation, MYADDR must not be the shadow_contents buffer of one
+   of the breakpoint locations.  Passing a breakpoint's shadow_contents
+   buffer will cause that buffer to become corrupted.  */
 
 int
 target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, int len)
diff --git a/gdb/target.h b/gdb/target.h
index 50a0ea6..1e7d01a 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -310,8 +310,13 @@ DEF_VEC_P(static_tracepoint_marker_p);
    transfer is not supported or otherwise fails.  Return of a positive
    value less than LEN indicates that no further transfer is possible.
    Unlike the raw to_xfer_partial interface, callers of these
-   functions do not need to retry partial transfers.  */
+   functions do not need to retry partial transfers.
 
+   As a limitation when OBJECT is TARGET_OBJECT_MEMORY or
+   TARGET_OBJECT_STACK_MEMORY,  MYADDR must not be the shadow_contents
+   buffer of one of the breakpoint locations.  Passing a breakpoint's
+   shadow_contents buffer in that situation will cause that buffer
+   to become corrupted.  */
 extern LONGEST target_read (struct target_ops *ops,
 			    enum target_object object,
 			    const char *annex, gdb_byte *buf,
-- 
1.7.1


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]