From: Pedro Alves <palves@redhat.com>
Date: Mon, 2 Sep 2013 15:15:57 +0000 (+0000)
Subject: [gdbserver] Fix trace-buffer-size.exp FAILs.
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=3aafd2ffb61b3afaba478449ec0dbe45f38dac40;p=binutils-gdb.git

[gdbserver] Fix trace-buffer-size.exp FAILs.

I'm seeing trace-buffer-size.exp failing (with gdbserver):

  (gdb) PASS: gdb.trace/trace-buffer-size.exp: tstatus check 2
  show trace-buffer-size 4
  Requested size of trace buffer is 4.
  (gdb) PASS: gdb.trace/trace-buffer-size.exp: show trace buffer size
  set trace-buffer-size -1
  memory clobbered past end of allocated block
  Remote connection closed
  (gdb) FAIL: gdb.trace/trace-buffer-size.exp: set trace buffer size 2
  set trace-buffer-size unlimited
  (gdb) PASS: gdb.trace/trace-buffer-size.exp: set trace-buffer-size unlimited

That "memory clobbered past end of allocated block" is mcheck triggering.

Valgrind shows:

  ==23624== Invalid write of size 1
  ==23624==    at 0x418DD8: clear_trace_buffer (tracepoint.c:1443)
  ==23624==    by 0x418F3A: init_trace_buffer (tracepoint.c:1497)
  ==23624==    by 0x41D95B: cmd_bigqtbuffer_size (tracepoint.c:4061)
  ==23624==    by 0x41DEEC: handle_tracepoint_general_set (tracepoint.c:4193)

clear_trace_buffer does:

  static void
  clear_trace_buffer (void)
  {
    trace_buffer_start = trace_buffer_lo;
    trace_buffer_free = trace_buffer_lo;
    trace_buffer_end_free = trace_buffer_hi;
    trace_buffer_wrap = trace_buffer_hi;
    /* A traceframe with zeroed fields marks the end of trace data.  */
    ((struct traceframe *) trace_buffer_free)->tpnum = 0;
    ((struct traceframe *) trace_buffer_free)->data_size = 0;
    traceframe_read_count = traceframe_write_count = 0;
    traceframes_created = 0;
  }

And the tpnum+data_size fields are over 4 bytes...  This fixes it by
ensuring we allocate space at least for an EOB.  We have code
elsewhere that relies on the EOB being present (like e.g.,
find_traceframe), so this seems simplest.

gdb/gdbserver/
2013-09-02  Pedro Alves  <palves@redhat.com>

	* tracepoint.c (TRACEFRAME_EOB_MARKER_SIZE): New macro.
	(init_trace_buffer): Ensure at least TRACEFRAME_EOB_MARKER_SIZE is
	allocated.
	(trace_buffer_alloc): Use TRACEFRAME_EOB_MARKER_SIZE.
---

diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 54348e5a732..f0651aa0a95 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,10 @@
+2013-09-02  Pedro Alves  <palves@redhat.com>
+
+	* tracepoint.c (TRACEFRAME_EOB_MARKER_SIZE): New macro.
+	(init_trace_buffer): Ensure at least TRACEFRAME_EOB_MARKER_SIZE is
+	allocated.
+	(trace_buffer_alloc): Use TRACEFRAME_EOB_MARKER_SIZE.
+
 2013-09-02  Pierre Muller  <muller@sourceware.org>
 
 	* win32-low.c (child_xfer_memory): Check if ReadProcessMemory
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 5c0dec7f6b4..1327845657d 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -982,6 +982,10 @@ struct traceframe
 
 } ATTR_PACKED;
 
+/* The size of the EOB marker, in bytes.  A traceframe with zeroed
+   fields (and no data) marks the end of trace data.  */
+#define TRACEFRAME_EOB_MARKER_SIZE offsetof (struct traceframe, data)
+
 /* The traceframe to be used as the source of data to send back to
    GDB.  A value of -1 means to get data from the live program.  */
 
@@ -1487,12 +1491,17 @@ clear_inferior_trace_buffer (void)
 static void
 init_trace_buffer (LONGEST bufsize)
 {
+  size_t alloc_size;
+
   trace_buffer_size = bufsize;
 
-  /* If we already have a trace buffer, try realloc'ing.  */
-  trace_buffer_lo = xrealloc (trace_buffer_lo, bufsize);
+  /* Make sure to internally allocate at least space for the EOB
+     marker.  */
+  alloc_size = (bufsize < TRACEFRAME_EOB_MARKER_SIZE
+		? TRACEFRAME_EOB_MARKER_SIZE : bufsize);
+  trace_buffer_lo = xrealloc (trace_buffer_lo, alloc_size);
 
-  trace_buffer_hi = trace_buffer_lo + bufsize;
+  trace_buffer_hi = trace_buffer_lo + trace_buffer_size;
 
   clear_trace_buffer ();
 }
@@ -1532,7 +1541,7 @@ trace_buffer_alloc (size_t amt)
 	       (long) amt, (long) sizeof (struct traceframe));
 
   /* Account for the EOB marker.  */
-  amt += sizeof (struct traceframe);
+  amt += TRACEFRAME_EOB_MARKER_SIZE;
 
 #ifdef IN_PROCESS_AGENT
  again: