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]

Fix GDB crash when loading a tracefile without having loaded a symbol table, and, $tpnum mishandling


If you load a tfile that has uploaded tracepoint definitions into gdb,
without a symbol file loaded, and the tracepoint defines a pass count,
gdb crashes:

$ gdb
...
(gdb) target tfile tfile.txt
No symbol table is loaded.  Use the "file" command.
Make breakpoint pending on future shared library load? (y or [n]) n
Segmentation fault

The problem is that create_tracepoint_from_upload assumes break_command_really 
always returns with a tracepoint created, but in some cases, like
above, it doesn't.

The fix is to change break_command_really's return type to indicate
if a breakpoint was really created or not, and bail out early in
create_tracepoint_from_upload in the latter case.

With the fix, one now gets the expected:

 "Failed to create tracepoint for target's tracepoint 2 (...), skipping it."

Closer inspection reveals that all the calls to set_tracepoint_count
should have the same guard --- otherwise, in the case of 
breakpoint_command_really not really creating any tracepoint, we'll
be clobbering the $tpnum convenience variable with a breakpoint number
that may not correspond to a tracepoint.  E.g.,:

 (gdb) p $bpnum
 $1 = 1
 (gdb) p $tpnum
 $2 = void
 (gdb) trace foofoofoo
 Function "foofoofoo" not defined.
 Make breakpoint pending on future shared library load? (y or [n]) n
 (gdb) p $tpnum
 $3 = 1

Whoops.

Applied.

-- 
Pedro Alves

2010-02-12  Pedro Alves  <pedro@codesourcery.com>

	* breakpoint.c (break_command_really): Change return type to int.
	Return false if no breakpoint was created, true otherwise.
	(trace_command): Don't set the tracepoint count if no tracepoint
	was created.
	(ftrace_command): Ditto.
	(create_tracepoint_from_upload): Bail out if the tracepoint wasn't
	created in the breakpoints table.

---
 gdb/breakpoint.c |   98 ++++++++++++++++++++++++++++---------------------------
 1 file changed, 51 insertions(+), 47 deletions(-)

Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c	2010-02-11 23:16:08.000000000 +0000
+++ src/gdb/breakpoint.c	2010-02-12 00:22:28.000000000 +0000
@@ -6722,17 +6722,16 @@ find_condition_and_thread (char *tok, CO
     }
 }
 
-/* Set a breakpoint.  This function is shared between
-   CLI and MI functions for setting a breakpoint.
-   This function has two major modes of operations,
-   selected by the PARSE_CONDITION_AND_THREAD parameter.
-   If non-zero, the function will parse arg, extracting
-   breakpoint location, address and thread. Otherwise,
-   ARG is just the location of breakpoint, with condition
-   and thread specified by the COND_STRING and THREAD
-   parameters.  */
+/* Set a breakpoint.  This function is shared between CLI and MI
+   functions for setting a breakpoint.  This function has two major
+   modes of operations, selected by the PARSE_CONDITION_AND_THREAD
+   parameter.  If non-zero, the function will parse arg, extracting
+   breakpoint location, address and thread. Otherwise, ARG is just the
+   location of breakpoint, with condition and thread specified by the
+   COND_STRING and THREAD parameters.  Returns true if any breakpoint
+   was created; false otherwise.  */
 
-static void
+static int
 break_command_really (struct gdbarch *gdbarch,
 		      char *arg, char *cond_string, int thread,
 		      int parse_condition_and_thread,
@@ -6793,7 +6792,7 @@ break_command_really (struct gdbarch *gd
 	     selects no, then simply return the error code.  */
 	  if (pending_break_support == AUTO_BOOLEAN_AUTO
 	      && !nquery ("Make breakpoint pending on future shared library load? "))
-	    return;
+	    return 0;
 
 	  /* At this point, either the user was queried about setting
 	     a pending breakpoint and selected yes, or pending
@@ -6811,7 +6810,7 @@ break_command_really (struct gdbarch *gd
 	}
     default:
       if (!sals.nelts)
-	return;
+	return 0;
     }
 
   /* Create a chain of things that always need to be cleaned up. */
@@ -6923,6 +6922,8 @@ break_command_really (struct gdbarch *gd
 
   /* error call may happen here - have BKPT_CHAIN already discarded.  */
   update_global_location_list (1);
+
+  return 1;
 }
 
 /* Set a breakpoint. 
@@ -9789,33 +9790,33 @@ set_tracepoint_count (int num)
 void
 trace_command (char *arg, int from_tty)
 {
-  break_command_really (get_current_arch (),
-			arg,
-			NULL, 0, 1 /* parse arg */,
-			0 /* tempflag */, 0 /* hardwareflag */,
-			1 /* traceflag */,
-			0 /* Ignore count */,
-			pending_break_support, 
-			NULL,
-			from_tty,
-			1 /* enabled */);
-  set_tracepoint_count (breakpoint_count);
+  if (break_command_really (get_current_arch (),
+			    arg,
+			    NULL, 0, 1 /* parse arg */,
+			    0 /* tempflag */, 0 /* hardwareflag */,
+			    1 /* traceflag */,
+			    0 /* Ignore count */,
+			    pending_break_support,
+			    NULL,
+			    from_tty,
+			    1 /* enabled */))
+    set_tracepoint_count (breakpoint_count);
 }
 
 void
 ftrace_command (char *arg, int from_tty)
 {
-  break_command_really (get_current_arch (),
-			arg, 
-			NULL, 0, 1 /* parse arg */,
-			0 /* tempflag */, 1 /* hardwareflag */,
-			1 /* traceflag */,
-			0 /* Ignore count */,
-			pending_break_support, 
-			NULL,
-			from_tty,
-			1 /* enabled */);
-  set_tracepoint_count (breakpoint_count);
+  if (break_command_really (get_current_arch (),
+			    arg,
+			    NULL, 0, 1 /* parse arg */,
+			    0 /* tempflag */, 1 /* hardwareflag */,
+			    1 /* traceflag */,
+			    0 /* Ignore count */,
+			    pending_break_support,
+			    NULL,
+			    from_tty,
+			    1 /* enabled */))
+    set_tracepoint_count (breakpoint_count);
 }
 
 /* Given information about a tracepoint as recorded on a target (which
@@ -9829,24 +9830,27 @@ create_tracepoint_from_upload (struct up
 {
   char buf[100];
   struct breakpoint *tp;
-  
+
   /* In the absence of a source location, fall back to raw address.  */
   sprintf (buf, "*%s", paddress (get_current_arch(), utp->addr));
 
-  break_command_really (get_current_arch (),
-			buf, 
-			NULL, 0, 1 /* parse arg */,
-			0 /* tempflag */,
-			(utp->type == bp_fast_tracepoint) /* hardwareflag */,
-			1 /* traceflag */,
-			0 /* Ignore count */,
-			pending_break_support, 
-			NULL,
-			0 /* from_tty */,
-			utp->enabled /* enabled */);
+  if (!break_command_really (get_current_arch (),
+			     buf,
+			     NULL, 0, 1 /* parse arg */,
+			     0 /* tempflag */,
+			     (utp->type == bp_fast_tracepoint) /* hardwareflag */,
+			     1 /* traceflag */,
+			     0 /* Ignore count */,
+			     pending_break_support,
+			     NULL,
+			     0 /* from_tty */,
+			     utp->enabled /* enabled */))
+    return NULL;
+
   set_tracepoint_count (breakpoint_count);
   
-    tp = get_tracepoint (tracepoint_count);
+  tp = get_tracepoint (tracepoint_count);
+  gdb_assert (tp != NULL);
 
   if (utp->pass > 0)
     {


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