This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Fix GDB crash when loading a tracefile without having loaded a symbol table, and, $tpnum mishandling
- From: Pedro Alves <pedro at codesourcery dot com>
- To: gdb-patches at sourceware dot org
- Date: Fri, 12 Feb 2010 00:52:03 +0000
- Subject: 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)
{