From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22443 invoked by alias); 26 Jun 2003 01:04:36 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 24683 invoked from network); 26 Jun 2003 00:27:11 -0000 Received: from unknown (HELO coconut.kealia.com) (216.101.126.244) by sources.redhat.com with SMTP; 26 Jun 2003 00:27:11 -0000 Received: from coconut.kealia.com (coconut.kealia.com [127.0.0.1]) by coconut.kealia.com (8.12.8/8.12.8) with ESMTP id h5Q0RAKR002477; Wed, 25 Jun 2003 17:27:10 -0700 Received: (from carlton@localhost) by coconut.kealia.com (8.12.8/8.12.8/Submit) id h5Q0R9WU002475; Wed, 25 Jun 2003 17:27:09 -0700 X-Authentication-Warning: coconut.kealia.com: carlton set sender to carlton@kealia.com using -f To: gdb-patches@sources.redhat.com Cc: Daniel Jacobowitz Subject: [rfa] partial fix for PR gdb/1245 From: David Carlton Date: Thu, 26 Jun 2003 01:04:00 -0000 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2003-06/txt/msg00783.txt.bz2 Here's a partial fix for PR gdb/1245, the one about an assertion failure coming from a demangled name like "int operator<". 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 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 * 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::bar" "C" gdb_test "maint cp first_component C > >::bar" "C > >" + + # 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<" } gdb_exit