diff mbox

[v2] PR 18303, Tolerate malformed input for lookup_symbol-called functions.

Message ID 1453944094-31885-1-git-send-email-donb@codesourcery.com
State New
Headers show

Commit Message

Don Breazeal Jan. 28, 2016, 1:21 a.m. UTC
This patch includes an updated version of the alternate fix that
Keith proposed for my patch that addressed PR breakpoints/18303.  Doug
has approved (in concept, at least) the code portion of the patch.

I've also done some renaming of the new tests and consolidated all three
new tests into this one patch along with Keith's patch for the code.

Thanks,
--Don

-----------

lookup_symbol is often called with user input.  Consequently, any
function called from lookup_symbol{,_in_language} should attempt to
deal with malformed input gracefully.  After all, malformed user
input is not a programming/API error.

This patch does not attempt to find/correct all instances of this.  It
only fixes locations in the code that trigger test suite failures.

This patch fixes PR breakpoints/18303, "Assertion: -breakpoint-insert
with windows paths of file in non-current directory".

The patch includes three new tests related to this.  One is just
gdb.linespec/ls-errs.exp copied and converted to use C++ instead of C, and
to add a case using a file name containing a Windows-style logical drive
specifier.  The others include an MI test to provide a regression test for
the specific case reported in PR 18303, and a C++ test for proper error
handling of access to a program variable when using a file scope specifier
that refers to a non-existent file.

Tested on x86_64 native Linux.

gdb/ChangeLog
2016-01-27  Keith Seitz  <keiths@redhat.com>

	PR breakpoints/18303
	* cp-namespace.c (cp_lookup_bare_symbol): Change assertion to
	look for "::" instead of simply ":".
	(cp_search_static_and_baseclasses): Return null_block_symbol for
	malformed input.
	Remove assertions.
	* cp-support.c (cp_find_first_component_aux): Do not return
	a prefix length for ':' unless the next character is also ':'.

gdb/testsuite/ChangeLog
2016-01-27  Don Breazeal  <donb@codesourcery.com>

	gdb.cp/scope-err.cc: New test program.
	gdb.cp/scope-err.exp: New test script.
	gdb.linespec/ls-errs-cp.cc: New test program.
	gdb.linespec/ls-errs-cp.exp: New test script.
	gdb.mi/mi-linespec-err-cp.cc: New test program.
	gdb.mi/mi-linespec-err-cp.exp: New test script.
---
 gdb/cp-namespace.c                          |   9 +-
 gdb/cp-support.c                            |   7 +-
 gdb/testsuite/gdb.cp/scope-err.cc           |  35 ++++
 gdb/testsuite/gdb.linespec/ls-errs-cp.cc    |  36 +++++
 gdb/testsuite/gdb.linespec/ls-errs-cp.exp   | 240 ++++++++++++++++++++++++++++
 gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc  |  35 ++++
 gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp |  57 +++++++
 7 files changed, 413 insertions(+), 6 deletions(-)

Comments

Pedro Alves Jan. 28, 2016, 12:06 p.m. UTC | #1
On 01/28/2016 01:21 AM, Don Breazeal wrote:
> The patch includes three new tests related to this.  One is just
> gdb.linespec/ls-errs.exp copied and converted to use C++ instead of C, and
> to add a case using a file name containing a Windows-style logical drive
> specifier.  
....
> gdb/testsuite/gdb.linespec/ls-errs-cp.cc    |  36 +++++
> gdb/testsuite/gdb.linespec/ls-errs-cp.exp   | 240 ++++++++++++++++++++++++++++
...

Can't we somehow reuse the existing test?  Say, either:

 - move the main body of ls-errs.exp a procedure, and call it twice,
   once for each language, or,

 - make gdb.linespec/ls-errs-cp.exp set some $language var and then
   source gdb.linespec/ls-errs.exp, like gdb.base/checkpoint-ns.exp.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 72002d6..016a42f 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -170,7 +170,7 @@  cp_lookup_bare_symbol (const struct language_defn *langdef,
      ':' may be in the args of a template spec.  This isn't intended to be
      a complete test, just cheap and documentary.  */
   if (strchr (name, '<') == NULL && strchr (name, '(') == NULL)
-    gdb_assert (strchr (name, ':') == NULL);
+    gdb_assert (strstr (name, "::") == NULL);
 
   sym = lookup_symbol_in_static_block (name, block, domain);
   if (sym.symbol != NULL)
@@ -246,10 +246,9 @@  cp_search_static_and_baseclasses (const char *name,
   struct block_symbol klass_sym;
   struct type *klass_type;
 
-  /* The test here uses <= instead of < because Fortran also uses this,
-     and the module.exp testcase will pass "modmany::" for NAME here.  */
-  gdb_assert (prefix_len + 2 <= strlen (name));
-  gdb_assert (name[prefix_len + 1] == ':');
+  /* Check for malformed input.  */
+  if (prefix_len + 2 > strlen (name) || name[prefix_len + 1] != ':')
+    return null_block_symbol;
 
   /* Find the name of the class and the name of the method, variable, etc.  */
 
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index df127c4..a71c6ad 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -1037,8 +1037,13 @@  cp_find_first_component_aux (const char *name, int permissive)
 	      return strlen (name);
 	    }
 	case '\0':
-	case ':':
 	  return index;
+	case ':':
+	  /* ':' marks a component iff the next character is also a ':'.
+	     Otherwise it is probably malformed input.  */
+	  if (name[index + 1] == ':')
+	    return index;
+	  break;
 	case 'o':
 	  /* Operator names can screw up the recursion.  */
 	  if (operator_possible
diff --git a/gdb/testsuite/gdb.cp/scope-err.cc b/gdb/testsuite/gdb.cp/scope-err.cc
new file mode 100644
index 0000000..19c92d8
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/scope-err.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.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/mi-linespec-err-cp.cc b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.cc
new file mode 100644
index 0000000..19c92d8
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/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/mi-linespec-err-cp.exp b/gdb/testsuite/gdb.mi/mi-linespec-err-cp.exp
new file mode 100644
index 0000000..1a8682e
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/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.*"