Re: [RFA] Select a particular mangling of a demangled symbol in lookup_block_symbol

Re: [RFA] Select a particular mangling of a demangled symbol in lookup_block_symbol

[Michael Elizabeth Chastain]

dj>	A::A(int) [not-in-charge]

Can the input parser handle that syntax?  Suppose the user wants to
disassemble the not-in-charge constructor?

That's why I want to stick the information in the name:

  A::A(int)
  A::A$nic(int)

dj> Meanwhile, I'd like to put this patch in because it is a strict
dj> improvement over what we have.

I'll look at this particular patch so that I can offer an opinion.

Michael C

Re: [RFA] Select a particular mangling of a demangled symbol in lookup_block_symbol

[Michael Elizabeth Chastain]

Hi Daniel,

> Meanwhile, I'd like to put this patch in because it is a strict
> improvement over what we have.

I got some sleep so that I could look at this particular patch 
with fresh eyes.

I agree with you; this patch can never do harm (as far as I can tell).
So I don't object to it.

Michael C

Re: [RFA] Select a particular mangling of a demangled symbol in lookup_block_symbol

[Michael Elizabeth Chastain]

Hi Daniel,

> What we really SHOULD do is set it on both constructors silently, without
> even acknowledging that they are different functions, or else offer the user
> the choice.  My preference is actually for the former.

There's more grossness than that.  Suppose that the constructor calls
another function foo().  Suppose the user sets a breakpoint on foo()
and looks at the stack and sees the constructor.  Suppose the user looks
at the assembly code for the constructor.  Suppose the user continues,
and takes the breakpoint on foo() again, and disassembles the constructor
again, and sees different assembly code for the constructor.

Games like that impair the user's trust in gdb.  They start with a lie,
And they lead to endless scenarios where you have to fix things up in
order to maintain the lie.

I would prefer that the different blocks of object code with different
mangled names have different demangled names as well, such as "Foo::Foo()"
and "Foo::Foo$nic()".  (I guess if the user actually names one of his
member functions Foo$nic then we are screwed ... is there any possible
name which is not legal C++ member name?)

Michael C

Re: [RFA] Select a particular mangling of a demangled symbol in lookup_block_symbol

[Daniel Jacobowitz]

On Fri, Feb 15, 2002 at 06:38:37PM -0600, Michael Elizabeth Chastain wrote:
> Hi Daniel,
> 
> > What we really SHOULD do is set it on both constructors silently, without
> > even acknowledging that they are different functions, or else offer the user
> > the choice.  My preference is actually for the former.
> 
> There's more grossness than that.  Suppose that the constructor calls
> another function foo().  Suppose the user sets a breakpoint on foo()
> and looks at the stack and sees the constructor.  Suppose the user looks
> at the assembly code for the constructor.  Suppose the user continues,
> and takes the breakpoint on foo() again, and disassembles the constructor
> again, and sees different assembly code for the constructor.
> 
> Games like that impair the user's trust in gdb.  They start with a lie,
> And they lead to endless scenarios where you have to fix things up in
> order to maintain the lie.
> 
> I would prefer that the different blocks of object code with different
> mangled names have different demangled names as well, such as "Foo::Foo()"
> and "Foo::Foo$nic()".  (I guess if the user actually names one of his
> member functions Foo$nic then we are screwed ... is there any possible
> name which is not legal C++ member name?)

We -could- just use the names the C++ demangler provides with
DMGL_VERBOSE for some printing.  However, we want to be careful about
that.  They look like:
	A::A[not-in-charge](int).
and for max confusion:
	A::~A [not-in-charge](int)
[I should report that, I suppose...]

I would prefer to recognize this using our constructor machinery and
describe this as something like:
	A::A(int) [not-in-charge]
which is more aesthetic and lets us have a better conception of what is
going on.  Bear in mind that sometimes we look up "A::A" wanting to get
that.  Our overload handling needs some work, which I'm trying to
figure out right now.

Meanwhile, I'd like to put this patch in because it is a strict
improvement over what we have.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

[RFA] Select a particular mangling of a demangled symbol in lookup_block_symbol

[Daniel Jacobowitz]

I just described the problem this patch addresses in my testsuite patch;
perhaps not the best place :)  Here's the relevant bit:

  - Multiple symbols with the same demangled name.  We can work around this
   for stabs, because we have the physname.  We don't have that option for   
   DWARF-2, and we shouldn't need to for stabs.  I have a patch for the
   workaround.  We get the [not-in-charge] constructor by default,
   unfortunately.


What this means is that we call lookup_block_symbol on something like:
  _ZN3fooC1ERS_
but get the information for:
  _ZN3fooC2ERS_

The breakpoint ends up on the base-not-in-charge constructor.

What we really SHOULD do is set it on both constructors silently, without
even acknowledging that they are different functions, or else offer the user
the choice.  My preference is actually for the former.  That requires
support for a single function existing in multiple places, which will also
give us nice things like better support for inlined functions with DWARF-2
(which will always be somewhat shoddy due to the nature of inlining, in that
it only occurs with lots of other optimization - but we can do much better
than we do).

Is this patch OK, or is it deemed too gross?

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

2002-02-14  Daniel Jacobowitz  <drow@mvista.com>

	* symtab.h (lookup_block_symbol): Add mangled_name argument
	to prototype.

	* symmisc.c (maintenance_check_symtabs): Call lookup_block_symbol
	with new mangled_name argument.
	* linespec.c (decode_line_1): Likewise.
	* valops (value_of_this): Likewise.
	* symtab.c (lookup_transparent_type): Likewise.
	(lookup_symbol_aux): Likewise.  Accept new mangled_name argument.
	(lookup_symbol): If we are given a mangled name, pass it down
	to lookup_symbol_aux.
	(lookup_block_symbol): If we are given a mangled name to check
	against, only return symbols which match it.

Index: linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.14
diff -p -u -r1.14 linespec.c
--- linespec.c	2002/02/02 03:42:58	1.14
+++ linespec.c	2002/02/14 23:34:24
@@ -1180,7 +1180,7 @@ symbol_found:			/* We also jump here fro
 	    {
 	      struct blockvector *bv = BLOCKVECTOR (sym_symtab);
 	      struct block *b = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
-	      if (lookup_block_symbol (b, copy, VAR_NAMESPACE) != NULL)
+	      if (lookup_block_symbol (b, copy, NULL, VAR_NAMESPACE) != NULL)
 		build_canonical_line_spec (values.sals, copy, canonical);
 	    }
 	  return values;
Index: symmisc.c
===================================================================
RCS file: /cvs/src/src/gdb/symmisc.c,v
retrieving revision 1.7
diff -p -u -r1.7 symmisc.c
--- symmisc.c	2001/12/02 22:38:23	1.7
+++ symmisc.c	2002/02/14 23:34:24
@@ -959,7 +959,7 @@ maintenance_check_symtabs (char *ignore,
     while (length--)
       {
 	sym = lookup_block_symbol (b, SYMBOL_NAME (*psym),
-				   SYMBOL_NAMESPACE (*psym));
+				   NULL, SYMBOL_NAMESPACE (*psym));
 	if (!sym)
 	  {
 	    printf_filtered ("Static symbol `");
@@ -976,7 +976,7 @@ maintenance_check_symtabs (char *ignore,
     while (length--)
       {
 	sym = lookup_block_symbol (b, SYMBOL_NAME (*psym),
-				   SYMBOL_NAMESPACE (*psym));
+				   NULL, SYMBOL_NAMESPACE (*psym));
 	if (!sym)
 	  {
 	    printf_filtered ("Global symbol `");
Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.54
diff -p -u -r1.54 symtab.c
--- symtab.c	2002/02/11 03:21:53	1.54
+++ symtab.c	2002/02/14 23:34:24
@@ -80,11 +80,12 @@ static struct partial_symbol *lookup_par
 						     const char *, int,
 						     namespace_enum);
 
-static struct symbol *lookup_symbol_aux (const char *name, const
-					 struct block *block, const
-					 namespace_enum namespace, int
-					 *is_a_field_of_this, struct
-					 symtab **symtab);
+static struct symbol *lookup_symbol_aux (const char *name,
+					 const char *mangled_name,
+					 const struct block *block,
+					 const namespace_enum namespace,
+					 int *is_a_field_of_this,
+					 struct symtab **symtab);
 
 
 static struct symbol *find_active_alias (struct symbol *sym, CORE_ADDR addr);
@@ -567,6 +568,7 @@ lookup_symbol (const char *name, const s
 {
   char *modified_name = NULL;
   char *modified_name2 = NULL;
+  const char *mangled_name = NULL;
   int needtofreename = 0;
   struct symbol *returnval;
 
@@ -592,13 +594,14 @@ lookup_symbol (const char *name, const s
       modified_name2 = cplus_demangle (modified_name, DMGL_ANSI | DMGL_PARAMS);
       if (modified_name2)
 	{
+	  mangled_name = name;
 	  modified_name = modified_name2;
 	  needtofreename = 1;
 	}
     }
 
-  returnval = lookup_symbol_aux (modified_name, block, namespace,
-				 is_a_field_of_this, symtab);
+  returnval = lookup_symbol_aux (modified_name, mangled_name, block,
+				 namespace, is_a_field_of_this, symtab);
   if (needtofreename)
     xfree (modified_name2);
 
@@ -606,9 +609,9 @@ lookup_symbol (const char *name, const s
 }
 
 static struct symbol *
-lookup_symbol_aux (const char *name, const struct block *block,
-	       const namespace_enum namespace, int *is_a_field_of_this,
-	       struct symtab **symtab)
+lookup_symbol_aux (const char *name, const char *mangled_name,
+		   const struct block *block, const namespace_enum namespace,
+		   int *is_a_field_of_this, struct symtab **symtab)
 {
   register struct symbol *sym;
   register struct symtab *s = NULL;
@@ -623,7 +626,7 @@ lookup_symbol_aux (const char *name, con
 
   while (block != 0)
     {
-      sym = lookup_block_symbol (block, name, namespace);
+      sym = lookup_block_symbol (block, name, mangled_name, namespace);
       if (sym)
 	{
 	  block_found = block;
@@ -676,7 +679,7 @@ lookup_symbol_aux (const char *name, con
 	if (BLOCK_START (b) <= BLOCK_START (block)
 	    && BLOCK_END (b) > BLOCK_START (block))
 	  {
-	    sym = lookup_block_symbol (b, name, VAR_NAMESPACE);
+	    sym = lookup_block_symbol (b, name, mangled_name, VAR_NAMESPACE);
 	    if (sym)
 	      {
 		block_found = b;
@@ -714,7 +717,7 @@ lookup_symbol_aux (const char *name, con
   {
     bv = BLOCKVECTOR (s);
     block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
-    sym = lookup_block_symbol (block, name, namespace);
+    sym = lookup_block_symbol (block, name, mangled_name, namespace);
     if (sym)
       {
 	block_found = block;
@@ -743,14 +746,14 @@ lookup_symbol_aux (const char *name, con
 	      bv = BLOCKVECTOR (s);
 	      block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
 	      sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
-					 namespace);
+					 mangled_name, namespace);
 	      /* We kept static functions in minimal symbol table as well as
 	         in static scope. We want to find them in the symbol table. */
 	      if (!sym)
 		{
 		  block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
 		  sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
-					     namespace);
+					     mangled_name, namespace);
 		}
 
 	      /* sym == 0 if symbol was found in the minimal symbol table
@@ -776,7 +779,7 @@ lookup_symbol_aux (const char *name, con
 	    {
 	      /* This is a mangled variable, look it up by its
 	         mangled name.  */
-	      return lookup_symbol_aux (SYMBOL_NAME (msymbol), block,
+	      return lookup_symbol_aux (SYMBOL_NAME (msymbol), mangled_name, block,
 					namespace, is_a_field_of_this, symtab);
 	    }
 	  /* There are no debug symbols for this file, or we are looking
@@ -794,7 +797,7 @@ lookup_symbol_aux (const char *name, con
 	s = PSYMTAB_TO_SYMTAB (ps);
 	bv = BLOCKVECTOR (s);
 	block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
-	sym = lookup_block_symbol (block, name, namespace);
+	sym = lookup_block_symbol (block, name, mangled_name, namespace);
 	if (!sym)
 	  {
 	    /* This shouldn't be necessary, but as a last resort
@@ -803,7 +806,7 @@ lookup_symbol_aux (const char *name, con
 	     * the psymtab gets it wrong in some cases.
 	     */
 	    block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
-	    sym = lookup_block_symbol (block, name, namespace);
+	    sym = lookup_block_symbol (block, name, mangled_name, namespace);
 	    if (!sym)
 	      error ("Internal: global symbol `%s' found in %s psymtab but not in symtab.\n\
 %s may be an inlined function, or may be a template function\n\
@@ -827,7 +830,7 @@ lookup_symbol_aux (const char *name, con
   {
     bv = BLOCKVECTOR (s);
     block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
-    sym = lookup_block_symbol (block, name, namespace);
+    sym = lookup_block_symbol (block, name, mangled_name, namespace);
     if (sym)
       {
 	block_found = block;
@@ -844,7 +847,7 @@ lookup_symbol_aux (const char *name, con
 	s = PSYMTAB_TO_SYMTAB (ps);
 	bv = BLOCKVECTOR (s);
 	block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
-	sym = lookup_block_symbol (block, name, namespace);
+	sym = lookup_block_symbol (block, name, mangled_name, namespace);
 	if (!sym)
 	  {
 	    /* This shouldn't be necessary, but as a last resort
@@ -853,7 +856,7 @@ lookup_symbol_aux (const char *name, con
 	     * the psymtab gets it wrong in some cases.
 	     */
 	    block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
-	    sym = lookup_block_symbol (block, name, namespace);
+	    sym = lookup_block_symbol (block, name, mangled_name, namespace);
 	    if (!sym)
 	      error ("Internal: static symbol `%s' found in %s psymtab but not in symtab.\n\
 %s may be an inlined function, or may be a template function\n\
@@ -910,14 +913,14 @@ lookup_symbol_aux (const char *name, con
 	      bv = BLOCKVECTOR (s);
 	      block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
 	      sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
-					 namespace);
+					 mangled_name, namespace);
 	      /* We kept static functions in minimal symbol table as well as
 	         in static scope. We want to find them in the symbol table. */
 	      if (!sym)
 		{
 		  block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
 		  sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
-					     namespace);
+					     mangled_name, namespace);
 		}
 	      /* If we found one, return it */
 	      if (sym)
@@ -954,8 +957,9 @@ lookup_symbol_aux (const char *name, con
 		   && MSYMBOL_TYPE (msymbol) != mst_file_text
 		   && !STREQ (name, SYMBOL_NAME (msymbol)))
 	    {
-	      return lookup_symbol_aux (SYMBOL_NAME (msymbol), block,
-					namespace, is_a_field_of_this, symtab);
+	      return lookup_symbol_aux (SYMBOL_NAME (msymbol), mangled_name,
+					block, namespace, is_a_field_of_this,
+					symtab);
 	    }
 	}
     }
@@ -1081,7 +1085,7 @@ lookup_transparent_type (const char *nam
   {
     bv = BLOCKVECTOR (s);
     block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
-    sym = lookup_block_symbol (block, name, STRUCT_NAMESPACE);
+    sym = lookup_block_symbol (block, name, NULL, STRUCT_NAMESPACE);
     if (sym && !TYPE_IS_OPAQUE (SYMBOL_TYPE (sym)))
       {
 	return SYMBOL_TYPE (sym);
@@ -1095,7 +1099,7 @@ lookup_transparent_type (const char *nam
 	s = PSYMTAB_TO_SYMTAB (ps);
 	bv = BLOCKVECTOR (s);
 	block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
-	sym = lookup_block_symbol (block, name, STRUCT_NAMESPACE);
+	sym = lookup_block_symbol (block, name, NULL, STRUCT_NAMESPACE);
 	if (!sym)
 	  {
 	    /* This shouldn't be necessary, but as a last resort
@@ -1104,7 +1108,7 @@ lookup_transparent_type (const char *nam
 	     * the psymtab gets it wrong in some cases.
 	     */
 	    block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
-	    sym = lookup_block_symbol (block, name, STRUCT_NAMESPACE);
+	    sym = lookup_block_symbol (block, name, NULL, STRUCT_NAMESPACE);
 	    if (!sym)
 	      error ("Internal: global symbol `%s' found in %s psymtab but not in symtab.\n\
 %s may be an inlined function, or may be a template function\n\
@@ -1128,7 +1132,7 @@ lookup_transparent_type (const char *nam
   {
     bv = BLOCKVECTOR (s);
     block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
-    sym = lookup_block_symbol (block, name, STRUCT_NAMESPACE);
+    sym = lookup_block_symbol (block, name, NULL, STRUCT_NAMESPACE);
     if (sym && !TYPE_IS_OPAQUE (SYMBOL_TYPE (sym)))
       {
 	return SYMBOL_TYPE (sym);
@@ -1142,7 +1146,7 @@ lookup_transparent_type (const char *nam
 	s = PSYMTAB_TO_SYMTAB (ps);
 	bv = BLOCKVECTOR (s);
 	block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
-	sym = lookup_block_symbol (block, name, STRUCT_NAMESPACE);
+	sym = lookup_block_symbol (block, name, NULL, STRUCT_NAMESPACE);
 	if (!sym)
 	  {
 	    /* This shouldn't be necessary, but as a last resort
@@ -1151,7 +1155,7 @@ lookup_transparent_type (const char *nam
 	     * the psymtab gets it wrong in some cases.
 	     */
 	    block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
-	    sym = lookup_block_symbol (block, name, STRUCT_NAMESPACE);
+	    sym = lookup_block_symbol (block, name, NULL, STRUCT_NAMESPACE);
 	    if (!sym)
 	      error ("Internal: static symbol `%s' found in %s psymtab but not in symtab.\n\
 %s may be an inlined function, or may be a template function\n\
@@ -1195,10 +1199,15 @@ find_main_psymtab (void)
    binary search terminates, we drop through and do a straight linear
    search on the symbols.  Each symbol which is marked as being a C++
    symbol (language_cplus set) has both the encoded and non-encoded names
-   tested for a match. */
+   tested for a match.
+
+   If MANGLED_NAME is non-NULL, verify that any symbol we find has this
+   particular mangled name.
+*/
 
 struct symbol *
 lookup_block_symbol (register const struct block *block, const char *name,
+		     const char *mangled_name,
 		     const namespace_enum namespace)
 {
   register int bot, top, inc;
@@ -1258,14 +1267,19 @@ lookup_block_symbol (register const stru
          return the first one; I believe it is now impossible for us
          to encounter two symbols with the same name and namespace
          here, because blocks containing argument symbols are no
-         longer sorted.  */
+         longer sorted.  The exception is for C++, where multiple functions
+	 (cloned constructors / destructors, in particular) can have
+	 the same demangled name.  So if we have a particular
+	 mangled name to match, try to do so.  */
 
       top = BLOCK_NSYMS (block);
       while (bot < top)
 	{
 	  sym = BLOCK_SYM (block, bot);
-	  if (SYMBOL_NAMESPACE (sym) == namespace &&
-	      SYMBOL_MATCHES_NAME (sym, name))
+	  if (SYMBOL_NAMESPACE (sym) == namespace
+	      && (mangled_name
+		  ? strcmp (SYMBOL_NAME (sym), mangled_name) == 0
+		  : SYMBOL_MATCHES_NAME (sym, name)))
 	    {
 	      return sym;
 	    }
@@ -1297,8 +1311,10 @@ lookup_block_symbol (register const stru
       while (bot < top)
 	{
 	  sym = BLOCK_SYM (block, bot);
-	  if (SYMBOL_NAMESPACE (sym) == namespace &&
-	      SYMBOL_MATCHES_NAME (sym, name))
+	  if (SYMBOL_NAMESPACE (sym) == namespace
+	      && (mangled_name
+		  ? strcmp (SYMBOL_NAME (sym), mangled_name) == 0
+		  : SYMBOL_MATCHES_NAME (sym, name)))
 	    {
 	      /* If SYM has aliases, then use any alias that is active
 	         at the current PC.  If no alias is active at the current
Index: symtab.h
===================================================================
RCS file: /cvs/src/src/gdb/symtab.h,v
retrieving revision 1.26
diff -p -u -r1.26 symtab.h
--- symtab.h	2001/12/21 22:32:37	1.26
+++ symtab.h	2002/02/14 23:34:24
@@ -1095,6 +1095,7 @@ extern struct symbol *lookup_symbol (con
 /* lookup a symbol by name, within a specified block */
 
 extern struct symbol *lookup_block_symbol (const struct block *, const char *,
+					   const char *,
 					   const namespace_enum);
 
 /* lookup a [struct, union, enum] by name, within a specified block */
Index: valops.c
===================================================================
RCS file: /cvs/src/src/gdb/valops.c,v
retrieving revision 1.51
diff -p -u -r1.51 valops.c
--- valops.c	2002/02/10 05:50:34	1.51
+++ valops.c	2002/02/14 23:34:25
@@ -3249,7 +3249,7 @@ value_of_this (int complain)
 
   /* Calling lookup_block_symbol is necessary to get the LOC_REGISTER
      symbol instead of the LOC_ARG one (if both exist).  */
-  sym = lookup_block_symbol (b, funny_this, VAR_NAMESPACE);
+  sym = lookup_block_symbol (b, funny_this, NULL, VAR_NAMESPACE);
   if (sym == NULL)
     {
       if (complain)

Re: [RFA] Select a particular mangling of a demangled symbol in lookup_block_symbol

[Daniel Jacobowitz]

Could someone please review this patch from last month?  Thanks.

On Thu, Feb 14, 2002 at 06:55:03PM -0500, Daniel Jacobowitz wrote:
> I just described the problem this patch addresses in my testsuite patch;
> perhaps not the best place :)  Here's the relevant bit:
> 
>   - Multiple symbols with the same demangled name.  We can work around this
>    for stabs, because we have the physname.  We don't have that option for   
>    DWARF-2, and we shouldn't need to for stabs.  I have a patch for the
>    workaround.  We get the [not-in-charge] constructor by default,
>    unfortunately.
> 
> 
> What this means is that we call lookup_block_symbol on something like:
>   _ZN3fooC1ERS_
> but get the information for:
>   _ZN3fooC2ERS_
> 
> The breakpoint ends up on the base-not-in-charge constructor.
> 
> What we really SHOULD do is set it on both constructors silently, without
> even acknowledging that they are different functions, or else offer the user
> the choice.  My preference is actually for the former.  That requires
> support for a single function existing in multiple places, which will also
> give us nice things like better support for inlined functions with DWARF-2
> (which will always be somewhat shoddy due to the nature of inlining, in that
> it only occurs with lots of other optimization - but we can do much better
> than we do).
> 
> Is this patch OK, or is it deemed too gross?
> 
> -- 
> Daniel Jacobowitz                           Carnegie Mellon University
> MontaVista Software                         Debian GNU/Linux Developer
> 
> 2002-02-14  Daniel Jacobowitz  <drow@mvista.com>
> 
> 	* symtab.h (lookup_block_symbol): Add mangled_name argument
> 	to prototype.
> 
> 	* symmisc.c (maintenance_check_symtabs): Call lookup_block_symbol
> 	with new mangled_name argument.
> 	* linespec.c (decode_line_1): Likewise.
> 	* valops (value_of_this): Likewise.
> 	* symtab.c (lookup_transparent_type): Likewise.
> 	(lookup_symbol_aux): Likewise.  Accept new mangled_name argument.
> 	(lookup_symbol): If we are given a mangled name, pass it down
> 	to lookup_symbol_aux.
> 	(lookup_block_symbol): If we are given a mangled name to check
> 	against, only return symbols which match it.
> 
> Index: linespec.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/linespec.c,v
> retrieving revision 1.14
> diff -p -u -r1.14 linespec.c
> --- linespec.c	2002/02/02 03:42:58	1.14
> +++ linespec.c	2002/02/14 23:34:24
> @@ -1180,7 +1180,7 @@ symbol_found:			/* We also jump here fro
>  	    {
>  	      struct blockvector *bv = BLOCKVECTOR (sym_symtab);
>  	      struct block *b = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> -	      if (lookup_block_symbol (b, copy, VAR_NAMESPACE) != NULL)
> +	      if (lookup_block_symbol (b, copy, NULL, VAR_NAMESPACE) != NULL)
>  		build_canonical_line_spec (values.sals, copy, canonical);
>  	    }
>  	  return values;
> Index: symmisc.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/symmisc.c,v
> retrieving revision 1.7
> diff -p -u -r1.7 symmisc.c
> --- symmisc.c	2001/12/02 22:38:23	1.7
> +++ symmisc.c	2002/02/14 23:34:24
> @@ -959,7 +959,7 @@ maintenance_check_symtabs (char *ignore,
>      while (length--)
>        {
>  	sym = lookup_block_symbol (b, SYMBOL_NAME (*psym),
> -				   SYMBOL_NAMESPACE (*psym));
> +				   NULL, SYMBOL_NAMESPACE (*psym));
>  	if (!sym)
>  	  {
>  	    printf_filtered ("Static symbol `");
> @@ -976,7 +976,7 @@ maintenance_check_symtabs (char *ignore,
>      while (length--)
>        {
>  	sym = lookup_block_symbol (b, SYMBOL_NAME (*psym),
> -				   SYMBOL_NAMESPACE (*psym));
> +				   NULL, SYMBOL_NAMESPACE (*psym));
>  	if (!sym)
>  	  {
>  	    printf_filtered ("Global symbol `");
> Index: symtab.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/symtab.c,v
> retrieving revision 1.54
> diff -p -u -r1.54 symtab.c
> --- symtab.c	2002/02/11 03:21:53	1.54
> +++ symtab.c	2002/02/14 23:34:24
> @@ -80,11 +80,12 @@ static struct partial_symbol *lookup_par
>  						     const char *, int,
>  						     namespace_enum);
>  
> -static struct symbol *lookup_symbol_aux (const char *name, const
> -					 struct block *block, const
> -					 namespace_enum namespace, int
> -					 *is_a_field_of_this, struct
> -					 symtab **symtab);
> +static struct symbol *lookup_symbol_aux (const char *name,
> +					 const char *mangled_name,
> +					 const struct block *block,
> +					 const namespace_enum namespace,
> +					 int *is_a_field_of_this,
> +					 struct symtab **symtab);
>  
>  
>  static struct symbol *find_active_alias (struct symbol *sym, CORE_ADDR addr);
> @@ -567,6 +568,7 @@ lookup_symbol (const char *name, const s
>  {
>    char *modified_name = NULL;
>    char *modified_name2 = NULL;
> +  const char *mangled_name = NULL;
>    int needtofreename = 0;
>    struct symbol *returnval;
>  
> @@ -592,13 +594,14 @@ lookup_symbol (const char *name, const s
>        modified_name2 = cplus_demangle (modified_name, DMGL_ANSI | DMGL_PARAMS);
>        if (modified_name2)
>  	{
> +	  mangled_name = name;
>  	  modified_name = modified_name2;
>  	  needtofreename = 1;
>  	}
>      }
>  
> -  returnval = lookup_symbol_aux (modified_name, block, namespace,
> -				 is_a_field_of_this, symtab);
> +  returnval = lookup_symbol_aux (modified_name, mangled_name, block,
> +				 namespace, is_a_field_of_this, symtab);
>    if (needtofreename)
>      xfree (modified_name2);
>  
> @@ -606,9 +609,9 @@ lookup_symbol (const char *name, const s
>  }
>  
>  static struct symbol *
> -lookup_symbol_aux (const char *name, const struct block *block,
> -	       const namespace_enum namespace, int *is_a_field_of_this,
> -	       struct symtab **symtab)
> +lookup_symbol_aux (const char *name, const char *mangled_name,
> +		   const struct block *block, const namespace_enum namespace,
> +		   int *is_a_field_of_this, struct symtab **symtab)
>  {
>    register struct symbol *sym;
>    register struct symtab *s = NULL;
> @@ -623,7 +626,7 @@ lookup_symbol_aux (const char *name, con
>  
>    while (block != 0)
>      {
> -      sym = lookup_block_symbol (block, name, namespace);
> +      sym = lookup_block_symbol (block, name, mangled_name, namespace);
>        if (sym)
>  	{
>  	  block_found = block;
> @@ -676,7 +679,7 @@ lookup_symbol_aux (const char *name, con
>  	if (BLOCK_START (b) <= BLOCK_START (block)
>  	    && BLOCK_END (b) > BLOCK_START (block))
>  	  {
> -	    sym = lookup_block_symbol (b, name, VAR_NAMESPACE);
> +	    sym = lookup_block_symbol (b, name, mangled_name, VAR_NAMESPACE);
>  	    if (sym)
>  	      {
>  		block_found = b;
> @@ -714,7 +717,7 @@ lookup_symbol_aux (const char *name, con
>    {
>      bv = BLOCKVECTOR (s);
>      block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> -    sym = lookup_block_symbol (block, name, namespace);
> +    sym = lookup_block_symbol (block, name, mangled_name, namespace);
>      if (sym)
>        {
>  	block_found = block;
> @@ -743,14 +746,14 @@ lookup_symbol_aux (const char *name, con
>  	      bv = BLOCKVECTOR (s);
>  	      block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
>  	      sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
> -					 namespace);
> +					 mangled_name, namespace);
>  	      /* We kept static functions in minimal symbol table as well as
>  	         in static scope. We want to find them in the symbol table. */
>  	      if (!sym)
>  		{
>  		  block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
>  		  sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
> -					     namespace);
> +					     mangled_name, namespace);
>  		}
>  
>  	      /* sym == 0 if symbol was found in the minimal symbol table
> @@ -776,7 +779,7 @@ lookup_symbol_aux (const char *name, con
>  	    {
>  	      /* This is a mangled variable, look it up by its
>  	         mangled name.  */
> -	      return lookup_symbol_aux (SYMBOL_NAME (msymbol), block,
> +	      return lookup_symbol_aux (SYMBOL_NAME (msymbol), mangled_name, block,
>  					namespace, is_a_field_of_this, symtab);
>  	    }
>  	  /* There are no debug symbols for this file, or we are looking
> @@ -794,7 +797,7 @@ lookup_symbol_aux (const char *name, con
>  	s = PSYMTAB_TO_SYMTAB (ps);
>  	bv = BLOCKVECTOR (s);
>  	block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> -	sym = lookup_block_symbol (block, name, namespace);
> +	sym = lookup_block_symbol (block, name, mangled_name, namespace);
>  	if (!sym)
>  	  {
>  	    /* This shouldn't be necessary, but as a last resort
> @@ -803,7 +806,7 @@ lookup_symbol_aux (const char *name, con
>  	     * the psymtab gets it wrong in some cases.
>  	     */
>  	    block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> -	    sym = lookup_block_symbol (block, name, namespace);
> +	    sym = lookup_block_symbol (block, name, mangled_name, namespace);
>  	    if (!sym)
>  	      error ("Internal: global symbol `%s' found in %s psymtab but not in symtab.\n\
>  %s may be an inlined function, or may be a template function\n\
> @@ -827,7 +830,7 @@ lookup_symbol_aux (const char *name, con
>    {
>      bv = BLOCKVECTOR (s);
>      block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> -    sym = lookup_block_symbol (block, name, namespace);
> +    sym = lookup_block_symbol (block, name, mangled_name, namespace);
>      if (sym)
>        {
>  	block_found = block;
> @@ -844,7 +847,7 @@ lookup_symbol_aux (const char *name, con
>  	s = PSYMTAB_TO_SYMTAB (ps);
>  	bv = BLOCKVECTOR (s);
>  	block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> -	sym = lookup_block_symbol (block, name, namespace);
> +	sym = lookup_block_symbol (block, name, mangled_name, namespace);
>  	if (!sym)
>  	  {
>  	    /* This shouldn't be necessary, but as a last resort
> @@ -853,7 +856,7 @@ lookup_symbol_aux (const char *name, con
>  	     * the psymtab gets it wrong in some cases.
>  	     */
>  	    block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> -	    sym = lookup_block_symbol (block, name, namespace);
> +	    sym = lookup_block_symbol (block, name, mangled_name, namespace);
>  	    if (!sym)
>  	      error ("Internal: static symbol `%s' found in %s psymtab but not in symtab.\n\
>  %s may be an inlined function, or may be a template function\n\
> @@ -910,14 +913,14 @@ lookup_symbol_aux (const char *name, con
>  	      bv = BLOCKVECTOR (s);
>  	      block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
>  	      sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
> -					 namespace);
> +					 mangled_name, namespace);
>  	      /* We kept static functions in minimal symbol table as well as
>  	         in static scope. We want to find them in the symbol table. */
>  	      if (!sym)
>  		{
>  		  block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
>  		  sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
> -					     namespace);
> +					     mangled_name, namespace);
>  		}
>  	      /* If we found one, return it */
>  	      if (sym)
> @@ -954,8 +957,9 @@ lookup_symbol_aux (const char *name, con
>  		   && MSYMBOL_TYPE (msymbol) != mst_file_text
>  		   && !STREQ (name, SYMBOL_NAME (msymbol)))
>  	    {
> -	      return lookup_symbol_aux (SYMBOL_NAME (msymbol), block,
> -					namespace, is_a_field_of_this, symtab);
> +	      return lookup_symbol_aux (SYMBOL_NAME (msymbol), mangled_name,
> +					block, namespace, is_a_field_of_this,
> +					symtab);
>  	    }
>  	}
>      }
> @@ -1081,7 +1085,7 @@ lookup_transparent_type (const char *nam
>    {
>      bv = BLOCKVECTOR (s);
>      block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> -    sym = lookup_block_symbol (block, name, STRUCT_NAMESPACE);
> +    sym = lookup_block_symbol (block, name, NULL, STRUCT_NAMESPACE);
>      if (sym && !TYPE_IS_OPAQUE (SYMBOL_TYPE (sym)))
>        {
>  	return SYMBOL_TYPE (sym);
> @@ -1095,7 +1099,7 @@ lookup_transparent_type (const char *nam
>  	s = PSYMTAB_TO_SYMTAB (ps);
>  	bv = BLOCKVECTOR (s);
>  	block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> -	sym = lookup_block_symbol (block, name, STRUCT_NAMESPACE);
> +	sym = lookup_block_symbol (block, name, NULL, STRUCT_NAMESPACE);
>  	if (!sym)
>  	  {
>  	    /* This shouldn't be necessary, but as a last resort
> @@ -1104,7 +1108,7 @@ lookup_transparent_type (const char *nam
>  	     * the psymtab gets it wrong in some cases.
>  	     */
>  	    block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> -	    sym = lookup_block_symbol (block, name, STRUCT_NAMESPACE);
> +	    sym = lookup_block_symbol (block, name, NULL, STRUCT_NAMESPACE);
>  	    if (!sym)
>  	      error ("Internal: global symbol `%s' found in %s psymtab but not in symtab.\n\
>  %s may be an inlined function, or may be a template function\n\
> @@ -1128,7 +1132,7 @@ lookup_transparent_type (const char *nam
>    {
>      bv = BLOCKVECTOR (s);
>      block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> -    sym = lookup_block_symbol (block, name, STRUCT_NAMESPACE);
> +    sym = lookup_block_symbol (block, name, NULL, STRUCT_NAMESPACE);
>      if (sym && !TYPE_IS_OPAQUE (SYMBOL_TYPE (sym)))
>        {
>  	return SYMBOL_TYPE (sym);
> @@ -1142,7 +1146,7 @@ lookup_transparent_type (const char *nam
>  	s = PSYMTAB_TO_SYMTAB (ps);
>  	bv = BLOCKVECTOR (s);
>  	block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> -	sym = lookup_block_symbol (block, name, STRUCT_NAMESPACE);
> +	sym = lookup_block_symbol (block, name, NULL, STRUCT_NAMESPACE);
>  	if (!sym)
>  	  {
>  	    /* This shouldn't be necessary, but as a last resort
> @@ -1151,7 +1155,7 @@ lookup_transparent_type (const char *nam
>  	     * the psymtab gets it wrong in some cases.
>  	     */
>  	    block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> -	    sym = lookup_block_symbol (block, name, STRUCT_NAMESPACE);
> +	    sym = lookup_block_symbol (block, name, NULL, STRUCT_NAMESPACE);
>  	    if (!sym)
>  	      error ("Internal: static symbol `%s' found in %s psymtab but not in symtab.\n\
>  %s may be an inlined function, or may be a template function\n\
> @@ -1195,10 +1199,15 @@ find_main_psymtab (void)
>     binary search terminates, we drop through and do a straight linear
>     search on the symbols.  Each symbol which is marked as being a C++
>     symbol (language_cplus set) has both the encoded and non-encoded names
> -   tested for a match. */
> +   tested for a match.
> +
> +   If MANGLED_NAME is non-NULL, verify that any symbol we find has this
> +   particular mangled name.
> +*/
>  
>  struct symbol *
>  lookup_block_symbol (register const struct block *block, const char *name,
> +		     const char *mangled_name,
>  		     const namespace_enum namespace)
>  {
>    register int bot, top, inc;
> @@ -1258,14 +1267,19 @@ lookup_block_symbol (register const stru
>           return the first one; I believe it is now impossible for us
>           to encounter two symbols with the same name and namespace
>           here, because blocks containing argument symbols are no
> -         longer sorted.  */
> +         longer sorted.  The exception is for C++, where multiple functions
> +	 (cloned constructors / destructors, in particular) can have
> +	 the same demangled name.  So if we have a particular
> +	 mangled name to match, try to do so.  */
>  
>        top = BLOCK_NSYMS (block);
>        while (bot < top)
>  	{
>  	  sym = BLOCK_SYM (block, bot);
> -	  if (SYMBOL_NAMESPACE (sym) == namespace &&
> -	      SYMBOL_MATCHES_NAME (sym, name))
> +	  if (SYMBOL_NAMESPACE (sym) == namespace
> +	      && (mangled_name
> +		  ? strcmp (SYMBOL_NAME (sym), mangled_name) == 0
> +		  : SYMBOL_MATCHES_NAME (sym, name)))
>  	    {
>  	      return sym;
>  	    }
> @@ -1297,8 +1311,10 @@ lookup_block_symbol (register const stru
>        while (bot < top)
>  	{
>  	  sym = BLOCK_SYM (block, bot);
> -	  if (SYMBOL_NAMESPACE (sym) == namespace &&
> -	      SYMBOL_MATCHES_NAME (sym, name))
> +	  if (SYMBOL_NAMESPACE (sym) == namespace
> +	      && (mangled_name
> +		  ? strcmp (SYMBOL_NAME (sym), mangled_name) == 0
> +		  : SYMBOL_MATCHES_NAME (sym, name)))
>  	    {
>  	      /* If SYM has aliases, then use any alias that is active
>  	         at the current PC.  If no alias is active at the current
> Index: symtab.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/symtab.h,v
> retrieving revision 1.26
> diff -p -u -r1.26 symtab.h
> --- symtab.h	2001/12/21 22:32:37	1.26
> +++ symtab.h	2002/02/14 23:34:24
> @@ -1095,6 +1095,7 @@ extern struct symbol *lookup_symbol (con
>  /* lookup a symbol by name, within a specified block */
>  
>  extern struct symbol *lookup_block_symbol (const struct block *, const char *,
> +					   const char *,
>  					   const namespace_enum);
>  
>  /* lookup a [struct, union, enum] by name, within a specified block */
> Index: valops.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/valops.c,v
> retrieving revision 1.51
> diff -p -u -r1.51 valops.c
> --- valops.c	2002/02/10 05:50:34	1.51
> +++ valops.c	2002/02/14 23:34:25
> @@ -3249,7 +3249,7 @@ value_of_this (int complain)
>  
>    /* Calling lookup_block_symbol is necessary to get the LOC_REGISTER
>       symbol instead of the LOC_ARG one (if both exist).  */
> -  sym = lookup_block_symbol (b, funny_this, VAR_NAMESPACE);
> +  sym = lookup_block_symbol (b, funny_this, NULL, VAR_NAMESPACE);
>    if (sym == NULL)
>      {
>        if (complain)
> 

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] Select a particular mangling of a demangled symbol in lookup_block_symbol

[Elena Zannoni]

Daniel Jacobowitz writes:
 > Could someone please review this patch from last month?  Thanks.

Hey, I just did! I didn't even see this solicitation. Are you psychic?
:-)

Elena

Re: [RFA] Select a particular mangling of a demangled symbol in lookup_block_symbol

[Elena Zannoni]

Daniel Jacobowitz writes:
 > I just described the problem this patch addresses in my testsuite patch;
 > perhaps not the best place :)  Here's the relevant bit:
 > 
 >   - Multiple symbols with the same demangled name.  We can work around this
 >    for stabs, because we have the physname.  We don't have that option for   
 >    DWARF-2, and we shouldn't need to for stabs.  I have a patch for the
 >    workaround.  We get the [not-in-charge] constructor by default,
 >    unfortunately.
 > 
 > 
 > What this means is that we call lookup_block_symbol on something like:
 >   _ZN3fooC1ERS_
 > but get the information for:
 >   _ZN3fooC2ERS_
 > 
 > The breakpoint ends up on the base-not-in-charge constructor.
 > 
 > What we really SHOULD do is set it on both constructors silently, without
 > even acknowledging that they are different functions, or else offer the user
 > the choice.  My preference is actually for the former.  That requires
 > support for a single function existing in multiple places, which will also
 > give us nice things like better support for inlined functions with DWARF-2
 > (which will always be somewhat shoddy due to the nature of inlining, in that
 > it only occurs with lots of other optimization - but we can do much better
 > than we do).
 > 
 > Is this patch OK, or is it deemed too gross?
 > 
 > -- 
 > Daniel Jacobowitz                           Carnegie Mellon University
 > MontaVista Software                         Debian GNU/Linux Developer
 > 
 > 2002-02-14  Daniel Jacobowitz  <drow@mvista.com>
 > 
 > 	* symtab.h (lookup_block_symbol): Add mangled_name argument
 > 	to prototype.
 > 
 > 	* symmisc.c (maintenance_check_symtabs): Call lookup_block_symbol
 > 	with new mangled_name argument.
 > 	* linespec.c (decode_line_1): Likewise.
 > 	* valops (value_of_this): Likewise.
 > 	* symtab.c (lookup_transparent_type): Likewise.
 > 	(lookup_symbol_aux): Likewise.  Accept new mangled_name argument.
 > 	(lookup_symbol): If we are given a mangled name, pass it down
 > 	to lookup_symbol_aux.
 > 	(lookup_block_symbol): If we are given a mangled name to check
 > 	against, only return symbols which match it.
 > 
 > @@ -567,6 +568,7 @@ lookup_symbol (const char *name, const s
 >  {
 >    char *modified_name = NULL;
 >    char *modified_name2 = NULL;
 > +  const char *mangled_name = NULL;
 >    int needtofreename = 0;
 >    struct symbol *returnval;
 >  
 > @@ -592,13 +594,14 @@ lookup_symbol (const char *name, const s
 >        modified_name2 = cplus_demangle (modified_name, DMGL_ANSI | DMGL_PARAMS);
 >        if (modified_name2)
 >  	{
 > +	  mangled_name = name;
 >  	  modified_name = modified_name2;
 >  	  needtofreename = 1;
 >  	}
 >      }
 >  
 > -  returnval = lookup_symbol_aux (modified_name, block, namespace,
 > -				 is_a_field_of_this, symtab);
 > +  returnval = lookup_symbol_aux (modified_name, mangled_name, block,
 > +				 namespace, is_a_field_of_this, symtab);
 >    if (needtofreename)
 >      xfree (modified_name2);
 >  


OK, approved. But I have my usual couple of questions: 

Was the corresponding testsuite patch sorted out?  Looks like it
wasn't. Does this patch have any effect on the testsuite results w/o
the testsuite patch?

In the above, should it be mangled_name = name or mangled_name =
modified_name?  It would seem more uniform with the rest of the
function if we just used modified_name. Unless there is some problem
with case sensitivity, in which case, calling cplus_demangle with
modified_name seems wrong anyway. I.e. is it guaranteed that
case_sensitive_off is NOT in effect?  Just out of curiosity What
would happen if the user sets the case sensitivity off?
Wouldn't it change _ZN3fooC1ERS_ to _zn3fooc1ers_ ? (of course the user
can always do a lot of things to screw himself up)

I guess what I am really asking is when is lookup_symbol called with a
mangled name. I tried to do "break foo::foo", and I never saw it called
with a mangled name.

Elena

Re: [RFA] Select a particular mangling of a demangled symbol in lookup_block_symbol

[Elena Zannoni]

Elena Zannoni writes:
 > Daniel Jacobowitz writes:
 >  > I just described the problem this patch addresses in my testsuite patch;
 >  > perhaps not the best place :)  Here's the relevant bit:
 >  > 
 >  >   - Multiple symbols with the same demangled name.  We can work around this
 >  >    for stabs, because we have the physname.  We don't have that option for   
 >  >    DWARF-2, and we shouldn't need to for stabs.  I have a patch for the
 >  >    workaround.  We get the [not-in-charge] constructor by default,
 >  >    unfortunately.
 >  > 
 >  > 
 >  > What this means is that we call lookup_block_symbol on something like:
 >  >   _ZN3fooC1ERS_
 >  > but get the information for:
 >  >   _ZN3fooC2ERS_
 >  > 
 >  > The breakpoint ends up on the base-not-in-charge constructor.
 >  > 
 >  > What we really SHOULD do is set it on both constructors silently, without
 >  > even acknowledging that they are different functions, or else offer the user
 >  > the choice.  My preference is actually for the former.  That requires
 >  > support for a single function existing in multiple places, which will also
 >  > give us nice things like better support for inlined functions with DWARF-2
 >  > (which will always be somewhat shoddy due to the nature of inlining, in that
 >  > it only occurs with lots of other optimization - but we can do much better
 >  > than we do).
 >  > 
 >  > Is this patch OK, or is it deemed too gross?
 >  > 
 >  > -- 
 >  > Daniel Jacobowitz                           Carnegie Mellon University
 >  > MontaVista Software                         Debian GNU/Linux Developer
 >  > 
 >  > 2002-02-14  Daniel Jacobowitz  <drow@mvista.com>
 >  > 
 >  > 	* symtab.h (lookup_block_symbol): Add mangled_name argument
 >  > 	to prototype.
 >  > 
 >  > 	* symmisc.c (maintenance_check_symtabs): Call lookup_block_symbol
 >  > 	with new mangled_name argument.
 >  > 	* linespec.c (decode_line_1): Likewise.
 >  > 	* valops (value_of_this): Likewise.
 >  > 	* symtab.c (lookup_transparent_type): Likewise.
 >  > 	(lookup_symbol_aux): Likewise.  Accept new mangled_name argument.
 >  > 	(lookup_symbol): If we are given a mangled name, pass it down
 >  > 	to lookup_symbol_aux.
 >  > 	(lookup_block_symbol): If we are given a mangled name to check
 >  > 	against, only return symbols which match it.
 >  > 
 >  > @@ -567,6 +568,7 @@ lookup_symbol (const char *name, const s
 >  >  {
 >  >    char *modified_name = NULL;
 >  >    char *modified_name2 = NULL;
 >  > +  const char *mangled_name = NULL;
 >  >    int needtofreename = 0;
 >  >    struct symbol *returnval;
 >  >  
 >  > @@ -592,13 +594,14 @@ lookup_symbol (const char *name, const s
 >  >        modified_name2 = cplus_demangle (modified_name, DMGL_ANSI | DMGL_PARAMS);
 >  >        if (modified_name2)
 >  >  	{
 >  > +	  mangled_name = name;
 >  >  	  modified_name = modified_name2;
 >  >  	  needtofreename = 1;
 >  >  	}
 >  >      }
 >  >  
 >  > -  returnval = lookup_symbol_aux (modified_name, block, namespace,
 >  > -				 is_a_field_of_this, symtab);
 >  > +  returnval = lookup_symbol_aux (modified_name, mangled_name, block,
 >  > +				 namespace, is_a_field_of_this, symtab);
 >  >    if (needtofreename)
 >  >      xfree (modified_name2);
 >  >  
 > 
 > 
 > OK, approved. But I have my usual couple of questions: 
 > 
 > Was the corresponding testsuite patch sorted out?  Looks like it
 > wasn't. Does this patch have any effect on the testsuite results w/o
 > the testsuite patch?
 > 
 > In the above, should it be mangled_name = name or mangled_name =
 > modified_name?  It would seem more uniform with the rest of the
 > function if we just used modified_name. Unless there is some problem
 > with case sensitivity, in which case, calling cplus_demangle with
 > modified_name seems wrong anyway. I.e. is it guaranteed that
 > case_sensitive_off is NOT in effect?  Just out of curiosity What
 > would happen if the user sets the case sensitivity off?
 > Wouldn't it change _ZN3fooC1ERS_ to _zn3fooc1ers_ ? (of course the user
 > can always do a lot of things to screw himself up)
 > 
 > I guess what I am really asking is when is lookup_symbol called with a
 > mangled name. I tried to do "break foo::foo", and I never saw it called
 > with a mangled name.
 > 
 > Elena
 > 
 > 

Replying to myself. I was testing with dwarf2, but using stabs I can
see it.  I can also see that we call cplus_demangle (__3foor3foo, ...)
instead of cplus_demangle (__3fooR3foo, ...), if 'set case off'. But
it returns foo::foo(long double, foo) anyway. So it works?

Elena

Re: [RFA] Select a particular mangling of a demangled symbol in lookup_block_symbol

[Daniel Jacobowitz]

On Tue, Mar 19, 2002 at 05:46:58PM -0500, Elena Zannoni wrote:
> Replying to myself. I was testing with dwarf2, but using stabs I can
> see it.  I can also see that we call cplus_demangle (__3foor3foo, ...)
> instead of cplus_demangle (__3fooR3foo, ...), if 'set case off'. But
> it returns foo::foo(long double, foo) anyway. So it works?

Absolutely not:

drow@nevyn:~% c++filt
__3fooR3foo
foo::foo(foo &)
__3foor3foo
foo::foo(long double, foo)

Those aren't the same function at all :)

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] Select a particular mangling of a demangled symbol in lookup_block_symbol

[Elena Zannoni]

Daniel Jacobowitz writes:
 > On Tue, Mar 19, 2002 at 05:46:58PM -0500, Elena Zannoni wrote:
 > > Replying to myself. I was testing with dwarf2, but using stabs I can
 > > see it.  I can also see that we call cplus_demangle (__3foor3foo, ...)
 > > instead of cplus_demangle (__3fooR3foo, ...), if 'set case off'. But
 > > it returns foo::foo(long double, foo) anyway. So it works?
 > 
 > Absolutely not:
 > 
 > drow@nevyn:~% c++filt
 > __3fooR3foo
 > foo::foo(foo &)
 > __3foor3foo
 > foo::foo(long double, foo)
 > 

Ah, I didn't think twice when I saw it not core dumping!

 > Those aren't the same function at all :)
 > 

Indeed. Then I think we definitely should use the uppercase parameter.

Elena


 > -- 
 > Daniel Jacobowitz                           Carnegie Mellon University
 > MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] Select a particular mangling of a demangled symbol in lookup_block_symbol

[Daniel Jacobowitz]

On Fri, Mar 22, 2002 at 03:11:01PM -0500, Elena Zannoni wrote:
> Daniel Jacobowitz writes:
>  > On Tue, Mar 19, 2002 at 05:46:58PM -0500, Elena Zannoni wrote:
>  > > Replying to myself. I was testing with dwarf2, but using stabs I can
>  > > see it.  I can also see that we call cplus_demangle (__3foor3foo, ...)
>  > > instead of cplus_demangle (__3fooR3foo, ...), if 'set case off'. But
>  > > it returns foo::foo(long double, foo) anyway. So it works?
>  > 
>  > Absolutely not:
>  > 
>  > drow@nevyn:~% c++filt
>  > __3fooR3foo
>  > foo::foo(foo &)
>  > __3foor3foo
>  > foo::foo(long double, foo)
>  > 
> 
> Ah, I didn't think twice when I saw it not core dumping!
> 
>  > Those aren't the same function at all :)
>  > 
> 
> Indeed. Then I think we definitely should use the uppercase parameter.

Is this what you had in mind?

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

2002-03-22  Daniel Jacobowitz  <drow@mvista.com>

	* symtab.c (lookup_symbol): Demangled before lowercasing.

Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.58
diff -u -p -r1.58 symtab.c
--- symtab.c	2002/03/22 18:57:08	1.58
+++ symtab.c	2002/03/22 22:22:23
@@ -569,12 +569,27 @@ lookup_symbol (const char *name, const s
 	       const namespace_enum namespace, int *is_a_field_of_this,
 	       struct symtab **symtab)
 {
-  char *modified_name = NULL;
-  char *modified_name2 = NULL;
+  char *demangled_name = NULL;
+  const char *modified_name = NULL;
   const char *mangled_name = NULL;
   int needtofreename = 0;
   struct symbol *returnval;
 
+  modified_name = name;
+
+  /* If we are using C++ language, demangle the name before doing a lookup, so
+     we can always binary search. */
+  if (current_language->la_language == language_cplus)
+    {
+      demangled_name = cplus_demangle (name, DMGL_ANSI | DMGL_PARAMS);
+      if (demangled_name)
+	{
+	  mangled_name = name;
+	  modified_name = demangled_name;
+	  needtofreename = 1;
+	}
+    }
+
   if (case_sensitivity == case_sensitive_off)
     {
       char *copy;
@@ -587,26 +602,11 @@ lookup_symbol (const char *name, const s
       copy[len] = 0;
       modified_name = copy;
     }
-  else 
-      modified_name = (char *) name;
-
-  /* If we are using C++ language, demangle the name before doing a lookup, so
-     we can always binary search. */
-  if (current_language->la_language == language_cplus)
-    {
-      modified_name2 = cplus_demangle (modified_name, DMGL_ANSI | DMGL_PARAMS);
-      if (modified_name2)
-	{
-	  mangled_name = name;
-	  modified_name = modified_name2;
-	  needtofreename = 1;
-	}
-    }
 
   returnval = lookup_symbol_aux (modified_name, mangled_name, block,
 				 namespace, is_a_field_of_this, symtab);
   if (needtofreename)
-    xfree (modified_name2);
+    xfree (demangled_name);
 
   return returnval;	 
 }

Re: [RFA] Select a particular mangling of a demangled symbol in lookup_block_symbol

[Daniel Jacobowitz]

On Tue, Mar 19, 2002 at 05:19:41PM -0500, Elena Zannoni wrote:
> OK, approved. But I have my usual couple of questions: 

Committed to trunk only; I'll move it to the branch in a week or so if
it doesn't break anything.

> Was the corresponding testsuite patch sorted out?  Looks like it
> wasn't. Does this patch have any effect on the testsuite results w/o

Nope.

> the testsuite patch?

I don't believe so.

> In the above, should it be mangled_name = name or mangled_name =
> modified_name?  It would seem more uniform with the rest of the
> function if we just used modified_name. Unless there is some problem
> with case sensitivity, in which case, calling cplus_demangle with
> modified_name seems wrong anyway. I.e. is it guaranteed that
> case_sensitive_off is NOT in effect?  Just out of curiosity What
> would happen if the user sets the case sensitivity off?
> Wouldn't it change _ZN3fooC1ERS_ to _zn3fooc1ers_ ? (of course the user
> can always do a lot of things to screw himself up)

As far as I'm concerned - if the user sets case sensitivity off while
debugging C++, they deserve what they get.  Mangling is not
case-insensitive.  In fact, in v3, cplus_demangle is absolutely
guaranteed to fail (_z is not a legal prefix for a mangled name). 
Perhaps it would be better to call cplus_demangle with the original
name.

> I guess what I am really asking is when is lookup_symbol called with a
> mangled name. I tried to do "break foo::foo", and I never saw it called
> with a mangled name.

find_methods will do it, in two places, at least if you're using stabs:
                sym_arr[i1] = lookup_symbol (phys_name,
                                             NULL, VAR_NAMESPACE,
                                             (int *) NULL,
                                             (struct symtab **) NULL);

It shouldn't do this, of course, but that requires a lot of work that I
haven't gotten to yet.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] Select a particular mangling of a demangled symbol in lookup_block_symbol

[Elena Zannoni]

Daniel Jacobowitz writes:
 > On Tue, Mar 19, 2002 at 05:19:41PM -0500, Elena Zannoni wrote:
 > > OK, approved. But I have my usual couple of questions: 
 > 
 > Committed to trunk only; I'll move it to the branch in a week or so if
 > it doesn't break anything.
 > 
 > > Was the corresponding testsuite patch sorted out?  Looks like it
 > > wasn't. Does this patch have any effect on the testsuite results w/o
 > 
 > Nope.
 > 
 > > the testsuite patch?
 > 
 > I don't believe so.
 > 
 > > In the above, should it be mangled_name = name or mangled_name =
 > > modified_name?  It would seem more uniform with the rest of the
 > > function if we just used modified_name. Unless there is some problem
 > > with case sensitivity, in which case, calling cplus_demangle with
 > > modified_name seems wrong anyway. I.e. is it guaranteed that
 > > case_sensitive_off is NOT in effect?  Just out of curiosity What
 > > would happen if the user sets the case sensitivity off?
 > > Wouldn't it change _ZN3fooC1ERS_ to _zn3fooc1ers_ ? (of course the user
 > > can always do a lot of things to screw himself up)
 > 
 > As far as I'm concerned - if the user sets case sensitivity off while
 > debugging C++, they deserve what they get.  Mangling is not
 > case-insensitive.  In fact, in v3, cplus_demangle is absolutely
 > guaranteed to fail (_z is not a legal prefix for a mangled name). 
 > Perhaps it would be better to call cplus_demangle with the original
 > name.

I think that just to be safe we should do that, yes.

 > 
 > > I guess what I am really asking is when is lookup_symbol called with a
 > > mangled name. I tried to do "break foo::foo", and I never saw it called
 > > with a mangled name.
 > 
 > find_methods will do it, in two places, at least if you're using stabs:
 >                 sym_arr[i1] = lookup_symbol (phys_name,
 >                                              NULL, VAR_NAMESPACE,
 >                                              (int *) NULL,
 >                                              (struct symtab **) NULL);
 > 

Yes, I notice a difference stabs vs. dwarf2.

 > It shouldn't do this, of course, but that requires a lot of work that I
 > haven't gotten to yet.
 > 

Elena


 > -- 
 > Daniel Jacobowitz                           Carnegie Mellon University
 > MontaVista Software                         Debian GNU/Linux Developer