This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[rfa] partial fix for PR gdb/1245


Here's a partial fix for PR gdb/1245, the one about an assertion
failure coming from a demangled name like "int operator<<foo>".  The
best fix there would be to fix the demangler to put a space betwen the
two <'s, but given that this is the second time we've run into that
assertion failure, it's probably best to change it into a complaint,
because demangled name weirdness really shouldn't leave GDB in an
unusable state.

So that's what this patch does.  It ensures that components found by
cp_find_first_component either end in a colon or are the entire
string.  If something else happens, it complains and returns the
entire string.

With this change to the function, some other gdb_asserts (e.g. the one
in cp_entire_prefix_len) become necessary.  I'm ambivalent about
removing correct assertions; I'm happy to either do so or not in this
case, depending on how you see fit.

The next question is: do we want to go any further with a fix for PR
gdb/1245?  On the one hand, the demangler's output really is
incorrect.  On the other hand, even if we fix the demangler's output,
GDB will never do anything useful with the output in this particular
situation, I think: it only occurs in the context of templated
functions, and the fact that their demangled names include the return
value means that the prefixes that we get from them aren't correct in
the first place.  Being a lazy person who has never submitted a patch
to libiberty, I'm inclined to put a real fix for this issue on the
back burner.

Tested on GCC 3.2, DWARF 2, i686-pc-linux-gnu; no new regressions.  OK
to commit on mainline and branch?

David Carlton
carlton@kealia.com

2003-06-25  David Carlton  <carlton@kealia.com>

	Band-aid for PR c++/1245.
	* Makefile.in (cp-support.o): Depend on complaints_h.
	* cp-support.c: Include complaints.h.  Add declaration for
	find_last_component.
	(cp_find_first_component): Separate code into
	cp_find_first_component_aux.
	(cp_find_first_component_aux): Call demangled_name_complaint.
	(demangled_name_complaint): New.

2003-06-25  David Carlton  <carlton@kealia.com>

	* gdb.c++/maint.exp (test_invalid_name): New.
	(test_first_component): Add tests for invalid names.

Index: cp-support.c
===================================================================
RCS file: /cvs/src/src/gdb/cp-support.c,v
retrieving revision 1.5
diff -u -p -r1.5 cp-support.c
--- cp-support.c	12 Jun 2003 15:33:45 -0000	1.5
+++ cp-support.c	26 Jun 2003 00:08:20 -0000
@@ -32,6 +32,16 @@
 #include "frame.h"
 #include "symtab.h"
 #include "block.h"
+#include "complaints.h"
+
+/* Functions related to demangled name parsing.  */
+
+static const char *find_last_component (const char *name);
+
+static unsigned int cp_find_first_component_aux (const char *name,
+						 int permissive);
+
+static void demangled_name_complaint (const char *name);
 
 /* Functions/variables related to overload resolution.  */
 
@@ -199,21 +209,31 @@ method_name_from_physname (const char *p
    boundary of the first component: so, given 'A::foo' or 'A::B::foo'
    it returns the 1, and given 'foo', it returns 0.  */
 
-/* Well, that's what it should do when called externally, but to make
-   the recursion easier, it also stops if it reaches an unexpected ')'
-   or '>'.  */
+/* The character in NAME indexed by the return value is guaranteed to
+   always be either ':' or '\0'.  */
 
 /* NOTE: carlton/2003-03-13: This function is currently only intended
    for internal use: it's probably not entirely safe when called on
-   user-generated input, because some of the 'index += 2' lines might
-   go past the end of malformed input.  */
+   user-generated input, because some of the 'index += 2' lines in
+   cp_find_first_component_aux might go past the end of malformed
+   input.  */
+
+unsigned int cp_find_first_component (const char *name)
+{
+  return cp_find_first_component_aux (name, 0);
+}
+
+/* Helper function for cp_find_first_component.  Like that function,
+   it returns the length of the first component of NAME, but to make
+   the recursion easier, it also stops if it reaches an unexpected ')'
+   or '>' if the value of PERMISSIVE is nonzero.  */
 
 /* Let's optimize away calls to strlen("operator").  */
 
 #define LENGTH_OF_OPERATOR 8
 
 unsigned int
-cp_find_first_component (const char *name)
+cp_find_first_component_aux (const char *name, int permissive)
 {
   unsigned int index = 0;
   /* Operator names can show up in unexpected places.  Since these can
@@ -234,11 +254,15 @@ cp_find_first_component (const char *nam
 	     terminating the component or a '::' between two
 	     components.  (Hence the '+ 2'.)  */
 	  index += 1;
-	  for (index += cp_find_first_component (name + index);
+	  for (index += cp_find_first_component_aux (name + index, 1);
 	       name[index] != '>';
-	       index += cp_find_first_component (name + index))
+	       index += cp_find_first_component_aux (name + index, 1))
 	    {
-	      gdb_assert (name[index] == ':');
+	      if (name[index] != ':')
+		{
+		  demangled_name_complaint (name);
+		  return strlen (name);
+		}
 	      index += 2;
 	    }
 	  operator_possible = 1;
@@ -246,17 +270,30 @@ cp_find_first_component (const char *nam
 	case '(':
 	  /* Similar comment as to '<'.  */
 	  index += 1;
-	  for (index += cp_find_first_component (name + index);
+	  for (index += cp_find_first_component_aux (name + index, 1);
 	       name[index] != ')';
-	       index += cp_find_first_component (name + index))
+	       index += cp_find_first_component_aux (name + index, 1))
 	    {
-	      gdb_assert (name[index] == ':');
+	      if (name[index] != ':')
+		{
+		  demangled_name_complaint (name);
+		  return strlen (name);
+		}
 	      index += 2;
 	    }
 	  operator_possible = 1;
 	  break;
 	case '>':
 	case ')':
+	  if (permissive)
+	    {
+	      return index;
+	    }
+	  else
+	    {
+	      demangled_name_complaint (name);
+	      return strlen (name);
+	    }
 	case '\0':
 	case ':':
 	  return index;
@@ -313,6 +350,16 @@ cp_find_first_component (const char *nam
 	  break;
 	}
     }
+}
+
+/* Complain about a demangled name that we don't know how to parse.
+   NAME is the demangled name in question.  */
+
+static void
+demangled_name_complaint (const char *name)
+{
+  complaint (&symfile_complaints,
+	     "unexpected demangled name '%s'", name);
 }
 
 /* If NAME is the fully-qualified name of a C++
Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.410
diff -u -p -r1.410 Makefile.in
--- Makefile.in	21 Jun 2003 23:14:43 -0000	1.410
+++ Makefile.in	26 Jun 2003 00:09:28 -0000
@@ -1645,7 +1645,7 @@ cp-namespace.o: cp-namespace.c $(defs_h)
 	$(symtab_h) $(symfile_h) $(gdb_assert_h) $(block_h)
 cp-support.o: cp-support.c $(defs_h) $(cp_support_h) $(gdb_string_h) \
 	$(demangle_h) $(gdb_assert_h) $(gdbcmd_h) $(dictionary_h) \
-	$(objfiles_h) $(frame_h) $(block_h)
+	$(objfiles_h) $(frame_h) $(block_h) $(complaints_h)
 cp-valprint.o: cp-valprint.c $(defs_h) $(gdb_obstack_h) $(symtab_h) \
 	$(gdbtypes_h) $(expression_h) $(value_h) $(command_h) $(gdbcmd_h) \
 	$(demangle_h) $(annotate_h) $(gdb_string_h) $(c_lang_h) $(target_h) \
Index: maint.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.c++/maint.exp,v
retrieving revision 1.2
diff -u -p -r1.2 maint.exp
--- maint.exp	23 Apr 2003 23:45:24 -0000	1.2
+++ maint.exp	26 Jun 2003 00:09:34 -0000
@@ -45,7 +45,19 @@ proc test_single_component {name} {
     gdb_test "maint cp first_component $name" "$matchname"
 }
 
+# This is used when NAME is invalid.
+proc test_invalid_name {name} {
+    set matchname [string_to_regexp "$name"]
+    gdb_test "maint cp first_component $name" \
+	"During symbol reading, unexpected demangled name '$matchname'.\r\n$matchname"
+}
+
 proc test_first_component {} {
+    # The function in question might complain; make sure that we see
+    # all complaints.
+
+    gdb_test "set complaints -1" ""
+
     test_single_component "foo"
     test_single_component "operator<<"
     test_single_component "operator>>"
@@ -79,6 +91,16 @@ proc test_first_component {} {
     gdb_test "maint cp first_component foo::bar::baz" "foo"
     gdb_test "maint cp first_component C<A>::bar" "C<A>"
     gdb_test "maint cp first_component C<std::basic_streambuf<wchar_t,std::char_traits<wchar_t> > >::bar" "C<std::basic_streambuf<wchar_t,std::char_traits<wchar_t> > >"
+
+    # Make sure we behave appropriately on invalid input.
+
+    # NOTE: carlton/2003-06-25: As of today, the demangler can in fact
+    # produce examples like the third case below: there really should
+    # be a space between the two <'s.  See PR gdb/1245.
+
+    test_invalid_name "foo<"
+    test_invalid_name "foo("
+    test_invalid_name "bool operator<<char>"
 }
 
 gdb_exit


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]