This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
robustify target tfile's open code
- From: Pedro Alves <pedro at codesourcery dot com>
- To: gdb-patches at sourceware dot org
- Date: Fri, 20 May 2011 14:54:11 +0100
- Subject: robustify target tfile's open code
I'm applying this patch to mainline and branch.
While adding a test for Marc's bug report, I saw
(gdb) interpreter-exec mi "-target-select tfile error.tf"
&"warning: Uploaded tracepoint 1 has no source location, using raw address\n"
~"Created tracepoint 1 for target's tracepoint 1 at 0x40088c.\n"
&"warning: No traceframes present in this file.\n"
^connected
(gdb)
(gdb) interpreter-exec mi "-target-select tfile error.tf"
~"../../src/gdb/inferior.c:363: internal-error: find_inferior_pid: Assertion `pid != 0' failed.\n"
~"A problem internal to GDB has been detected,\n"
~"further debugging may prove unreliable.\n"
~"Quit this debugging session? (y or n) "
The target tfile's open code can return leaving the target open,
but with no threads listed. This path fixes it, using a
try/finally pattern similar to remote.c's. Everything that's
early indespensible setup that fail pops the target. A similar problem
is that the tfile target does not claim its own thread is "alive", so
an "info threads" or any other command that updates the thread
list wipes it, leading to similar internal errors.
--
Pedro Alves
2011-05-20 Pedro Alves <pedro@codesourcery.com>
gdb/
* tracepoint.c (TFILE_PID): Move higher in file.
(tfile_open): Delay pushing the tfile target until we're assured
the tfile header is present in the file. Wrap reading the initial
newline-terminated lines in TRY_CATCH. Pop the target if the
initial setup failed. Add the tfile's thread immediately
aftwards, before any non-essential setup. Don't skip
post_create_inferior if there are no traceframes present in the
file.
(tfile_close): Remove redundant check for null before xfree call.
(tfile_thread_alive): New function.
(init_tfile_ops): Register it as to_thread_alive callback.
---
gdb/tracepoint.c | 97 ++++++++++++++++++++++++++++++++-----------------------
1 file changed, 57 insertions(+), 40 deletions(-)
Index: src/gdb/tracepoint.c
===================================================================
--- src.orig/gdb/tracepoint.c 2011-05-20 13:58:42.000000000 +0100
+++ src/gdb/tracepoint.c 2011-05-20 14:28:46.088819003 +0100
@@ -51,6 +51,7 @@
#include "ax.h"
#include "ax-gdb.h"
#include "memrange.h"
+#include "exceptions.h"
/* readline include files */
#include "readline/readline.h"
@@ -80,6 +81,8 @@ extern int bin2hex (const gdb_byte *bin,
large. (400 - 31)/2 == 184 */
#define MAX_AGENT_EXPR_LEN 184
+#define TFILE_PID (1)
+
/* A hook used to notify the UI of tracepoint operations. */
void (*deprecated_trace_find_hook) (char *arg, int from_tty);
@@ -3293,6 +3296,7 @@ tfile_read (gdb_byte *readbuf, int size)
static void
tfile_open (char *filename, int from_tty)
{
+ volatile struct gdb_exception ex;
char *temp;
struct cleanup *old_chain;
int flags;
@@ -3330,8 +3334,6 @@ tfile_open (char *filename, int from_tty
discard_cleanups (old_chain); /* Don't free filename any more. */
unpush_target (&tfile_ops);
- push_target (&tfile_ops);
-
trace_filename = xstrdup (filename);
trace_fd = scratch_chan;
@@ -3344,6 +3346,8 @@ tfile_open (char *filename, int from_tty
&& (strncmp (header + 1, "TRACE0\n", 7) == 0)))
error (_("File is not a valid trace file."));
+ push_target (&tfile_ops);
+
trace_regblock_size = 0;
ts = current_trace_status ();
/* We know we're working with a file. */
@@ -3358,29 +3362,53 @@ tfile_open (char *filename, int from_tty
cur_traceframe_number = -1;
- /* Read through a section of newline-terminated lines that
- define things like tracepoints. */
- i = 0;
- while (1)
+ TRY_CATCH (ex, RETURN_MASK_ALL)
{
- tfile_read (&byte, 1);
-
- ++bytes;
- if (byte == '\n')
+ /* Read through a section of newline-terminated lines that
+ define things like tracepoints. */
+ i = 0;
+ while (1)
{
- /* Empty line marks end of the definition section. */
- if (i == 0)
- break;
- linebuf[i] = '\0';
- i = 0;
- tfile_interp_line (linebuf, &uploaded_tps, &uploaded_tsvs);
+ tfile_read (&byte, 1);
+
+ ++bytes;
+ if (byte == '\n')
+ {
+ /* Empty line marks end of the definition section. */
+ if (i == 0)
+ break;
+ linebuf[i] = '\0';
+ i = 0;
+ tfile_interp_line (linebuf, &uploaded_tps, &uploaded_tsvs);
+ }
+ else
+ linebuf[i++] = byte;
+ if (i >= 1000)
+ error (_("Excessively long lines in trace file"));
}
- else
- linebuf[i++] = byte;
- if (i >= 1000)
- error (_("Excessively long lines in trace file"));
+
+ /* Record the starting offset of the binary trace data. */
+ trace_frames_offset = bytes;
+
+ /* If we don't have a blocksize, we can't interpret the
+ traceframes. */
+ if (trace_regblock_size == 0)
+ error (_("No register block size recorded in trace file"));
+ }
+ if (ex.reason < 0)
+ {
+ /* Pop the partially set up target. */
+ pop_target ();
+ throw_exception (ex);
}
+ inferior_appeared (current_inferior (), TFILE_PID);
+ inferior_ptid = pid_to_ptid (TFILE_PID);
+ add_thread_silent (inferior_ptid);
+
+ if (ts->traceframe_count <= 0)
+ warning (_("No traceframes present in this file."));
+
/* Add the file's tracepoints and variables into the current mix. */
/* Get trace state variables first, they may be checked when parsing
@@ -3389,24 +3417,6 @@ tfile_open (char *filename, int from_tty
merge_uploaded_tracepoints (&uploaded_tps);
- /* Record the starting offset of the binary trace data. */
- trace_frames_offset = bytes;
-
- /* If we don't have a blocksize, we can't interpret the
- traceframes. */
- if (trace_regblock_size == 0)
- error (_("No register block size recorded in trace file"));
- if (ts->traceframe_count <= 0)
- {
- warning (_("No traceframes present in this file."));
- return;
- }
-
-#define TFILE_PID (1)
- inferior_appeared (current_inferior (), TFILE_PID);
- inferior_ptid = pid_to_ptid (TFILE_PID);
- add_thread_silent (inferior_ptid);
-
post_create_inferior (&tfile_ops, from_tty);
}
@@ -3712,8 +3722,8 @@ tfile_close (int quitting)
close (trace_fd);
trace_fd = -1;
- if (trace_filename)
- xfree (trace_filename);
+ xfree (trace_filename);
+ trace_filename = NULL;
}
static void
@@ -4206,6 +4216,12 @@ tfile_has_registers (struct target_ops *
return traceframe_number != -1;
}
+static int
+tfile_thread_alive (struct target_ops *ops, ptid_t ptid)
+{
+ return 1;
+}
+
/* Callback for traceframe_walk_blocks. Builds a traceframe_info
object for the tfile target's current traceframe. */
@@ -4278,6 +4294,7 @@ init_tfile_ops (void)
tfile_ops.to_has_stack = tfile_has_stack;
tfile_ops.to_has_registers = tfile_has_registers;
tfile_ops.to_traceframe_info = tfile_traceframe_info;
+ tfile_ops.to_thread_alive = tfile_thread_alive;
tfile_ops.to_magic = OPS_MAGIC;
}