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]

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;
 }
 


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