This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[RFA] Fix gdb/15678
- From: Keith Seitz <keiths at redhat dot com>
- To: "gdb-patches at sourceware dot org ml" <gdb-patches at sourceware dot org>
- Date: Tue, 06 Aug 2013 10:30:58 -0700
- Subject: [RFA] Fix gdb/15678
Hi,
$SUBJECT involves a crash when the user types "enable count" with no
arguments.
This is happening because enable_count_command never checks that `args'
is non-NULL, and get_number immediately starts dereferencing this argument.
I've chosen to fix this by adding a check in enable_count and adding an
assert to get_number should it ever get invalid input again.
I've done it this way because get_number really isn't in a position to
issue a meaningful error message (other than perhaps "invalid input" or
"missing input"). The caller is able to issue a clearer, more meaningful
error message. IMO callers should also be validating their own input.
I've audited all callers of get_number to make sure they were checking
this and issuing appropriate error messages. I've also added a test for
"enable count".
I'll have a follow-on patch from this audit.
Keith
ChangeLog
2013-08-06 Keith Seitz <keiths@redhat.com>
PR gdb/15678
* breakpoint.c (enable_count_command): Call error_no_arg if
`arg' is NULL.
* cli/cli-utils.c (get_number): Assert if `pp' or `*pp' is NULL.
* cli/cli-utils.h (get_number): Mention that the
argument should be valid or the function will assert.
testsuite/ChangeLog
2013-08-06 Keith Seitz <keiths@redhat.com>
PR gdb/15678
* gdb.base/default.exp: Add test for calling `enable count'
with no argument.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index eec7b7c..cb218c3 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -14740,8 +14740,12 @@ do_map_enable_count_breakpoint (struct breakpoint *bpt, void *countptr)
static void
enable_count_command (char *args, int from_tty)
{
- int count = get_number (&args);
+ int count;
+
+ if (args == NULL)
+ error_no_arg (_("a breakpoint number"));
+ count = get_number (&args);
map_breakpoint_numbers (args, do_map_enable_count_breakpoint, &count);
}
diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
index f74e6b1..8270573 100644
--- a/gdb/cli/cli-utils.c
+++ b/gdb/cli/cli-utils.c
@@ -112,6 +112,7 @@ get_number_trailer (char **pp, int trailer)
int
get_number (char **pp)
{
+ gdb_assert (pp != NULL && *pp != NULL);
return get_number_trailer (pp, '\0');
}
diff --git a/gdb/cli/cli-utils.h b/gdb/cli/cli-utils.h
index 152fb89..a1a2aa0 100644
--- a/gdb/cli/cli-utils.h
+++ b/gdb/cli/cli-utils.h
@@ -20,13 +20,15 @@
#ifndef CLI_UTILS_H
#define CLI_UTILS_H
-/* *PP is a string denoting a number. Get the number of the. Advance
+/* *PP is a string denoting a number. Return this number. Advance
*PP after the string and any trailing whitespace.
Currently the string can either be a number, or "$" followed by the
- name of a convenience variable, or ("$" or "$$") followed by digits. */
+ name of a convenience variable, or ("$" or "$$") followed by digits.
-extern int get_number (char **);
+ PP and *PP should be valid, otherwise the function will assert. */
+
+extern int get_number (char **pp);
/* An object of this type is passed to get_number_or_range. It must
be initialized by calling init_number_or_range. This type is
diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
index e5194f7..c684828 100644
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -177,6 +177,8 @@ gdb_test "enable breakpoints delete" "Argument required .one or more breakpoint
gdb_test "enable breakpoints once" "Argument required .one or more breakpoint numbers.*" "enable breakpoints once"
#test enable breakpoints
gdb_test_no_output "enable breakpoints" "enable breakpoints"
+#test enable count
+gdb_test "enable count" "Argument required \\(a breakpoint number\\)\."
#test enable delete
gdb_test "enable delete" "Argument required .one or more breakpoint numbers.*" "enable delete"
#test enable display