Re: new demangler

Re: new demangler

[Michael Elizabeth Chastain]

Works for me with c++filt.

  [mec.gnu@berman migdmt]$ /berman/migchain/install/target/native/binutils-2.14/bin/c++filt _ZN1C6CClassaSERKS0_
  C::CClass::operator=(C::CClass const&)

  [mec.gnu@berman migdmt]$ echo _ZN1C6CClassaSERKS0_ | /berman/migchain/install/target/native/binutils-2.14/bin/c++filt
  C::CClass::operator=(C::CClass const&)

  [mec.gnu@berman migdmt]$ /berman/fsf/_current_/berman/install/target/native/binutils/HEAD/bin/c++filt _ZN1C6CClassaSERKS0_
  C::CClass::operator=(C::CClass const&)

  [mec.gnu@berman migdmt]$ echo _ZN1C6CClassaSERKS0_ | /berman/fsf/_current_/berman/install/target/native/binutils/HEAD/bin/c++filt
  C::CClass::operator=(C::CClass const&)

One problem with talking about "the demangler" is that the options
parameter is different when the demangler is invoked in different ways.
c++filt uses different options for command line versus standard input,
and gdb specifies different options in different places.

Can you breakpoint cp_demangle inside gdb and see what's going on?

Michael C

Re: new demangler

[Michael Elizabeth Chastain]

Jim Blandy writes:
> When working on big C++ programs, I think I remember seeing that the
> mangled and demangled names take up a *lot* of memory.

I analyzed this recently.

  http://sources.redhat.com/ml/gdb/2003-12/msg00071.html
  http://sources.redhat.com/ml/gdb/2003-12/msg00092.html

One specific thinko caused a lot of trouble for monotone, which is built
with the boost libraries.  But mozilla does not have any symbols which
were affected by this bug, so memory size on mozilla probably didn't get
any better with this bug fix.

The new demangler has the same memory requirement has old demangler +
bug fix (46 megabytes on monotone).  But the new demangler runs a lot
faster.  Check out the difference in elapsed time between "H21" and
"H30" in my table.  gdb with old demangler + bug fix starts up in 9.46
seconds, and gdb with new demangler starts up in 3.03 seconds.

> If that recollection is correct, it might be cool to have GDB store
> the trees instead of the names, and throw a bcache at the nodes in
> these demangled name trees, working from the leaves upward, to get
> maximum sharing.  I bet that would bring the memory requirements down
> to something more closely resembling the actual number of classes,
> methods, etc. in the program.

How about quantifying gdb performance (space and time) on a real
program first.  There might not be a problem at all.  Then look for some
low-hanging fruit before pulling out the algorithmic guns.

Michael C

Re: new demangler

[Michael Elizabeth Chastain]

dc> Thanks!  Now, when I demangle that name with just DMGL_ANSI, I get
dc> "C::CClass::operator=".  Am I going crazy, or was there once '()'
dc> stuck on the end of that?

There used to be '()' stuck on the end of the output with DMGL_ANSI,
maybe with parameters between them, maybe not (I dunno).

Here are relevant PR's:

  http://sources.redhat.com/gdb/bugs/1471
  http://sources.redhat.com/gdb/bugs/34

Michael C

new demangler

[David Carlton]

I just tried to merge my GDB branch with the current sources, and I'm
running into problems that seem to be caused by the new demangler.
Specifically, it doesn't seem to like "_ZN1C6CClassaSERKS0_", which
the old demangler claims is "C::CClass::operator=(C::CClass const&)".

David Carlton
carlton@kealia.com

Re: new demangler

[Daniel Jacobowitz]

On Mon, Dec 15, 2003 at 02:53:11PM -0800, David Carlton wrote:
> I just tried to merge my GDB branch with the current sources, and I'm
> running into problems that seem to be caused by the new demangler.
> Specifically, it doesn't seem to like "_ZN1C6CClassaSERKS0_", which
> the old demangler claims is "C::CClass::operator=(C::CClass const&)".

It does for me:
drow@nevyn:~% /opt/src/gdb/x86-as/binutils/cxxfilt _ZN1C6CClassaSERKS0_
C::CClass::operator=(C::CClass const&)

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: new demangler

[David Carlton]

On Mon, 15 Dec 2003 17:54:52 -0500, Daniel Jacobowitz <drow@mvista.com> said:
> On Mon, Dec 15, 2003 at 02:53:11PM -0800, David Carlton wrote:

>> I just tried to merge my GDB branch with the current sources, and
>> I'm running into problems that seem to be caused by the new
>> demangler.  Specifically, it doesn't seem to like
>> "_ZN1C6CClassaSERKS0_", which the old demangler claims is
>> "C::CClass::operator=(C::CClass const&)".

> It does for me:
> drow@nevyn:~% /opt/src/gdb/x86-as/binutils/cxxfilt _ZN1C6CClassaSERKS0_
> C::CClass::operator=(C::CClass const&)

Hmm.  I don't have a CVS binutils tree around, but I get, with GDB
from current CVS:

(gdb) maint demangle _ZN1C6CClassaSERKS0_
Can't demangle "_ZN1C6CClassaSERKS0_"

So either we're using different demanglers or else there's something
strange about maint demangle/class_name_from_physname.  Either of
which is possible - nobody other than me uses class_name_from_physname
regularly, after all, and maint demangle is only a maint command.

David Carlton
carlton@kealia.com

Re: new demangler

[Ian Lance Taylor]

David Carlton <carlton@kealia.com> writes:

> Hmm.  I don't have a CVS binutils tree around, but I get, with GDB
> from current CVS:
> 
> (gdb) maint demangle _ZN1C6CClassaSERKS0_
> Can't demangle "_ZN1C6CClassaSERKS0_"
> 
> So either we're using different demanglers or else there's something
> strange about maint demangle/class_name_from_physname.  Either of
> which is possible - nobody other than me uses class_name_from_physname
> regularly, after all, and maint demangle is only a maint command.

`maint demangle' only works for me when I explicitly run gdb on a C++
program.  When I do specify a C++ program, it appears to work
correctly.

Otherwise, I see that maintenance_demangle() calls
language_demangle().  When I try running gdb without an explicit C++
program, either current_language is NULL or
current_language->la_demangle is NULL, so language_demangle returns
NULL.

Ian

Re: new demangler

[David Carlton]

On 15 Dec 2003 18:10:36 -0500, Ian Lance Taylor <ian@wasabisystems.com> said:

> `maint demangle' only works for me when I explicitly run gdb on a
> C++ program.  When I do specify a C++ program, it appears to work
> correctly.

Ah, I see.  Now I'm getting it to work correctly with 'maint
demangle', too.

So is there a problem with the way class_name_from_physname invokes
the demangler?  It does this:

  char *demangled_name = cplus_demangle (physname, DMGL_ANSI);

Compared to other places in GDB where the demangler is called, this is
unusual since DMGL_PARAMS isn't passed in as well, but I don't see why
that would cause the demangling to fail completely.

David Carlton
carlton@kealia.com

Re: new demangler

[Ian Lance Taylor]

David Carlton <carlton@kealia.com> writes:

> So is there a problem with the way class_name_from_physname invokes
> the demangler?  It does this:
> 
>   char *demangled_name = cplus_demangle (physname, DMGL_ANSI);
> 
> Compared to other places in GDB where the demangler is called, this is
> unusual since DMGL_PARAMS isn't passed in as well, but I don't see why
> that would cause the demangling to fail completely.

Well, in fact, there was a bug in the case where DMGL_PARAMS was not
set.  I just checked in a patch for it.

Sorry about that.  I've had the patch for a while, but I haven't had a
chance to check it in.

Ian

Re: new demangler

[Ian Lance Taylor]

Ian Lance Taylor <ian@wasabisystems.com> writes:

> David Carlton <carlton@kealia.com> writes:
> 
> > So is there a problem with the way class_name_from_physname invokes
> > the demangler?  It does this:
> > 
> >   char *demangled_name = cplus_demangle (physname, DMGL_ANSI);
> > 
> > Compared to other places in GDB where the demangler is called, this is
> > unusual since DMGL_PARAMS isn't passed in as well, but I don't see why
> > that would cause the demangling to fail completely.
> 
> Well, in fact, there was a bug in the case where DMGL_PARAMS was not
> set.  I just checked in a patch for it.
> 
> Sorry about that.  I've had the patch for a while, but I haven't had a
> chance to check it in.

BTW, when I say that I just checked in the patch, I mean that I just
checked it in to the master sources in the gcc repository.

Here's the patch.

Ian

Index: cp-demangle.c
===================================================================
RCS file: /cvs/gcc/gcc/libiberty/cp-demangle.c,v
retrieving revision 1.58
diff -u -p -r1.58 cp-demangle.c
--- cp-demangle.c	15 Dec 2003 14:37:25 -0000	1.58
+++ cp-demangle.c	15 Dec 2003 23:14:02 -0000
@@ -3622,9 +3622,11 @@ d_demangle (mangled, options, palc)
   else
     dc = d_type (&di);
 
-  /* If we didn't consume the entire mangled string, then we didn't
-     successfully demangle it.  */
-  if (d_peek_char (&di) != '\0')
+  /* If DMGL_PARAMS is set, then if we didn't consume the entire
+     mangled string, then we didn't successfully demangle it.  If
+     DMGL_PARAMS is not set, we didn't look at the trailing
+     parameters.  */
+  if (((options & DMGL_PARAMS) != 0) && d_peek_char (&di) != '\0')
     dc = NULL;
 
 #ifdef CP_DEMANGLE_DEBUG
@@ -3829,37 +3831,37 @@ is_ctor_or_dtor (mangled, ctor_kind, dto
 
   dc = d_mangled_name (&di, 1);
 
+  /* Note that because we did not pass DMGL_PARAMS, we don't expect to
+     demangle the entire string.  */
+
   ret = 0;
-  if (d_peek_char (&di) == '\0')
+  while (dc != NULL)
     {
-      while (dc != NULL)
+      switch (dc->type)
 	{
-	  switch (dc->type)
-	    {
-	    default:
-	      dc = NULL;
-	      break;
-	    case D_COMP_TYPED_NAME:
-	    case D_COMP_TEMPLATE:
-	    case D_COMP_RESTRICT_THIS:
-	    case D_COMP_VOLATILE_THIS:
-	    case D_COMP_CONST_THIS:
-	      dc = d_left (dc);
-	      break;
-	    case D_COMP_QUAL_NAME:
-	      dc = d_right (dc);
-	      break;
-	    case D_COMP_CTOR:
-	      *ctor_kind = dc->u.s_ctor.kind;
-	      ret = 1;
-	      dc = NULL;
-	      break;
-	    case D_COMP_DTOR:
-	      *dtor_kind = dc->u.s_dtor.kind;
-	      ret = 1;
-	      dc = NULL;
-	      break;
-	    }
+	default:
+	  dc = NULL;
+	  break;
+	case D_COMP_TYPED_NAME:
+	case D_COMP_TEMPLATE:
+	case D_COMP_RESTRICT_THIS:
+	case D_COMP_VOLATILE_THIS:
+	case D_COMP_CONST_THIS:
+	  dc = d_left (dc);
+	  break;
+	case D_COMP_QUAL_NAME:
+	  dc = d_right (dc);
+	  break;
+	case D_COMP_CTOR:
+	  *ctor_kind = dc->u.s_ctor.kind;
+	  ret = 1;
+	  dc = NULL;
+	  break;
+	case D_COMP_DTOR:
+	  *dtor_kind = dc->u.s_dtor.kind;
+	  ret = 1;
+	  dc = NULL;
+	  break;
 	}
     }
 

Re: new demangler

[David Carlton]

On 15 Dec 2003 18:23:22 -0500, Ian Lance Taylor <ian@wasabisystems.com> said:

> Here's the patch.

Thanks!  Now, when I demangle that name with just DMGL_ANSI, I get
"C::CClass::operator=".  Am I going crazy, or was there once '()'
stuck on the end of that?

David Carlton
carlton@kealia.com

Re: new demangler

[Ian Lance Taylor]

David Carlton <carlton@kealia.com> writes:

> On 15 Dec 2003 18:23:22 -0500, Ian Lance Taylor <ian@wasabisystems.com> said:
> 
> > Here's the patch.
> 
> Thanks!  Now, when I demangle that name with just DMGL_ANSI, I get
> "C::CClass::operator=".  Am I going crazy, or was there once '()'
> stuck on the end of that?

Hmmm, I doubt there was ever '()' at the end.  I would expect to see
`(C::CClass const&)' at the end.

Note that the old V3 demangler ignored DMGL_PARAMS--it effectively
assumed that DMGL_PARAMS was always passed, so your code used to
always get the parameter types.  Now you have a choice.  It is
slightly more efficient to demangle strings without passing
DMGL_PARAMS, but of course then you don't get any information on
parameter types.

I don't know which you actually want.  If you want the parameter
types, you need to pass DMGL_PARAMS.

Ian

Re: new demangler

[David Carlton]

On 15 Dec 2003 18:33:39 -0500, Ian Lance Taylor <ian@wasabisystems.com> said:
> David Carlton <carlton@kealia.com> writes:
>> On 15 Dec 2003 18:23:22 -0500, Ian Lance Taylor <ian@wasabisystems.com> said:

>>> Here's the patch.

>> Thanks!  Now, when I demangle that name with just DMGL_ANSI, I get
>> "C::CClass::operator=".  Am I going crazy, or was there once '()'
>> stuck on the end of that?

> Hmmm, I doubt there was ever '()' at the end.  I would expect to see
> `(C::CClass const&)' at the end.

Okay; I'll just stick in the DMGL_PARAMS for now, then - my tests
start passing again with that change.  (Or I could rewrite the code
that parses the output to not look for parentheses, I suppose.)

David Carlton
carlton@kealia.com

Re: new demangler

[Daniel Jacobowitz]

On Mon, Dec 15, 2003 at 03:38:28PM -0800, David Carlton wrote:
> On 15 Dec 2003 18:33:39 -0500, Ian Lance Taylor <ian@wasabisystems.com> said:
> > David Carlton <carlton@kealia.com> writes:
> >> On 15 Dec 2003 18:23:22 -0500, Ian Lance Taylor <ian@wasabisystems.com> said:
> 
> >>> Here's the patch.
> 
> >> Thanks!  Now, when I demangle that name with just DMGL_ANSI, I get
> >> "C::CClass::operator=".  Am I going crazy, or was there once '()'
> >> stuck on the end of that?
> 
> > Hmmm, I doubt there was ever '()' at the end.  I would expect to see
> > `(C::CClass const&)' at the end.
> 
> Okay; I'll just stick in the DMGL_PARAMS for now, then - my tests
> start passing again with that change.  (Or I could rewrite the code
> that parses the output to not look for parentheses, I suppose.)

Do the former for now.

When I can figure out a few more points of C++ grammar - it's taken me
a *(!&#@ month so far and it's barely functional... - I'll have an
efficient solution for breaking down names into the demangler's trees. 
This will let us get the information we want much more reliably.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: new demangler

[Jim Blandy]

Daniel Jacobowitz <drow@mvista.com> writes:
> When I can figure out a few more points of C++ grammar - it's taken me
> a *(!&#@ month so far and it's barely functional... - I'll have an
> efficient solution for breaking down names into the demangler's trees. 
> This will let us get the information we want much more reliably.

When working on big C++ programs, I think I remember seeing that the
mangled and demangled names take up a *lot* of memory.

If that recollection is correct, it might be cool to have GDB store
the trees instead of the names, and throw a bcache at the nodes in
these demangled name trees, working from the leaves upward, to get
maximum sharing.  I bet that would bring the memory requirements down
to something more closely resembling the actual number of classes,
methods, etc. in the program.

I think the trees would be a lot easier to do certain kinds of
matching on, too.  "Is this a user-defined cast operator?"

Re: new demangler

[Ian Lance Taylor]

Jim Blandy <jimb@redhat.com> writes:

> Daniel Jacobowitz <drow@mvista.com> writes:
> > When I can figure out a few more points of C++ grammar - it's taken me
> > a *(!&#@ month so far and it's barely functional... - I'll have an
> > efficient solution for breaking down names into the demangler's trees. 
> > This will let us get the information we want much more reliably.
> 
> When working on big C++ programs, I think I remember seeing that the
> mangled and demangled names take up a *lot* of memory.
> 
> If that recollection is correct, it might be cool to have GDB store
> the trees instead of the names, and throw a bcache at the nodes in
> these demangled name trees, working from the leaves upward, to get
> maximum sharing.  I bet that would bring the memory requirements down
> to something more closely resembling the actual number of classes,
> methods, etc. in the program.
> 
> I think the trees would be a lot easier to do certain kinds of
> matching on, too.  "Is this a user-defined cast operator?"

Tangentially, note that the trees that I wrote for the demangler are
based pretty directly on the mangling ABI.  They just follow the
grammar productions used there.

To use them for anything other than demangling, it might be useful to
try to work out a good tree representation for C++ names.  Then I or
somebody can modify the demangler to use that tree representation
instead.  It would probably be pretty close to the trees I use, but I
doubt it would be identical.

Ian

Re: new demangler

[Daniel Jacobowitz]

On Wed, Dec 17, 2003 at 09:58:29PM -0500, Ian Lance Taylor wrote:
> Jim Blandy <jimb@redhat.com> writes:
> 
> > Daniel Jacobowitz <drow@mvista.com> writes:
> > > When I can figure out a few more points of C++ grammar - it's taken me
> > > a *(!&#@ month so far and it's barely functional... - I'll have an
> > > efficient solution for breaking down names into the demangler's trees. 
> > > This will let us get the information we want much more reliably.
> > 
> > When working on big C++ programs, I think I remember seeing that the
> > mangled and demangled names take up a *lot* of memory.
> > 
> > If that recollection is correct, it might be cool to have GDB store
> > the trees instead of the names, and throw a bcache at the nodes in
> > these demangled name trees, working from the leaves upward, to get
> > maximum sharing.  I bet that would bring the memory requirements down
> > to something more closely resembling the actual number of classes,
> > methods, etc. in the program.

I don't know how well it would work compared to simply cutting down on
the number of demangled names we need in memory.  Specifically, instead
of one symbol table with fully qualified names, lots of little
dictionaries with the names of members.

> > I think the trees would be a lot easier to do certain kinds of
> > matching on, too.  "Is this a user-defined cast operator?"
> 
> Tangentially, note that the trees that I wrote for the demangler are
> based pretty directly on the mangling ABI.  They just follow the
> grammar productions used there.
> 
> To use them for anything other than demangling, it might be useful to
> try to work out a good tree representation for C++ names.  Then I or
> somebody can modify the demangler to use that tree representation
> instead.  It would probably be pretty close to the trees I use, but I
> doubt it would be identical.

So far, simply using yours hasn't been a problem.  A few of my rules
are a little awkward, but compared to the hacks normally found in yacc
grammars it's a downright beauty.

Was anything ever decided about conversion operators and typeof()?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: new demangler

[Ian Lance Taylor]

Daniel Jacobowitz <drow@mvista.com> writes:

> Was anything ever decided about conversion operators and typeof()?

I think there was a general leaning toward using __typeof__ when
demangling complicated conversion operators.  But there wasn't any
final decision.

Ian