From patchwork Fri Jan 8 18:44:11 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Don Breazeal X-Patchwork-Id: 10300 Received: (qmail 34081 invoked by alias); 8 Jan 2016 18:44:22 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 33991 invoked by uid 89); 8 Jan 2016 18:44:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, FROM_12LTRDOM, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=no version=3.3.2 spammy=unmatched, execcontinue, exec-continue, myfunction X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 08 Jan 2016 18:44:15 +0000 Received: from svr-orw-fem-06.mgc.mentorg.com ([147.34.97.120]) by relay1.mentorg.com with esmtp id 1aHc1E-0003vg-4R from Don_Breazeal@mentor.com for gdb-patches@sourceware.org; Fri, 08 Jan 2016 10:44:12 -0800 Received: from build4-lucid-cs (147.34.91.1) by SVR-ORW-FEM-06.mgc.mentorg.com (147.34.97.120) with Microsoft SMTP Server id 14.3.224.2; Fri, 8 Jan 2016 10:44:12 -0800 Received: by build4-lucid-cs (Postfix, from userid 1905) id 82ACC43523; Fri, 8 Jan 2016 10:44:11 -0800 (PST) From: Don Breazeal To: Subject: [PATCH] Fix problem handling colon in linespec, PR breakpoints/18303 Date: Fri, 8 Jan 2016 10:44:11 -0800 Message-ID: <1452278651-8630-1-git-send-email-donb@codesourcery.com> MIME-Version: 1.0 X-IsSubscribed: yes 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 . */ + +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 . + +# 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 . */ + +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 . + +# 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.*"