[PATCH] Fix problem handling colon in linespec, PR breakpoints/18303

Don Breazeal
2016-01-08T18:44:22Z

This patch fixes an assertion failure that results from a problem
with handling a single colon in a linespec. On a Windows host, using
the break command (or the MI request -break-insert) with the
"filename:linenumber" style linespec will cause an assertion failure if
the file doesn't exist, or isn't listed in the program's symbols, and
the filename contains a Windows logical volume, e.g. "C:".

One of the new tests in this patch demonstrated the failure like this:

Running ../../../binutils-gdb/gdb/testsuite/gdb.linespec/ls-errs-cp.exp ...
FAIL: gdb.linespec/ls-errs-cp.exp: break "spaces: and :colons.cc":3 (GDB internal error)
FAIL: gdb.linespec/ls-errs-cp.exp: break C:/nonexist-with-windrive.cc:3 (GDB internal error)

The assertion failure looked like this:

../../binutils-gdb/gdb/cp-namespace.c:252: internal-error: cp_search_static_and_baseclasses: Assertion `name[prefix_len + 1] == ':'' failed.

During GDB's parsing of the linespec, when the filename was not found in
the program's symbols, GDB tried to determine if the filename string
could be some other valid identifier. Eventually it would get to a point
where it concluded that the Windows logical volume, or whatever was to the
left of the colon, was a C++ namespace. When it found only one colon, it
would assert.

GDB's linespec grammar allows for several different linespecs that contain
':'. The only one that has a number after the colon is filename:linenum.

With this patch, if we get a "file not found" error when looking the
filename up in the program's symbols, we look ahead in the linespec
being parsed to see if there is a number following the colon. If there
is, we are guaranteed that the string preceding the colon is a filename.
In this case we can just throw an error and quit parsing the linespec,
since it is invalid.

The result of this is that GDB will set a pending breakpoint (as directed)
instead of terminating with an assertion failure.

There is another possible solution. After no symbols are found for the
file and we determine that it is a filename:linenum style linespec, we
could just consume the filename token and parse the rest of the
linespec. That works as well, but there is no advantage to it.

I created two new tests for this. One is just gdb.linespec/ls-errs.exp
copied and converted to use C++ instead of C, and to add the Windows
logical drive case. The other is an MI test to provide a regression
test for the specific case reported in PR 18303.

Tested on a Windows 7 x86_64 system with a PowerPC board running Linux,
and on an x86_64 system with both native and remote.

OK?
Thanks,
--Don

gdb/ChangeLog:
PR breakpoints/18303
* linespec.c (linespec_lexer_peek_second_token): New function.
(parse_linespec): Call linespec_lexer_peek_second_token and
throw error if sourcer file not found.

gdb/testsuite/ChangeLog:
* gdb/testsuite/gdb.linespec/ls-errs-cp.cc: New test program.
* gdb/testsuite/gdb.linespec/ls-errs-cp.exp: New test.
* gdb/testsuite/gdb.mi/linespec-err-cp.cc: New test program.
* gdb/testsuite/gdb.mi/linespec-err-cp.exp: New test.

---
gdb/linespec.c | 31 ++++
gdb/testsuite/gdb.linespec/ls-errs-cp.cc | 36 +++++
gdb/testsuite/gdb.linespec/ls-errs-cp.exp | 240 +++++++++++++++++++++++++++++
gdb/testsuite/gdb.mi/linespec-err-cp.cc | 35 ++++
gdb/testsuite/gdb.mi/linespec-err-cp.exp | 57 +++++++
5 files changed, 399 insertions(+), 0 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 50951bd..668a85c 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -801,6 +801,26 @@ linespec_lexer_peek_token (linespec_parser *parser)
return next;
}

+/* Return the token-after-next without consuming either the current
+ token or the next token. */
+
+static linespec_token
+linespec_lexer_peek_second_token (linespec_parser *parser)
+{
+ linespec_token next;
+ const char *saved_stream = PARSER_STREAM (parser);
+ linespec_token saved_token = parser->lexer.current;
+
+ /* Get the next token. */
+ next = linespec_lexer_consume_token (parser);
+
+ /* Get the token after that. */
+ next = linespec_lexer_consume_token (parser);
+ PARSER_STREAM (parser) = saved_stream;
+ parser->lexer.current = saved_token;
+ return next;
+}
+
/* Helper functions. */

/* Add SAL to SALS. */
@@ -2306,6 +2326,17 @@ parse_linespec (linespec_parser *parser, const char *arg)
}
else
{
+ /* Check to see if we have the case filename:linenum
+ where the file was not found in the program symbols.
+ Peek at the token after the ':'. If it is a line
+ number then we have that case, so propagate the error. */
+ token = linespec_lexer_peek_second_token (parser);
+ if (token.type == LSTOKEN_NUMBER)
+ {
+ throw_error (file_exception.error,
+ "%s", file_exception.message);
+ }
+
/* No symtabs found -- discard user_filename. */
xfree (user_filename);

diff --git a/gdb/testsuite/gdb.linespec/ls-errs-cp.cc b/gdb/testsuite/gdb.linespec/ls-errs-cp.cc
new file mode 100644
index 0000000..a3a43db
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/ls-errs-cp.cc
@@ -0,0 +1,36 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2012-2016 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+int
+myfunction (int aa)
+{
+ int i;
+
+ i = aa + 42;
+ return i; /* set breakpoint here */
+}
+
+int
+main (void)
+{
+ int a;
+
+ a = myfunction (a);
+
+ here:
+ return a;
+}
diff --git a/gdb/testsuite/gdb.linespec/ls-errs-cp.exp b/gdb/testsuite/gdb.linespec/ls-errs-cp.exp
new file mode 100644
index 0000000..be1c3ba
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/ls-errs-cp.exp
@@ -0,0 +1,240 @@
+# Copyright 2012-2016 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Tests for linespec errors with C++.
+# Derived from gdb.linespec/ls-errs.exp.
+
+if { [skip_cplus_tests] } { continue }
+
+standard_testfile .cc
+set exefile $testfile
+
+if {[prepare_for_testing $testfile $exefile $srcfile {debug c++}]} {
+ return -1
+}
+
+# Turn off the pending breakpoint queries.
+gdb_test_no_output "set breakpoint pending off"
+
+# Turn off completion limiting
+gdb_test_no_output "set max-completions unlimited"
+
+if ![runto_main] {
+ fail "Can't run to main"
+ return 0
+}
+
+# Run to a location in the file.
+set bp_location [gdb_get_line_number "set breakpoint here"]
+
+gdb_test "break $srcfile:$bp_location" \
+ "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \
+ "breakpoint line number in file"
+
+gdb_continue_to_breakpoint "$bp_location"
+
+# We intentionally do not use gdb_breakpoint for these tests.
+
+# Break at 'linespec' and expect the message in ::error_messages indexed by
+# msg_id with the associated args.
+proc test_break {linespec msg_id args} {
+ global error_messages
+
+ gdb_test "break $linespec" [string_to_regexp \
+ [eval format \$error_messages($msg_id) $args]]
+}
+
+# Common error message format strings.
+array set error_messages {
+ invalid_file "No source file named %s."
+ invalid_function "Function \"%s\" not defined."
+ invalid_var_or_func "Undefined convenience variable or function \"%s\" not defined."
+ invalid_function_f "Function \"%s\" not defined in \"%s\"."
+ invalid_var_or_func_f \
+ "Undefined convenience variable or function \"%s\" not defined in \"%s\"."
+ invalid_label "No label \"%s\" defined in function \"%s\"."
+ invalid_parm "invalid linespec argument, \"%s\""
+ invalid_offset "No line %d in the current file."
+ invalid_offset_f "No line %d in file \"%s\"."
+ malformed_line_offset "malformed line offset: \"%s\""
+ source_incomplete \
+ "Source filename requires function, label, or line offset."
+ unexpected "malformed linespec error: unexpected %s"
+ unexpected_opt "malformed linespec error: unexpected %s, \"%s\""
+ unmatched_quote "unmatched quote"
+ garbage "Garbage '%s' at end of command"
+}
+
+# Some commonly used whitespace tests around ':'.
+set spaces [list ":" ": " " :" " : " "\t: " " :\t" "\t:\t" " \t:\t " \
+ "\t \t:\t \t \t"]
+
+# A list of invalid offsets.
+set invalid_offsets [list -100 +500 1000]
+
+# Try some simple, invalid linespecs involving spaces.
+foreach x $spaces {
+ test_break $x unexpected "colon"
+}
+
+# Test invalid filespecs starting with offset. This is done
+# first so that default offsets are tested.
+foreach x $invalid_offsets {
+ set offset $x
+
+ # Relative offsets are relative to line 16. Adjust
+ # expected offset from error message accordingly.
+ if {[string index $x 0] == "+" ||
+ [string index $x 0] == "-"} {
+ incr offset 24
+ }
+ test_break $x invalid_offset $offset
+ test_break "-line $x" invalid_offset $offset
+}
+
+# Test offsets with trailing tokens w/ and w/o spaces.
+foreach x $spaces {
+ test_break "3$x" unexpected "colon"
+ test_break "+10$x" unexpected "colon"
+ test_break "-10$x" unexpected "colon"
+}
+
+foreach x {1 +1 +100 -10} {
+ test_break "3 $x" unexpected_opt "number" $x
+ test_break "-line 3 $x" garbage $x
+ test_break "+10 $x" unexpected_opt "number" $x
+ test_break "-line +10 $x" garbage $x
+ test_break "-10 $x" unexpected_opt "number" $x
+ test_break "-line -10 $x" garbage $x
+}
+
+foreach x {3 +10 -10} {
+ test_break "$x foo" unexpected_opt "string" "foo"
+ test_break "-line $x foo" garbage "foo"
+}
+
+# Test invalid linespecs starting with filename.
+set invalid_files [list "this_file_doesn't_exist.cc" \
+ "this file has spaces.cc" \
+ "\"file::colons.cc\"" \
+ "'file::colons.cc'" \
+ "\"this \"file\" has quotes.cc\"" \
+ "'this \"file\" has quotes.cc'" \
+ "'this 'file' has quotes.cc'" \
+ "\"this 'file' has quotes.cc\"" \
+ "\"spaces: and :colons.cc\"" \
+ "'more: :spaces: :and colons::.cc'" \
+ "C:/nonexist-with-windrive.cc"]
+
+foreach x $invalid_files {
+ # Remove any quoting from FILENAME for the error message.
+ test_break "$x:3" invalid_file [string trim $x \"']
+}
+foreach x [list "this_file_doesn't_exist.cc" \
+ "file::colons.cc" \
+ "'file::colons.cc'"] {
+ test_break "-source $x -line 3" \
+ invalid_file [string trim $x \"']
+}
+
+# Test that option lexing stops at whitespace boundaries
+test_break "-source this file has spaces.cc -line 3" \
+ invalid_file "this"
+
+test_break "-function function whitespace" \
+ invalid_function "function"
+
+test_break "-source $srcfile -function function whitespace" \
+ invalid_function_f "function" $srcfile
+
+test_break "-function main -label label whitespace" \
+ invalid_label "label" "main"
+
+# Test unmatched quotes.
+foreach x {"\"src-file.cc'" "'src-file.cc"} {
+ test_break "$x:3" unmatched_quote
+}
+
+test_break $srcfile invalid_function $srcfile
+foreach x {"foo" " foo" " foo "} {
+ # Trim any leading/trailing whitespace for error messages.
+ test_break "$srcfile:$x" invalid_function_f [string trim $x] $srcfile
+ test_break "-source $srcfile -function $x" \
+ invalid_function_f [string trim $x] $srcfile
+ test_break "$srcfile:main:$x" invalid_label [string trim $x] "main"
+ test_break "-source $srcfile -function main -label $x" \
+ invalid_label [string trim $x] "main"
+}
+
+foreach x $spaces {
+ test_break "$srcfile$x" unexpected "end of input"
+ test_break "$srcfile:main$x" unexpected "end of input"
+}
+
+test_break "${srcfile}::" invalid_function "${srcfile}::"
+test_break "$srcfile:3 1" unexpected_opt "number" "1"
+test_break "-source $srcfile -line 3 1" garbage "1"
+test_break "$srcfile:3 +100" unexpected_opt "number" "+100"
+test_break "-source $srcfile -line 3 +100" garbage "+100"
+test_break "$srcfile:3 -100" unexpected_opt "number" "-100"
+test_break "$srcfile:3 foo" unexpected_opt "string" "foo"
+test_break "-source $srcfile -line 3 foo" garbage "foo"
+
+foreach x $invalid_offsets {
+ test_break "$srcfile:$x" invalid_offset_f $x $srcfile
+ test_break "\"$srcfile:$x\"" invalid_offset_f $x $srcfile
+ test_break "'$srcfile:$x'" invalid_offset_f $x $srcfile
+ test_break "-source $srcfile -line $x" invalid_offset_f $x $srcfile
+}
+test_break "-source $srcfile -line -x" malformed_line_offset "-x"
+
+# Test invalid filespecs starting with function.
+foreach x {"foobar" "foo::bar" "foo.bar" "foo ." "foo bar" "foo 1" \
+ "foo 0" "foo +10" "foo -10" "foo +100" "foo -100"} {
+ test_break $x invalid_function $x
+ test_break "-function \"$x\"" invalid_function $x
+}
+
+foreach x $spaces {
+ test_break "main${x}there" invalid_label "there" "main"
+ if {[test_compiler_info {clang-*-*}]} { setup_xfail clang/14500 *-*-* }
+ test_break "main:here${x}" unexpected "end of input"
+}
+
+foreach x {"3" "+100" "-100" "foo"} {
+ test_break "main 3" invalid_function "main 3"
+ test_break "-function \"main $x\"" invalid_function "main $x"
+ test_break "main:here $x" invalid_label "here $x" "main"
+ test_break "-function main -label \"here $x\"" \
+ invalid_label "here $x" "main"
+}
+
+foreach x {"if" "task" "thread"} {
+ test_break $x invalid_function $x
+}
+
+test_break "'main.cc'flubber" unexpected_opt "string" "flubber"
+test_break "'main.cc',21" invalid_function "main.cc"
+test_break "'main.cc' " invalid_function "main.cc"
+test_break "'main.cc'3" unexpected_opt "number" "3"
+test_break "'main.cc'+3" unexpected_opt "number" "+3"
+
+# Test undefined convenience variables.
+set x {$zippo}
+test_break $x invalid_var_or_func $x
+test_break "$srcfile:$x" invalid_var_or_func_f $x $srcfile
+
+# Explicit linespec-specific tests
+test_break "-source $srcfile" source_incomplete
diff --git a/gdb/testsuite/gdb.mi/linespec-err-cp.cc b/gdb/testsuite/gdb.mi/linespec-err-cp.cc
new file mode 100644
index 0000000..19c92d8
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/linespec-err-cp.cc
@@ -0,0 +1,35 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2016 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+int
+myfunction (int aa)
+{
+ int i;
+
+ i = aa + 42;
+ return i; /* set breakpoint here */
+}
+
+int
+main (void)
+{
+ int a;
+
+ a = myfunction (a);
+
+ return a;
+}
diff --git a/gdb/testsuite/gdb.mi/linespec-err-cp.exp b/gdb/testsuite/gdb.mi/linespec-err-cp.exp
new file mode 100644
index 0000000..1a8682e
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/linespec-err-cp.exp
@@ -0,0 +1,57 @@
+# Copyright 2016 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Regression test for PR breakpoints/18303. Tests that the correct
+# errors is generated when setting a breakpoint in a non-existent
+# file with a Windows-style logical drive names and C++.
+
+if { [skip_cplus_tests] } { continue }
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+standard_testfile .cc
+set exefile $testfile
+
+if {[prepare_for_testing $testfile $exefile $srcfile {debug c++}]} {
+ return -1
+}
+
+gdb_exit
+if [mi_gdb_start] {
+ continue
+}
+
+# Turn off the pending breakpoint queries.
+mi_gdb_test "-interpreter-exec console \"set breakpoint pending off\"" \
+ {=cmd-param-changed,param=\"breakpoint pending\",.*\^done} \
+ "-interpreter-exec console \"set breakpoint pending off\""
+
+mi_run_to_main
+
+# Run to a location in the file.
+set bp_location [gdb_get_line_number "set breakpoint here"]
+
+mi_gdb_test "-break-insert ${srcfile}:${bp_location}" \
+ {\^done,bkpt=.number="2",type="breakpoint".*\}} "set breakpoint"
+
+mi_execute_to "exec-continue" "breakpoint-hit" "myfunction" ".*" ".*" "24" \
+ { "" "disp=\"keep\"" } "breakpoint hit"
+
+# Set a breakpoint in a C++ source file whose name contains a
+# Windows-style logical drive.
+mi_gdb_test \
+ "-break-insert -f \"c:/uu.cpp:13\"" \
+ ".*No source file named c:/uu.cpp.*"
--
1.7.0.4