The future of dwarf2_physname

The future of dwarf2_physname

[Keith Seitz]

Hi,

Several of us Red Hat folk have been discussing this internally, and 
it's time to take this discussion public.

Jan's recent patch proposal in response to my typedef'd method 
parameters patchset lead to some further investigation, and I believe I 
(we?) are all coming to the conclusion that Jan's patch is probably the 
best way to proceed for the future, i.e., post 7.3 release.

[Jan's proposal is here: 
http://sourceware.org/ml/gdb-patches/2011-05/msg00358.html . NOTE: 
DMGL_VERBOSE should be added to the demangler options in that patch if 
you want to play with it.]

In short, if gdb uses DW_AT[_MIPS]_linkage_name when available, we can 
bypass constructing the physname *and* canonicalization, so gdb would 
only have to do this when DW_AT[_MIPS]_linkage_name is missing. Today 
this only happens AFAIK with gcc and ctors. I don't know about other 
compilers.

This could be a sizable performance win (not yet tested or quantified). 
While it does introduce some regressions, many are compiler problems and 
some are template problems (the linkage name contains the return type, 
and 1) gdb isn't equipped to deal with that; 2) it makes for a horrible 
user experience having to specify it all the time). There might be 
others, I have only delved into this a bit myself today.

So the first question to be asked here is, Should we adopt Jan's patch, 
or some variation of it, to use DW_AT_linkage_name when available?

The only hesitation I have is the compiler issues. Just using 
DW_AT_linkage_name instead of computing it has lead to many test 
regressions with differing compiler versions. [Again, I have only tested 
gcc.]

It appears that computing the physname offers gdb some buffer from 
compiler changes/bugs. At least with gcc. [in case I haven't stressed 
that enough]

What do others think?

------

The bigger issue is what to do for 7.3, which has been in limbo for 
quite some time awaiting resolution of c++/12506 (for which I have 
posted patches).

I guess there are basically two options:
1) Keep physname & apply the 12266/12506 patchset (when approved)
2) Adopt Jan's patch and fix the fallout now

My take on this today is that I think the least risky/most timely 
solution is #1. Gdb has been used with the dwarf2_physname patchset for 
over a year now, and it is not altogether broken. We are seeing some 
regressions from previous releases (none of these are in the test 
suite), and I have been trying to keep on top of them.

The 12266/12506 patchset is almost orthogonal to this decision. It fixes 
several additional typedef-related problems. 12506 is fixed by using 
DW_AT_linkage_name, but 12266 is not.

My big fear with #2 is "fixing" linespecs (yet again). Any changes to 
this area take an unimaginably large amount of time and effort.

What do others think?

Keith

Re: The future of dwarf2_physname

[Jan Kratochvil]

Hi Keith,

On Thu, 19 May 2011 00:34:43 +0200, Keith Seitz wrote:
[...]
> It appears that computing the physname offers gdb some buffer from
> compiler changes/bugs. At least with gcc.

I find any difference from compiler's idea of a (demangled) linkage name to be
a bug.  When nm, objdump, other tools deal with some C++ name I believe GDB
also should.

If the linkage name is wrong for whatever reason this is an ABI issue for GCC,
it is out of scope of GDB.  Linkage name is defined by the ISO C++ standard,
neither GCC nor GDB can change it.  If GCC is not ISO C++ compliant then GCC
should be fixed (and possibly deal with the ABI breakage) but such ISO C++
compliance issue is out of scope of GDB.

Trying to introduce new "better" naming for C++ methods which is in 90% the
same as GCC's and ISO C++'s idea seems a bit messy for me.

That is the GCC and ISO C++ naming should be always available in GDB.  I am
not against allowing also other aliases for the same symbol.  The availability
of symbol aliases is discussed more at the end of this mail.


As parts of code have DWARF (=full symbols) and part only ELF .symtab
(=minimal symbols) the physname code (creating linkage names based on DWARF
DIEs) just cannot apply to ELF symbols.  Thefore if it creates different
physname from DWARF than what can be figured out from the library.

#include <sstream>
std::stringstream v;
int
main ()
{
  v.unsetf (std::ios_base::hex);
}
x86_64-fedora15-linux-gnu with no /usr/lib/debug

GDB with
	Re: [RFA] Typedef'd method parameters [0/4]
	http://sourceware.org/ml/gdb-patches/2011-05/msg00318.html
without my patch
	[rfc] physname cross-check [Re: [RFA] Typedef'd method parameters [0/4]]
	http://sourceware.org/ml/gdb-patches/2011-05/msg00358.html
(gdb) complete p 'std::ios_base::unsetf
p 'std::ios_base::unsetf(std::_Ios_Fmtflags)
p 'std::ios_base::unsetf(std::ios_base::fmtflags)
(gdb) p 'std::ios_base::unsetf(std::_Ios_Fmtflags)'
$2 = {<text variable, no debug info>} 0x400768 <std::ios_base::unsetf(std::ios_base::fmtflags)>
(gdb) ptype 'std::ios_base::unsetf(std::_Ios_Fmtflags)'
type = int (void)
(gdb) p 'std::ios_base::unsetf(std::ios_base::fmtflags)'
No symbol "std::ios_base::unsetf(std::ios_base::fmtflags)" in current context.
(gdb) ptype 'std::ios_base::unsetf(std::ios_base::fmtflags)'
No symbol "std::ios_base::unsetf(std::ios_base::fmtflags)" in current context.

while pre-physname GDB:
(gdb) complete p 'std::ios_base::unsetf
p 'std::ios_base::unsetf(std::_Ios_Fmtflags)
(gdb) p 'std::ios_base::unsetf(std::_Ios_Fmtflags)' 
$1 = {void (class std::ios_base * const, std::ios_base::fmtflags)} 0x400768 <std::ios_base::unsetf(std::_Ios_Fmtflags)>
(gdb) ptype 'std::ios_base::unsetf(std::_Ios_Fmtflags)'
type = void (class std::ios_base * const, std::ios_base::fmtflags)

This is a regression.  And it will always be a regression for any
physname != DW_AT_linkage_name as with my cross-check patch it prints:
Computed physname <std::ios_base::unsetf(enum std::_Ios_Fmtflags)> does not match demangled <std::ios_base::unsetf(std::_Ios_Fmtflags)> (from linkage <_ZNSt8ios_base6unsetfESt13_Ios_Fmtflags>)


The idea is simple - the compiled application with DWARF will contain:

 <1><135>: Abbrev Number: 17 (DW_TAG_namespace)
    <136>   DW_AT_name        : std     
 <2><44a>: Abbrev Number: 23 (DW_TAG_class_type)
    <44b>   DW_AT_name        : ios_base        
 <3><454>: Abbrev Number: 24 (DW_TAG_subprogram)
    <455>   DW_AT_external    : 1       
    <456>   DW_AT_name        : unsetf   
    <45a>   DW_AT_decl_file   : 1
    <45b>   DW_AT_decl_line   : 612     
    <45d>   DW_AT_MIPS_linkage_name: _ZNSt8ios_base6unsetfESt13_Ios_Fmtflags    
    <461>   DW_AT_accessibility: 1      (public)
    <462>   DW_AT_declaration : 1       
    <463>   DW_AT_object_pointer: <0x46b>
 <4><46b>: Abbrev Number: 25 (DW_TAG_formal_parameter)
    <46c>   DW_AT_type        : <0x18bb> 
    <470>   DW_AT_artificial  : 1       
 <4><471>: Abbrev Number: 26 (DW_TAG_formal_parameter)
    <472>   DW_AT_type        : <0x476> 
 <4><476>: Abbrev Number: 8 (DW_TAG_typedef)
    <477>   DW_AT_name        : fmtflags        
    <47b>   DW_AT_decl_file   : 1
    <47c>   DW_AT_decl_line   : 257
    <47e>   DW_AT_type        : <0x32e>

It does not (currently cannot - discussed at GCC PR debug/40040) contain
DW_AT_low_pc+DW_AT_high_pc for std::io_base::unsetf as it is DW_AT_external.
Therefore the linkage name (physname) which GDB thinks about must be always
exactly DW_AT_linkage_name (*) (**).

(*) As long as the GCC produced DW_AT_MIPS_linkage_name matches the .symtab
    ELF symbol name but there have never been bugs in this part in GCC AFAIK.

(**) DW_AT_MIPS_linkage_name always equals DW_AT_linkage_name.

So physname must be always the same as DW_AT_linkage_name.  Even if GCC
computes DW_AT_linkage_name wrongly then GDB has to be bug-to-bug compatible
with that GCC.  I found the only benefits of physname to reduce the debuginfo
size by omitting DW_AT_linkage_name from DWARF as it is redundant.  But in
such case there should be a verification of GCC-output DW_AT_linkage_name
names if they really match to what GDB thinkgs they should be.  That should
done during distro build of every package before stripping DW_AT_linkage_name.
A rough guess is saving something between 3% to 10% of .debug files size which
may be worth it with all the .debug size reduction efforts.  (As long as other
DWARF consumers do not need DW_AT_linkage_name but that is a bit out of scope
of this discussion, currently I am not aware of any.)


> The bigger issue is what to do for 7.3, which has been in limbo for
> quite some time awaiting resolution of c++/12506 (for which I have
> posted patches).
> 
> I guess there are basically two options:
> 1) Keep physname & apply the 12266/12506 patchset (when approved)
> 2) Adopt Jan's patch and fix the fallout now
> 
> My take on this today is that I think the least risky/most timely
> solution is #1.

As it introduces symbol names which are not compliant with ISO C++ I find it
harmful to get people used for the new names in any new release.  So far the
non-compliant symbol names are present only in FSF gdb-7.2 which could be
forgotten IMO but releasing another stable FSF GDB release with non-compliant
symbol names seems to me making backward compatibility difficult.  Still it is
questionable whether the GDB-only symbol variants can create any compatibility
issues - such as some .gdbinit scripts using such names.


> Gdb has been used with the dwarf2_physname patchset
> for over a year now, and it is not altogether broken. We are seeing
> some regressions from previous releases (none of these are in the
> test suite), and I have been trying to keep on top of them.

There are some regressions when using DW_AT_linkage_name by that patch of mine
but according to quick analysis IIUC they seem to be caused by coding the
testcases based on the ISO C++ non-compliant naming introduced by physname.
Therefore these should be fixed in the testcases.  But these regressions need
to be (attempted to be) fixed first to get definitive answer.


> The 12266/12506 patchset is almost orthogonal to this decision. It
> fixes several additional typedef-related problems. 12506 is fixed by
> using DW_AT_linkage_name, but 12266 is not.

12266 is about `break func(typedefname)' which never worked before physname
(before gdb-7.2).  This is about providing canonicalization in linespec which
you have implemented in the current pending patchset.
decode_variable->cp_canonicalize_string_no_typedefs in:
	Re: [RFA] Typedef'd method parameters [3/4]
	http://sourceware.org/ml/gdb-patches/2011-05/msg00317.html
typedef int t; void f(t p) {} int main () {}
(gdb) b f(t)
Breakpoint 1 at 0x4004db: file 4.C, line 1.

This is a kind of "symbol alias" and I do not find it much critical if it does
not exactly match in some cases - it is just user convenience and it does not
break any symbol references used in the inferior code.


> The only hesitation I have is the compiler issues. Just using
> DW_AT_linkage_name instead of computing it has lead to many test
> regressions with differing compiler versions. [Again, I have only
> tested gcc.]
+
> My big fear with #2 is "fixing" linespecs (yet again). Any changes
> to this area take an unimaginably large amount of time and effort.

I do not see any real regressions except incorrect testcase assumptions.

$ nm -C gdb.cp/cp-relocate.o
0000000000000000 W int func<1>(int)
print func<1>(int)
No symbol "func<1>" in current context.
(gdb) FAIL: gdb.cp/cp-relocate.exp: get address of func<1>(int)

but:
(gdb) print 'int func<1>(int)'
$1 = {int (int)} 0x28 <func<1>(int)>

This is a regression against gdb-7.2 (physname) but in pre-physname it also
did not work.  The symbols canonicalization in decode_variable could also
strip the leading return types.


Thanks,
Jan

Re: The future of dwarf2_physname

[Daniel Jacobowitz]

On Wed, May 18, 2011 at 03:34:43PM -0700, Keith Seitz wrote:
> In short, if gdb uses DW_AT[_MIPS]_linkage_name when available, we
> can bypass constructing the physname *and* canonicalization, so gdb
> would only have to do this when DW_AT[_MIPS]_linkage_name is missing.
> Today this only happens AFAIK with gcc and ctors. I don't know about
> other compilers.

For all intents and purposes, no other current compiler emits
DW_AT_MIPS_linkage_name.

> This could be a sizable performance win (not yet tested or
> quantified). While it does introduce some regressions, many are
> compiler problems and some are template problems (the linkage name
> contains the return type, and 1) gdb isn't equipped to deal with
> that; 2) it makes for a horrible user experience having to specify it
> all the time). There might be others, I have only delved into this a
> bit myself today.
> 
> So the first question to be asked here is, Should we adopt Jan's
> patch, or some variation of it, to use DW_AT_linkage_name when
> available?

I've spent a lot of time on this question.  It's similar to the
approach I took in my previous implementation, but I was never happy
with it.  You get bizarre differences in behavior between compilers
(although the same thing does admittedly happen just based on the
DWARF output).

GCC ideally ought to get away from outputting linkage names; it's a
big space cost in the debug information.  That offers a potential
tie-breaker between the two strategies; keep using the linkage names
for old versions of GCC, and transition away as GCC improves.

In general, the sort of cross-check Jan has implemented could never
reliably pass without extensions to DWARF.  Once you get up into
templates, especially, there are template arguments that are hard
or impossible to represent in template parameter DIEs.

-- 
Daniel Jacobowitz

Re: The future of dwarf2_physname

[Tom Tromey]

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> So the first question to be asked here is, Should we adopt Jan's
Keith> patch, or some variation of it, to use DW_AT_linkage_name when
Keith> available?

I think so.  In fact, I think it should be preferred.

My reasoning is that when DW_AT_linkage_name exists, it is definitive.
Our exposure here is to compiler bugs (which would be very bad ones --
but not our problem) and to demangler bugs -- something I am not
extremely concerned about.  And, we have seen bugs where the DWARF is
very incorrect but the linkage name is not (I think these have been
fixed, but the point is that they confound physname, and even reasonably
recent compilers, like the F13 gcc, still have this.)

However, it is also now clear to me that we do need physname.

First, there are cases where GCC does not emit DW_AT_linkage_name.  I
finally filed the one case we know about:

    http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49047

Second, maybe DW_AT_linkage_name will be dropped in order to shrink
debuginfo.

Third, there are other compilers.

Keith> The only hesitation I have is the compiler issues. Just using
Keith> DW_AT_linkage_name instead of computing it has lead to many test
Keith> regressions with differing compiler versions. [Again, I have only
Keith> tested gcc.]

Based on Jan's response, and discussions we've had recently, it seems
that the regressions in question are really just differences between
physname and the demangler, correct?  And the problem specifically is
that template functions have the return type in the demangled form,
requiring users, for the time being, to add quotes?

Keith> The bigger issue is what to do for 7.3, which has been in limbo for
Keith> quite some time awaiting resolution of c++/12506 (for which I have
Keith> posted patches).

Keith> I guess there are basically two options:
Keith> 1) Keep physname & apply the 12266/12506 patchset (when approved)
Keith> 2) Adopt Jan's patch and fix the fallout now

Or (3) Apply both and make linkage_name the preferred choice.

At present this is what I think we should do.

Keith> My big fear with #2 is "fixing" linespecs (yet again). Any changes to
Keith> this area take an unimaginably large amount of time and effort.

I think for 7.3 we would accept that there are some regressions, and
then fix them for a future release.

I am partial to Jan's view that we should be using the full name of the
object, and then have symbol aliases or code in linespec to expand nice
shortcuts to the full name.

Tom

Re: The future of dwarf2_physname

[Tom Tromey]

>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:

Daniel> In general, the sort of cross-check Jan has implemented could never
Daniel> reliably pass without extensions to DWARF.  Once you get up into
Daniel> templates, especially, there are template arguments that are hard
Daniel> or impossible to represent in template parameter DIEs.

DWARF 4 added some stuff to help with this, but I think g++ hasn't yet
caught up.  What problems do you know about?

I know of this:

    http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41736

... namely, that g++ doesn't emit a value for a pointer-to-member
template parameter.  But this is a g++ bug, not a DWARF omission.

Also there is:

    http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33861

I don't think I totally understand the issues with this one, but the
DWARF just has an address, so presumably in some scenarios we can wind
up with an odd canonical name (if we can't find the name corresponding
to that address).


It seems to me that if the DWARF spec is incomplete, then that is an
argument in favor of either (1) fixing DWARF and g++ or (2) keeping
DW_AT_linkage_name in g++ -- but not just dropping linkage-name, as that
would cause user-visible regressions.

Tom

Re: The future of dwarf2_physname

[Keith Seitz]

Hi, Jan,

On 05/19/2011 12:23 PM, Jan Kratochvil wrote:
> If the linkage name is wrong for whatever reason this is an ABI issue for GCC,
> it is out of scope of GDB.  Linkage name is defined by the ISO C++ standard,
> neither GCC nor GDB can change it.  If GCC is not ISO C++ compliant then GCC
> should be fixed (and possibly deal with the ABI breakage) but such ISO C++
> compliance issue is out of scope of GDB.

We work around GCC ABI issues all the time GDB. dwarf2read.cc is riddled 
with workarounds for various flavors of GCC.

> Trying to introduce new "better" naming for C++ methods which is in 90% the
> same as GCC's and ISO C++'s idea seems a bit messy for me.

If you mean "better" as in "more reliable", then I'm afraid that *was* 
the point. I don't like it, either, TBH, because there's a bunch of 
overhead involved. But it seemed like a reasonable thing to do at the 
time, and it still does, especially if you take GCC out of the discussion.

I wouldn't say that DW_AT_MIPS_linkage_name is any more ISO C++ 
compliant than anything else. Just because the compiler outputs it does 
not necessarily make it sacrosanct. [gcc/33861]

> That is the GCC and ISO C++ naming should be always available in GDB.  I am
> not against allowing also other aliases for the same symbol.  The availability
> of symbol aliases is discussed more at the end of this mail.

I agree.

> This is a regression.  And it will always be a regression for any
> physname != DW_AT_linkage_name as with my cross-check patch it prints:
> Computed physname<std::ios_base::unsetf(enum std::_Ios_Fmtflags)>  does not match demangled<std::ios_base::unsetf(std::_Ios_Fmtflags)>  (from linkage<_ZNSt8ios_base6unsetfESt13_Ios_Fmtflags>)

That's simply a bug. They get found, they get fixed. The sky is not falling.

> I do not see any real regressions except incorrect testcase assumptions.

Unless the assumption is that gdb can only set breakpoints on linkage 
names, I don't call the test cases broken or illegal. If your argument 
is that you don't like the way it was solved, that's an entirely 
different assertion, and one with much more merit.

> $ nm -C gdb.cp/cp-relocate.o
> 0000000000000000 W int func<1>(int)
> print func<1>(int)
> No symbol "func<1>" in current context.
> (gdb) FAIL: gdb.cp/cp-relocate.exp: get address of func<1>(int)
>
> but:
> (gdb) print 'int func<1>(int)'
> $1 = {int (int)} 0x28<func<1>(int)>
>
> This is a regression against gdb-7.2 (physname) but in pre-physname it also
> did not work.  The symbols canonicalization in decode_variable could also
> strip the leading return types.

Another bug slipped in. It, too, can be fixed. [That looks like a 
psymtab-related bug, btw.]

But as I keep saying, I am not a maintainer. This is a decision for 
maintainers. I was simply asking how maintainers wanted me to proceed.

Two maintainers believe that making the switch now is the best approach. 
As far as I am concerned, the matter is closed.

Keith

Re: The future of dwarf2_physname

[Jan Kratochvil]

Hi Keith,

On Fri, 20 May 2011 21:53:24 +0200, Keith Seitz wrote:
> I wouldn't say that DW_AT_MIPS_linkage_name is any more ISO C++
> compliant than anything else.

We still do not get to the agreement.  The primary source of addresses are ELF
symbols.  DW_AT_low_pc+DW_AT_high_pc are ignored in many cases.

(In other cases DW_AT_low_pc+DW_AT_high_pc is the only available address we
have but that is offtopic here.)

If you find different linkage name than DW_AT_MIPS_linkage_name then you will
not find the ELF symbol and therefore GDB will fail to find the function, as
I already reproduced on the sample code I provided.


> Just because the compiler outputs it
> does not necessarily make it sacrosanct. [gcc/33861]

DW_AT_MIPS_linkage_name is fortunately or unfortunately always right because
it is used for the ELF symbol.


> >This is a regression.  And it will always be a regression for any
> >physname != DW_AT_linkage_name as with my cross-check patch it prints:
> >Computed physname<std::ios_base::unsetf(enum std::_Ios_Fmtflags)>  does not match demangled<std::ios_base::unsetf(std::_Ios_Fmtflags)>  (from linkage<_ZNSt8ios_base6unsetfESt13_Ios_Fmtflags>)
> 
> That's simply a bug. They get found, they get fixed. The sky is not falling.

The sky has fallen as without that patch of mine GDB failed to find the
function as I have shown before:

Message-ID: <20110516154851.GA24555@host1.jankratochvil.net>
During symbol reading, Computed physname <C::m(struct x *)> does not match demangled <C::m(x*)> (from linkage <_ZN1C1mEP1x>) - DIE at 0x3d [in module .../1].
(gdb) p c.m
pre-phyname:	   $2 = {void (C *, x *)} 0x4004f0 <C::m(x*)>
HEAD + your patch: Cannot take address of method m.
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This is a regression.  Until we find agreement on this point the mails do not
make sense.

And if you fix the specific `struct' physname problem I can find many other
problems as long as any "Computed physname ... does not match demangled"
message gets printed.  After you fix all of the problems you will reach the
functionality of my patch preferring DW_AT_MIPS_linkage_name and we can
continue the discussion.


> >I do not see any real regressions except incorrect testcase assumptions.
> 
> Unless the assumption is that gdb can only set breakpoints on
> linkage names,

Yes.  You do not always have DWARF for very every part of the program and all
of its libraries, you need to stay compatible with ELF .symtab naming.


> If your argument is that you don't like the way it was solved, that's an
> entirely different assertion, and one with much more merit.

I really do not mind which way to solve it, I would never work on GDB if
I mind anything like it.  But it must be able to print and break very every
function in at least libstdc++so.6 and something big like
/usr/lib/debug/usr/lib64/libwebkitgtk-1.0.so.debug .  There are about 10000
failed lookups on it now.


> Another bug slipped in. It, too, can be fixed. [That looks like a
> psymtab-related bug, btw.]

Yes, you can be fixing it all the following months.  Or we can use
DW_AT_linkage_name till the time you fix it.

We need to look verify your fixes against DW_AT_linkage_name anyway.


> Two maintainers believe that making the switch now is the best
> approach. As far as I am concerned, the matter is closed.

The problem is physname introduced some new feature - like it introduced
DMGL_RET_POSTFIX-like functionality.  If we go the standard demangling way we
regress a bit against gdb-7.2 (but not against gdb-7.1).

So I am trying to find a way to regress neither against gdb-7.1 nor against
gdb-7.2.


Thanks,
Jan

Re: The future of dwarf2_physname

[Tom Tromey]

Keith> I wouldn't say that DW_AT_MIPS_linkage_name is any more ISO C++
Keith> compliant than anything else. Just because the compiler outputs it
Keith> does not necessarily make it sacrosanct. [gcc/33861]

I think that bug shows the opposite, actually.  If you look at the
DWARF:

 <1><25>: Abbrev Number: 2 (DW_TAG_structure_type)
    <26>   DW_AT_name        : (indirect string, offset: 0x77): Qux<((char*)(& name))>  

That is, the bug here is that G++ emits a bad and weird DW_AT_name.

But, if you look at the member function:

 <2><31>: Abbrev Number: 3 (DW_TAG_subprogram)
    <32>   DW_AT_external    : 1        
    <33>   DW_AT_name        : foo      
    <37>   DW_AT_decl_file   : 1        
    <38>   DW_AT_decl_line   : 7        
    <39>   DW_AT_MIPS_linkage_name: (indirect string, offset: 0x5a): _ZN3QuxIXadL_Z4nameEEE3fooEv       

.. run through c++filt:

    <39>   DW_AT_MIPS_linkage_name: (indirect string, offset: 0x5a): Qux<&name>::foo()  

Here, the linkage name and demangler are correct.

Also even if you strip down the class' DW_AT_name, you probably have
trouble reconstructing the template argument:

 <2><51>: Abbrev Number: 5 (DW_TAG_template_value_param)
    <52>   DW_AT_name        : V        
    <54>   DW_AT_type        : <0xaf>   
    <58>   DW_AT_location    : 6 byte block: 3 0 0 0 0 9f       (DW_OP_addr: 0; DW_OP_stack_value)

... since that is just the address of 'name', which IIUC may appear
elsewhere, perhaps without debuginfo.  I guess it ought to always appear
in minimal symbols in that case... maybe it isn't as bad as I thought.


I get this from my patch branch:

opsy. ../gdb -readnow -batch -ex 'set complaint 10000' -ex 'file /tmp/q.o'
During symbol reading...Computed physname <Qux<(char*)(&name)>::foo()> does not match demangled <Qux<&name>::foo()> (from linkage <_ZN3QuxIXadL_Z4nameEEE3fooEv>) - DIE at 0x31 [in module /tmp/q.o]...

Jan> That is the GCC and ISO C++ naming should be always available in
Jan> GDB.  I am not against allowing also other aliases for the same
Jan> symbol.  The availability of symbol aliases is discussed more at
Jan> the end of this mail.

Keith> I agree.

Great, we have a starting point :)

Jan> I do not see any real regressions except incorrect testcase assumptions.

Keith> Unless the assumption is that gdb can only set breakpoints on linkage
Keith> names, I don't call the test cases broken or illegal. If your argument
Keith> is that you don't like the way it was solved, that's an entirely
Keith> different assertion, and one with much more merit.

Yes, I actually agree with both of you, in the sense that I think both
sets of tests ought to pass.

Tom

Re: The future of dwarf2_physname

[Daniel Jacobowitz]

On Fri, May 20, 2011 at 01:25:49PM -0600, Tom Tromey wrote:
> DWARF 4 added some stuff to help with this, but I think g++ hasn't yet
> caught up.  What problems do you know about?

It's been five years since I was really working on this, but this...

> Also there is:
> 
>     http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33861
> 
> I don't think I totally understand the issues with this one, but the
> DWARF just has an address, so presumably in some scenarios we can wind
> up with an odd canonical name (if we can't find the name corresponding
> to that address).

... is a pretty good example (I don't think it's the only kind, but I
don't have anything more to hand).  You can't reliably go from an
address to a name and get the same thing the compiler got.

I think there's a way (for function templates?  parameters to
templated functions?) to get floating point constants mangled, too.

> It seems to me that if the DWARF spec is incomplete, then that is an
> argument in favor of either (1) fixing DWARF and g++ or (2) keeping
> DW_AT_linkage_name in g++ -- but not just dropping linkage-name, as that
> would cause user-visible regressions.

Completely agree.

-- 
Daniel Jacobowitz

Re: The future of dwarf2_physname

[Daniel Jacobowitz]

On Fri, May 20, 2011 at 02:38:51PM -0600, Tom Tromey wrote:
> Also even if you strip down the class' DW_AT_name, you probably have
> trouble reconstructing the template argument:
> 
>  <2><51>: Abbrev Number: 5 (DW_TAG_template_value_param)
>     <52>   DW_AT_name        : V        
>     <54>   DW_AT_type        : <0xaf>   
>     <58>   DW_AT_location    : 6 byte block: 3 0 0 0 0 9f       (DW_OP_addr: 0; DW_OP_stack_value)
> 
> ... since that is just the address of 'name', which IIUC may appear
> elsewhere, perhaps without debuginfo.  I guess it ought to always appear
> in minimal symbols in that case... maybe it isn't as bad as I thought.

You can never make this conversion from address to name reliably;
e.g. __attribute__((alias)), ICF [Identical Code Folding] in the
linker, et cetera.

> 
> 
> I get this from my patch branch:
> 
> opsy. ../gdb -readnow -batch -ex 'set complaint 10000' -ex 'file /tmp/q.o'
> During symbol reading...Computed physname <Qux<(char*)(&name)>::foo()> does not match demangled <Qux<&name>::foo()> (from linkage <_ZN3QuxIXadL_Z4nameEEE3fooEv>) - DIE at 0x31 [in module /tmp/q.o]...

What I think would be extraordinarily useful would be to put this
feature together with GCC's quality testsuite and actually get all the
names right... but that really begs the question of which component we're
testing here and where the test should live!

-- 
Daniel Jacobowitz

Re: The future of dwarf2_physname

[Jan Kratochvil]

On Thu, 19 May 2011 00:34:43 +0200, Keith Seitz wrote:
> I guess there are basically two options:
> 1) Keep physname & apply the 12266/12506 patchset (when approved)
> 2) Adopt Jan's patch and fix the fallout now
> 
> My take on this today is that I think the least risky/most timely
> solution is #1.

#1 is still IMO a lot of work (see "Computed physname" errors at the end of
this mail) and #1 should always be backed by #2 anyway and as #2 with current
GCCs has the same user experience as #1 I do not see a reason why to wait on #1
for gdb-7.3.

I have concerns about regressions for gdb-7.3, I would like to fix regressions
gdb-7.1 (pre-physname) -> gdb-7.2 (physname) but possibly not introduce
regressions gdb-7.2 (physname) -> gdb-7.3 (?).  Trying to propose a simple
DW_AT_linkage_name patch of mine but I do not yet have it finished.


problem #1 of post-physname GDB

(gdb) p 'f<char>()' 
$1 = {long (void)} 0x4004e4 <f<char>()>
(gdb) p 'long f<char>()' 
$2 = {<text variable, no debug info>} 0x4004e4 <f<char>()>
(gdb) p 'f<short>()' 
No symbol "f<short>()" in current context.
(gdb) p 'long f<short>()' 
$3 = {<text variable, no debug info>} 0x4004fb <long f<short>()>

==> dis.h <==
template <typename T>
long f () { return 1; }
==> dis1.C <==
#include "dis.h"
int main () { f<char> (); }
==> dis2.C <==
#include "dis.h"
void g () { f<short> (); }

The two different aliases for 0x4004e4 where the $2 one does not have any type
is a bug, at least both should have the same type.  It can be seen also on the
f<short>() case ($3) where the return type missing alias is not present because
only ELF (and not DWARF) symbol is present.


I haven't found how to reproduce the regression
"Cannot take address of method m." for function templates (which use the
return type in their linkage) as I haven't found how to reference function
template externally from a class - this field is missing there in such case:
class C { public:
  template <typename T>
  static long f ();
};
And I get
	error: explicit specialization in non-namespace scope ‘class C’
for
class C { public:
  template <typename T>
  static long f ();
  template <>
  static long f<char> ();
};
And in other cases there is alread the template function definition with
associated DWARF which gets processed by GDB-physname and the return type gets
stripped.


problem #2 of post-physname GDB

(gdb) p 'int f<long>()' 
$1 = {<text variable, no debug info>} 0x40050f <f<long>()>
(gdb) p 'short f<long>()' 
$2 = {<text variable, no debug info>} 0x400531 <f<long>()>
(gdb) p 'f<long>()' 
$3 = {int (void)} 0x40050f <f<long>()>

==> manya.C <==
template <typename T>
int f () { return 4; }
int a () { return f<long> () == 4; }
extern int b ();
int main () { return a () && b () ? 0 : 1; }
==> manyb.C <==
template <typename T>
short f () { return 2; }
int b () { return f<long> () == 2; }

As the return type is stripped the name can become ambiguous.  Different
template functions with same name, same parameter types but different return
type can be linked in a single program.  It is true one cannot declare both
such template functions in a single CU (Compilation Unit) so I do not expect
such case is found in real world.


I was trying to propose DMGL_RET_POSTFIX but it has other drawbacks and it is
in fact a new feature (and not a regression fixup) because the trailing type
was not present in gdb-7.2 anyway.  So it can be considered after some tidy up
for gdb-7.4 but it makes no sense for gdb-7.3.
	Re: [rfc] physname cross-check #2
	http://sourceware.org/ml/gdb-patches/2011-05/msg00466.html
Therefore it would apply for both ELF and both DWARF symbols producing the same
form.  For ELF symbols implemented by (patched) DMGL_RET_POSTFIX and for DWARF
symbols missing DW_AT_linkage_name implemented by GDB-physname code.

That is the 'int f<long>()' and 'short f<long>()' would be uniquely searchable
as `f<long>()int' and `f<long>()short'.  Still `nm -C', `objdump -C' normally
do not (and even cannot) request DMGL_RET_POSTFIX therefore to be able to
`break symbol_that_I_see_in_objdump_C' these DMGL_RET_POSTFIX-generated
symbols should be only as aliases to also available non-DMGL_RET_POSTFIX
symbols.


Then there is the code for NAME_KIND_PRINT (with typedefs etc. preserved,
different from NAME_KIND_PHYS with typedefs etc. resolved).  That may be a nice
new feature for gdb-7.4 but as such I do not find it relevant for gdb-7.3.
cp_canonicalize_string_no_typedefs in decode_variable is also a nice feature
(`break func(typedef)') but it is a new feature not present anytime before,
therefore for gdb-7.4.

Also the current NAME_KIND_PRINT behavior does not seem clearly the right one:
	typedef int t; void f(t p) {} int main () {}
(gdb) p f
$1 = {void (t)} 0x4004d4 <f(t)>
 - correct, in this CU `f' has the NAME_KIND_PRINT `t'.
(gdb) info sym 0x4004d4
f(int) in section .text
 - correct - `info symbol' uses only minimal=ELF symbols
(gdb) complete p 'f(
p 'f(int)
p 'f(t)
 - I do not find it clear this is right.  There should be definitely the
   `f(int)' - demangled linkage name - variant.  But whether to show the
   "convenient" aliases as SYMBOL_SEARCH_NAME I do not find clear, it may be
   better but it also complicates the list of all the available overloads for
   a function name, one should ask practical C++ programmer for it IMO.
   After all there is still only a single function `f':
(gdb) p f(t)
$2 = {void (t)} 0x4004d4 <f(t)>
(gdb) p f(int)
$3 = {void (t)} 0x4004d4 <f(t)>


Regarding the proposals to fix dwarf2_physname computation instead of
preferring DW_AT_linkage_name.  ISTM we have never found the agreement with
Keith, when DW_AT_linkage_name is present it is guaranteed to be correct and
I do not know why not to use it.  It is a good verification way to fix all the
existing dwarf2_physname computation issues so that in the future one may stop
shipping the redundant size-costly (3-10% of .debug files) DW_AT_linkage_name
strings.  But still even in future I do not see why GDB should not prefer
DW_AT_linkage_name over the dwarf2_physname computated name, when it is
present.

Preferring dwarf2_physname computation over DW_AT_linkage_name - when they
differ - means a failure to match the ELF demangled symbols leading to the
error "Cannot take address of method m." and/or missing function types etc.
So that way I find as always wrong.

Keith proposes to fix dwarf2_physname instead of using DW_AT_linkage_name.
If it really always matches sure there is no difference although (a) it still
may take some time for gdb-7.3 and (b) even in the future I do not see where
is any advantage over preferring DW_AT_linkage_name when it is available.

Bugs to fix for /usr/lib/debug/usr/lib64/libwebkitgtk-1.0.so.0.7.0.debug
from webkitgtk-debuginfo-1.4.0-1.fc15.x86_64 - it is definitely not
a complete bug list, raw messages list has 56490 lines:
http://people.redhat.com/jkratoch/webkitgtk-debuginfo-1.4.0-1.fc15.x86_64-gdb.err.xz
Computed physname <JSC::JSCell::markChildren(struct JSC::MarkStack &)> does not match demangled <JSC::JSCell::markChildren(JSC::MarkStack&)> (from linkage <_ZN3JSC6JSCell12markChildrenERNS_9MarkStackE>)
 - you reported the `struct' bug is already fixed in a patch being prepared
Computed physname <WebCore::lineCapName(enum WebCore::LineCap)> does not match demangled <WebCore::lineCapName(WebCore::LineCap)> (from linkage <_ZN7WebCore11lineCapNameENS_7LineCapE>)
 - IIRC you reported the `enum' bug is also already fixed
Computed physname <WTF::FixedArray<double, 8u>::data()> does not match demangled <WTF::FixedArray<double, 8ul>::data()> (from linkage <_ZN3WTF10FixedArrayIdLm8EE4dataEv>)
 - FYI its DW_TAG_class_type DW_AT_name is: FixedArray<double, 8u>
Computed physname <WTF::PtrHash<void const*>::hash(void*)> does not match demangled <WTF::PtrHash<void const*>::hash(void const*)> (from linkage <_ZN3WTF7PtrHashIPKvE4hashES2_>)
 - const issue
Computed physname <WebCore::RangeBoundaryPoint::offset()> does not match demangled <WebCore::RangeBoundaryPoint::offset() const> (from linkage <_ZNK7WebCore18RangeBoundaryPoint6offsetEv>)
 - maybe different const issue
Computed physname <WebCore::IntPoint::operator GdkPoint() const> does not match demangled <WebCore::IntPoint::operator _GdkPoint() const> (from linkage <_ZNK7WebCore8IntPointcv9_GdkPointEv>)
 - some leading _
Computed physname <JSC::JSParser::parseBreakStatement<JSC::ASTBuilder>(JSC::ASTBuilder&)> does not match demangled <JSC::ASTBuilder::Statement JSC::JSParser::parseBreakStatement<JSC::ASTBuilder>(JSC::ASTBuilder&)> (from linkage <_ZN3JSC8JSParser19parseBreakStatementINS_10ASTBuilderEEENT_9StatementERS3_>)
 - missing return type
etc.


After all I still do not have a patch at hand but I gave some thoughts of mine
on it.


Thanks,
Jan

Re: The future of dwarf2_physname

[Tom Tromey]

Jan> I have concerns about regressions for gdb-7.3, I would like to fix
Jan> regressions gdb-7.1 (pre-physname) -> gdb-7.2 (physname) but
Jan> possibly not introduce regressions gdb-7.2 (physname) -> gdb-7.3
Jan> (?).

I don't see how that is really possible.
I am willing to accept some minor regressions for 7.3 in exchange for
doing better in the long run.

Jan> I was trying to propose DMGL_RET_POSTFIX but it has other drawbacks
Jan> and it is in fact a new feature (and not a regression fixup)
Jan> because the trailing type was not present in gdb-7.2 anyway.

I think we should not take this approach, for the simple reason that it
will be weird to C++ users.  I think it would be better to just require
quotes in this case for 7.3, like "break 'int f<int> ()'", then see
about fixing up linespec, or whatever, in 7.4+.  Also for later would be
making some kind of symbol alias for this, so that in the normal case
users can omit the return type.

Jan> (gdb) complete p 'f(
Jan> p 'f(int)
Jan> p 'f(t)
Jan>  - I do not find it clear this is right.  There should be
Jan>  definitely the `f(int)' - demangled linkage name - variant.  But
Jan>  whether to show the "convenient" aliases as SYMBOL_SEARCH_NAME I
Jan>  do not find clear, it may be better but it also complicates the
Jan>  list of all the available overloads for a function name, one
Jan>  should ask practical C++ programmer for it IMO.

I think this one is ok, but I can see how that could go either way.
I find it a side issue at present.

Jan> Regarding the proposals to fix dwarf2_physname computation instead
Jan> of preferring DW_AT_linkage_name.  ISTM we have never found the
Jan> agreement with Keith, when DW_AT_linkage_name is present it is
Jan> guaranteed to be correct and I do not know why not to use it.

I think we should move forward with it, on the basis that it is more
correct and probably has better performance to boot.

I suggest something like the appended, on top of Keith's patches and
your patch.  The idea here is to prefer the fast path, but have a debug
setting so we can easily do physname checking.

WDYT?  I didn't run it through the test suite or anything yet.

Jan> Keith proposes to fix dwarf2_physname instead of using
Jan> DW_AT_linkage_name.

We must do both in the medium term.

Tom

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 7530e3e..fff8e3b 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -119,6 +119,9 @@ _STATEMENT_PROLOGUE;
 /* When non-zero, dump DIEs after they are read in.  */
 static int dwarf2_die_debug = 0;
 
+/* When non-zero, cross-check physname against demangler.  */
+static int check_physname = 0;
+
 static int pagesize;
 
 /* When set, the file that we're processing is known to have debugging
@@ -5142,71 +5145,71 @@ dwarf2_full_name (char *name, struct die_info *die, struct dwarf2_cu *cu)
 static const char *
 dwarf2_physname (char *name, struct die_info *die, struct dwarf2_cu *cu)
 {
-  const char *physname = dwarf2_compute_name (name, die, cu, NAME_KIND_PHYS);
+  const char *physname;
   struct attribute *attr;
-  const char *mangled, *retval;
-  char *demangled, *canon;
-  struct cleanup *back_to;
-
-  /* Do not check PHYSNAME if it never will be needed by GDB.  GDB does not
-     need to support something it has no use for.  */
-  if (!die_needs_namespace (die, cu))
-    return physname;
+  const char *retval, *mangled = NULL;
+  char *canon = NULL;
+  struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
+  int need_copy = 1;
 
   attr = dwarf2_attr (die, DW_AT_linkage_name, cu);
   if (!attr)
     attr = dwarf2_attr (die, DW_AT_MIPS_linkage_name, cu);
-  if (!attr || !DW_STRING (attr))
+  /* DW_AT_linkage_name is missing in some cases - depend on what GDB
+     has computed.  */
+  if (attr && DW_STRING (attr))
     {
-      /* DW_AT_linkage_name is missing in some cases - depend on what GDB has
-	 computed.  */
-
-      return physname;
-    }
-  mangled = DW_STRING (attr);
-
-  /* As both DW_AT_linkage_name (MANGLED) and computed PHYSNAME are present
-     cross-check them.  */
-
-  back_to = make_cleanup (null_cleanup, 0);
+      char *demangled;
 
-  demangled = cplus_demangle (mangled, DMGL_PARAMS | DMGL_ANSI);
-  if (demangled)
-    make_cleanup (xfree, demangled);
-  else
-    demangled = (char *) mangled;
+      mangled = DW_STRING (attr);
 
-  if (cu->language == language_cplus)
-    {
-      canon = cp_canonicalize_string (demangled);
-      if (canon != NULL)
-	make_cleanup (xfree, canon);
+      demangled = cplus_demangle (mangled, (DMGL_PARAMS | DMGL_ANSI
+					    | DMGL_VERBOSE));
+      if (demangled)
+	{
+	  make_cleanup (xfree, demangled);
+	  canon = demangled;
+	}
       else
-	canon = demangled;
+	{
+	  canon = (char *) mangled;
+	  need_copy = 0;
+	}
     }
-  else
-    canon = demangled;
 
-  if (strcmp (physname, canon) != 0)
+  if (canon == NULL || check_physname)
     {
-      /* It may not mean a bug in GDB.  The compiler could also compute
-	 DW_AT_linkage_name incorrectly.  But in such case GDB would need to be
-	 bug-to-bug compatible.  */
+      physname = dwarf2_compute_name (name, die, cu, NAME_KIND_PHYS);
 
-      complaint (&symfile_complaints,
-		 _("Computed physname <%s> does not match demangled <%s> "
-		   "(from linkage <%s>) - DIE at 0x%x [in module %s]"),
-		 physname, canon, mangled, die->offset, cu->objfile->name);
+      if (canon != NULL && strcmp (physname, canon) != 0)
+	{
+	  /* It may not mean a bug in GDB.  The compiler could also
+	     compute DW_AT_linkage_name incorrectly.  But in such case
+	     GDB would need to be bug-to-bug compatible.  */
+
+	  complaint (&symfile_complaints,
+		     _("Computed physname <%s> does not match demangled <%s> "
+		       "(from linkage <%s>) - DIE at 0x%x [in module %s]"),
+		     physname, canon, mangled, die->offset, cu->objfile->name);
 
-      /* Prefer DW_AT_linkage_name (in the CANON form) - when it is available
-	 here - over computed PHYSNAME.  It is safer against both buggy GDB and
-	 buggy compilers.  */
+	  /* Prefer DW_AT_linkage_name (in the CANON form) - when it
+	     is available here - over computed PHYSNAME.  It is safer
+	     against both buggy GDB and buggy compilers.  */
 
-      retval = obsavestring (canon, strlen (canon),
-			     &cu->objfile->objfile_obstack);
+	  retval = canon;
+	}
+      else
+	{
+	  retval = physname;
+	  need_copy = 0;
+	}
     }
   else
-    retval = physname;
+    retval = canon;
+
+  if (need_copy)
+    retval = obsavestring (retval, strlen (retval),
+			   &cu->objfile->objfile_obstack);
 
   do_cleanups (back_to);
   return retval;
@@ -16247,6 +16250,15 @@ show_dwarf2_always_disassemble (struct ui_file *file, int from_tty,
 		    value);
 }
 
+static void
+show_check_physname (struct ui_file *file, int from_tty,
+		     struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file,
+		    _("Whether to check \"physname\" is %s.\n"),
+		    value);
+}
+
 void _initialize_dwarf2_read (void);
 
 void
@@ -16302,6 +16314,14 @@ The value is the maximum depth to print."),
 			    NULL,
 			    &setdebuglist, &showdebuglist);
 
+  add_setshow_boolean_cmd ("check-physname", no_class, &check_physname, _("\
+Set cross-checking of \"physname\" code against demangler."), _("\
+Show cross-checking of \"physname\" code against demangler."), _("\
+When enabled, GDB's internal \"physname\" code is checked against\n\
+the demangler."),
+			   NULL, show_check_physname,
+			   &setdebuglist, &showdebuglist);
+
   c = add_cmd ("gdb-index", class_files, save_gdb_index_command,
 	       _("\
 Save a gdb-index file.\n\

Re: The future of dwarf2_physname

[Keith Seitz]

On 05/23/2011 12:52 PM, Tom Tromey wrote:
> I suggest something like the appended, on top of Keith's patches and
> your patch.  The idea here is to prefer the fast path, but have a debug
> setting so we can easily do physname checking.
>
> WDYT?  I didn't run it through the test suite or anything yet.

I know this question was for Jan, but I just wanted to chime in here and 
say that I was going to propose/do exactly the same thing. I'm all for it.

Keith

[rfc 1/2] libiberty/ options code reshuffle [Re: The future of dwarf2_physname]

[Jan Kratochvil]

Hi,

this patch is not interesting, no functionality change, just a prerequisite
for [rfc 2/2].


Thanks,
Jan


libiberty/
2011-05-24  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* cp-demangle.c (struct d_print_info): Remove field options.
	(d_print_init, d_print_comp, d_print_mod_list, d_print_mod)
	(d_print_function_type, d_print_array_type, d_print_expr_op)
	(d_print_cast, d_print_subexpr): Add parameter options, update all the
	callers.

--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -278,8 +278,6 @@ struct d_growable_string
 enum { D_PRINT_BUFFER_LENGTH = 256 };
 struct d_print_info
 {
-  /* The options passed to the demangler.  */
-  int options;
   /* Fixed-length allocated buffer for demangled data, flushed to the
      callback with a NUL termination once full.  */
   char buf[D_PRINT_BUFFER_LENGTH];
@@ -436,7 +434,7 @@ static void
 d_growable_string_callback_adapter (const char *, size_t, void *);
 
 static void
-d_print_init (struct d_print_info *, int, demangle_callbackref, void *);
+d_print_init (struct d_print_info *, demangle_callbackref, void *);
 
 static inline void d_print_error (struct d_print_info *);
 
@@ -454,32 +452,32 @@ static inline void d_append_string (struct d_print_info *, const char *);
 static inline char d_last_char (struct d_print_info *);
 
 static void
-d_print_comp (struct d_print_info *, const struct demangle_component *);
+d_print_comp (struct d_print_info *, const struct demangle_component *, int);
 
 static void
 d_print_java_identifier (struct d_print_info *, const char *, int);
 
 static void
-d_print_mod_list (struct d_print_info *, struct d_print_mod *, int);
+d_print_mod_list (struct d_print_info *, struct d_print_mod *, int, int);
 
 static void
-d_print_mod (struct d_print_info *, const struct demangle_component *);
+d_print_mod (struct d_print_info *, const struct demangle_component *, int);
 
 static void
 d_print_function_type (struct d_print_info *,
                        const struct demangle_component *,
-                       struct d_print_mod *);
+                       struct d_print_mod *, int);
 
 static void
 d_print_array_type (struct d_print_info *,
                     const struct demangle_component *,
-                    struct d_print_mod *);
+                    struct d_print_mod *, int);
 
 static void
-d_print_expr_op (struct d_print_info *, const struct demangle_component *);
+d_print_expr_op (struct d_print_info *, const struct demangle_component *, int);
 
 static void
-d_print_cast (struct d_print_info *, const struct demangle_component *);
+d_print_cast (struct d_print_info *, const struct demangle_component *, int);
 
 static int d_demangle_callback (const char *, int,
                                 demangle_callbackref, void *);
@@ -3293,10 +3291,9 @@ d_growable_string_callback_adapter (const char *s, size_t l, void *opaque)
 /* Initialize a print information structure.  */
 
 static void
-d_print_init (struct d_print_info *dpi, int options,
-              demangle_callbackref callback, void *opaque)
+d_print_init (struct d_print_info *dpi, demangle_callbackref callback,
+	      void *opaque)
 {
-  dpi->options = options;
   dpi->len = 0;
   dpi->last_char = '\0';
   dpi->templates = NULL;
@@ -3392,9 +3389,9 @@ cplus_demangle_print_callback (int options,
 {
   struct d_print_info dpi;
 
-  d_print_init (&dpi, options, callback, opaque);
+  d_print_init (&dpi, callback, opaque);
 
-  d_print_comp (&dpi, dc);
+  d_print_comp (&dpi, dc, options);
 
   d_print_flush (&dpi);
 
@@ -3537,8 +3534,8 @@ d_pack_length (const struct demangle_component *dc)
    if needed.  */
 
 static void
-d_print_subexpr (struct d_print_info *dpi,
-		 const struct demangle_component *dc)
+d_print_subexpr (struct d_print_info *dpi, const struct demangle_component *dc,
+		 int options)
 {
   int simple = 0;
   if (dc->type == DEMANGLE_COMPONENT_NAME
@@ -3546,7 +3543,7 @@ d_print_subexpr (struct d_print_info *dpi,
     simple = 1;
   if (!simple)
     d_append_char (dpi, '(');
-  d_print_comp (dpi, dc);
+  d_print_comp (dpi, dc, options);
   if (!simple)
     d_append_char (dpi, ')');
 }
@@ -3554,8 +3551,8 @@ d_print_subexpr (struct d_print_info *dpi,
 /* Subroutine to handle components.  */
 
 static void
-d_print_comp (struct d_print_info *dpi,
-              const struct demangle_component *dc)
+d_print_comp (struct d_print_info *dpi, const struct demangle_component *dc,
+	      int options)
 {
   if (dc == NULL)
     {
@@ -3568,7 +3565,7 @@ d_print_comp (struct d_print_info *dpi,
   switch (dc->type)
     {
     case DEMANGLE_COMPONENT_NAME:
-      if ((dpi->options & DMGL_JAVA) == 0)
+      if ((options & DMGL_JAVA) == 0)
 	d_append_buffer (dpi, dc->u.s_name.s, dc->u.s_name.len);
       else
 	d_print_java_identifier (dpi, dc->u.s_name.s, dc->u.s_name.len);
@@ -3576,12 +3573,12 @@ d_print_comp (struct d_print_info *dpi,
 
     case DEMANGLE_COMPONENT_QUAL_NAME:
     case DEMANGLE_COMPONENT_LOCAL_NAME:
-      d_print_comp (dpi, d_left (dc));
-      if ((dpi->options & DMGL_JAVA) == 0)
+      d_print_comp (dpi, d_left (dc), options);
+      if ((options & DMGL_JAVA) == 0)
 	d_append_string (dpi, "::");
       else
 	d_append_char (dpi, '.');
-      d_print_comp (dpi, d_right (dc));
+      d_print_comp (dpi, d_right (dc), options);
       return;
 
     case DEMANGLE_COMPONENT_TYPED_NAME:
@@ -3671,7 +3668,7 @@ d_print_comp (struct d_print_info *dpi,
 	      }
 	  }
 
-	d_print_comp (dpi, d_right (dc));
+	d_print_comp (dpi, d_right (dc), options);
 
 	if (typed_name->type == DEMANGLE_COMPONENT_TEMPLATE)
 	  dpi->templates = dpt.next;
@@ -3684,7 +3681,7 @@ d_print_comp (struct d_print_info *dpi,
 	    if (! adpm[i].printed)
 	      {
 		d_append_char (dpi, ' ');
-		d_print_mod (dpi, adpm[i].mod);
+		d_print_mod (dpi, adpm[i].mod, options);
 	      }
 	  }
 
@@ -3707,7 +3704,7 @@ d_print_comp (struct d_print_info *dpi,
 
         dcl = d_left (dc);
 
-        if ((dpi->options & DMGL_JAVA) != 0
+        if ((options & DMGL_JAVA) != 0
             && dcl->type == DEMANGLE_COMPONENT_NAME
             && dcl->u.s_name.len == 6
             && strncmp (dcl->u.s_name.s, "JArray", 6) == 0)
@@ -3715,16 +3712,16 @@ d_print_comp (struct d_print_info *dpi,
             /* Special-case Java arrays, so that JArray<TYPE> appears
                instead as TYPE[].  */
 
-            d_print_comp (dpi, d_right (dc));
+            d_print_comp (dpi, d_right (dc), options);
             d_append_string (dpi, "[]");
           }
         else
           {
-	    d_print_comp (dpi, dcl);
+	    d_print_comp (dpi, dcl, options);
 	    if (d_last_char (dpi) == '<')
 	      d_append_char (dpi, ' ');
 	    d_append_char (dpi, '<');
-	    d_print_comp (dpi, d_right (dc));
+	    d_print_comp (dpi, d_right (dc), options);
 	    /* Avoid generating two consecutive '>' characters, to avoid
 	       the C++ syntactic ambiguity.  */
 	    if (d_last_char (dpi) == '>')
@@ -3759,7 +3756,7 @@ d_print_comp (struct d_print_info *dpi,
 	hold_dpt = dpi->templates;
 	dpi->templates = hold_dpt->next;
 
-	d_print_comp (dpi, a);
+	d_print_comp (dpi, a, options);
 
 	dpi->templates = hold_dpt;
 
@@ -3767,79 +3764,79 @@ d_print_comp (struct d_print_info *dpi,
       }
 
     case DEMANGLE_COMPONENT_CTOR:
-      d_print_comp (dpi, dc->u.s_ctor.name);
+      d_print_comp (dpi, dc->u.s_ctor.name, options);
       return;
 
     case DEMANGLE_COMPONENT_DTOR:
       d_append_char (dpi, '~');
-      d_print_comp (dpi, dc->u.s_dtor.name);
+      d_print_comp (dpi, dc->u.s_dtor.name, options);
       return;
 
     case DEMANGLE_COMPONENT_VTABLE:
       d_append_string (dpi, "vtable for ");
-      d_print_comp (dpi, d_left (dc));
+      d_print_comp (dpi, d_left (dc), options);
       return;
 
     case DEMANGLE_COMPONENT_VTT:
       d_append_string (dpi, "VTT for ");
-      d_print_comp (dpi, d_left (dc));
+      d_print_comp (dpi, d_left (dc), options);
       return;
 
     case DEMANGLE_COMPONENT_CONSTRUCTION_VTABLE:
       d_append_string (dpi, "construction vtable for ");
-      d_print_comp (dpi, d_left (dc));
+      d_print_comp (dpi, d_left (dc), options);
       d_append_string (dpi, "-in-");
-      d_print_comp (dpi, d_right (dc));
+      d_print_comp (dpi, d_right (dc), options);
       return;
 
     case DEMANGLE_COMPONENT_TYPEINFO:
       d_append_string (dpi, "typeinfo for ");
-      d_print_comp (dpi, d_left (dc));
+      d_print_comp (dpi, d_left (dc), options);
       return;
 
     case DEMANGLE_COMPONENT_TYPEINFO_NAME:
       d_append_string (dpi, "typeinfo name for ");
-      d_print_comp (dpi, d_left (dc));
+      d_print_comp (dpi, d_left (dc), options);
       return;
 
     case DEMANGLE_COMPONENT_TYPEINFO_FN:
       d_append_string (dpi, "typeinfo fn for ");
-      d_print_comp (dpi, d_left (dc));
+      d_print_comp (dpi, d_left (dc), options);
       return;
 
     case DEMANGLE_COMPONENT_THUNK:
       d_append_string (dpi, "non-virtual thunk to ");
-      d_print_comp (dpi, d_left (dc));
+      d_print_comp (dpi, d_left (dc), options);
       return;
 
     case DEMANGLE_COMPONENT_VIRTUAL_THUNK:
       d_append_string (dpi, "virtual thunk to ");
-      d_print_comp (dpi, d_left (dc));
+      d_print_comp (dpi, d_left (dc), options);
       return;
 
     case DEMANGLE_COMPONENT_COVARIANT_THUNK:
       d_append_string (dpi, "covariant return thunk to ");
-      d_print_comp (dpi, d_left (dc));
+      d_print_comp (dpi, d_left (dc), options);
       return;
 
     case DEMANGLE_COMPONENT_JAVA_CLASS:
       d_append_string (dpi, "java Class for ");
-      d_print_comp (dpi, d_left (dc));
+      d_print_comp (dpi, d_left (dc), options);
       return;
 
     case DEMANGLE_COMPONENT_GUARD:
       d_append_string (dpi, "guard variable for ");
-      d_print_comp (dpi, d_left (dc));
+      d_print_comp (dpi, d_left (dc), options);
       return;
 
     case DEMANGLE_COMPONENT_REFTEMP:
       d_append_string (dpi, "reference temporary for ");
-      d_print_comp (dpi, d_left (dc));
+      d_print_comp (dpi, d_left (dc), options);
       return;
 
     case DEMANGLE_COMPONENT_HIDDEN_ALIAS:
       d_append_string (dpi, "hidden alias for ");
-      d_print_comp (dpi, d_left (dc));
+      d_print_comp (dpi, d_left (dc), options);
       return;
 
     case DEMANGLE_COMPONENT_SUB_STD:
@@ -3866,7 +3863,7 @@ d_print_comp (struct d_print_info *dpi,
 		  break;
 		if (pdpm->mod->type == dc->type)
 		  {
-		    d_print_comp (dpi, d_left (dc));
+		    d_print_comp (dpi, d_left (dc), options);
 		    return;
 		  }
 	      }
@@ -3892,12 +3889,12 @@ d_print_comp (struct d_print_info *dpi,
 	dpm.printed = 0;
 	dpm.templates = dpi->templates;
 
-	d_print_comp (dpi, d_left (dc));
+	d_print_comp (dpi, d_left (dc), options);
 
 	/* If the modifier didn't get printed by the type, print it
 	   now.  */
 	if (! dpm.printed)
-	  d_print_mod (dpi, dc);
+	  d_print_mod (dpi, dc, options);
 
 	dpi->modifiers = dpm.next;
 
@@ -3905,7 +3902,7 @@ d_print_comp (struct d_print_info *dpi,
       }
 
     case DEMANGLE_COMPONENT_BUILTIN_TYPE:
-      if ((dpi->options & DMGL_JAVA) == 0)
+      if ((options & DMGL_JAVA) == 0)
 	d_append_buffer (dpi, dc->u.s_builtin.type->name,
 			 dc->u.s_builtin.type->len);
       else
@@ -3914,13 +3911,13 @@ d_print_comp (struct d_print_info *dpi,
       return;
 
     case DEMANGLE_COMPONENT_VENDOR_TYPE:
-      d_print_comp (dpi, d_left (dc));
+      d_print_comp (dpi, d_left (dc), options);
       return;
 
     case DEMANGLE_COMPONENT_FUNCTION_TYPE:
       {
-	if ((dpi->options & DMGL_RET_POSTFIX) != 0)
-	  d_print_function_type (dpi, dc, dpi->modifiers);
+	if ((options & DMGL_RET_POSTFIX) != 0)
+	  d_print_function_type (dpi, dc, dpi->modifiers, options);
 
 	/* Print return type if present */
 	if (d_left (dc) != NULL)
@@ -3935,7 +3932,7 @@ d_print_comp (struct d_print_info *dpi,
 	    dpm.printed = 0;
 	    dpm.templates = dpi->templates;
 
-	    d_print_comp (dpi, d_left (dc));
+	    d_print_comp (dpi, d_left (dc), options);
 
 	    dpi->modifiers = dpm.next;
 
@@ -3944,12 +3941,12 @@ d_print_comp (struct d_print_info *dpi,
 
 	    /* In standard prefix notation, there is a space between the
 	       return type and the function signature.  */
-	    if ((dpi->options & DMGL_RET_POSTFIX) == 0)
+	    if ((options & DMGL_RET_POSTFIX) == 0)
 	      d_append_char (dpi, ' ');
 	  }
 
-	if ((dpi->options & DMGL_RET_POSTFIX) == 0) 
-	  d_print_function_type (dpi, dc, dpi->modifiers);
+	if ((options & DMGL_RET_POSTFIX) == 0)
+	  d_print_function_type (dpi, dc, dpi->modifiers, options);
 
 	return;
       }
@@ -4002,7 +3999,7 @@ d_print_comp (struct d_print_info *dpi,
 	    pdpm = pdpm->next;
 	  }
 
-	d_print_comp (dpi, d_right (dc));
+	d_print_comp (dpi, d_right (dc), options);
 
 	dpi->modifiers = hold_modifiers;
 
@@ -4012,10 +4009,10 @@ d_print_comp (struct d_print_info *dpi,
 	while (i > 1)
 	  {
 	    --i;
-	    d_print_mod (dpi, adpm[i].mod);
+	    d_print_mod (dpi, adpm[i].mod, options);
 	  }
 
-	d_print_array_type (dpi, dc, dpi->modifiers);
+	d_print_array_type (dpi, dc, dpi->modifiers, options);
 
 	return;
       }
@@ -4031,12 +4028,12 @@ d_print_comp (struct d_print_info *dpi,
 	dpm.printed = 0;
 	dpm.templates = dpi->templates;
 
-	d_print_comp (dpi, d_right (dc));
+	d_print_comp (dpi, d_right (dc), options);
 
 	/* If the modifier didn't get printed by the type, print it
 	   now.  */
 	if (! dpm.printed)
-	  d_print_mod (dpi, dc);
+	  d_print_mod (dpi, dc, options);
 
 	dpi->modifiers = dpm.next;
 
@@ -4050,7 +4047,7 @@ d_print_comp (struct d_print_info *dpi,
       if (dc->u.s_fixed.length->u.s_builtin.type
 	  != &cplus_demangle_builtin_types['i'-'a'])
 	{
-	  d_print_comp (dpi, dc->u.s_fixed.length);
+	  d_print_comp (dpi, dc->u.s_fixed.length, options);
 	  d_append_char (dpi, ' ');
 	}
       if (dc->u.s_fixed.accum)
@@ -4062,7 +4059,7 @@ d_print_comp (struct d_print_info *dpi,
     case DEMANGLE_COMPONENT_ARGLIST:
     case DEMANGLE_COMPONENT_TEMPLATE_ARGLIST:
       if (d_left (dc) != NULL)
-	d_print_comp (dpi, d_left (dc));
+	d_print_comp (dpi, d_left (dc), options);
       if (d_right (dc) != NULL)
 	{
 	  size_t len;
@@ -4074,7 +4071,7 @@ d_print_comp (struct d_print_info *dpi,
 	  d_append_string (dpi, ", ");
 	  len = dpi->len;
 	  flush_count = dpi->flush_count;
-	  d_print_comp (dpi, d_right (dc));
+	  d_print_comp (dpi, d_right (dc), options);
 	  /* If that didn't print anything (which can happen with empty
 	     template argument packs), remove the comma and space.  */
 	  if (dpi->flush_count == flush_count && dpi->len == len)
@@ -4097,24 +4094,24 @@ d_print_comp (struct d_print_info *dpi,
 
     case DEMANGLE_COMPONENT_EXTENDED_OPERATOR:
       d_append_string (dpi, "operator ");
-      d_print_comp (dpi, dc->u.s_extended_operator.name);
+      d_print_comp (dpi, dc->u.s_extended_operator.name, options);
       return;
 
     case DEMANGLE_COMPONENT_CAST:
       d_append_string (dpi, "operator ");
-      d_print_cast (dpi, dc);
+      d_print_cast (dpi, dc, options);
       return;
 
     case DEMANGLE_COMPONENT_UNARY:
       if (d_left (dc)->type != DEMANGLE_COMPONENT_CAST)
-	d_print_expr_op (dpi, d_left (dc));
+	d_print_expr_op (dpi, d_left (dc), options);
       else
 	{
 	  d_append_char (dpi, '(');
-	  d_print_cast (dpi, d_left (dc));
+	  d_print_cast (dpi, d_left (dc), options);
 	  d_append_char (dpi, ')');
 	}
-      d_print_subexpr (dpi, d_right (dc));
+      d_print_subexpr (dpi, d_right (dc), options);
       return;
 
     case DEMANGLE_COMPONENT_BINARY:
@@ -4132,18 +4129,18 @@ d_print_comp (struct d_print_info *dpi,
 	  && d_left (dc)->u.s_operator.op->name[0] == '>')
 	d_append_char (dpi, '(');
 
-      d_print_subexpr (dpi, d_left (d_right (dc)));
+      d_print_subexpr (dpi, d_left (d_right (dc)), options);
       if (strcmp (d_left (dc)->u.s_operator.op->code, "ix") == 0)
 	{
 	  d_append_char (dpi, '[');
-	  d_print_comp (dpi, d_right (d_right (dc)));
+	  d_print_comp (dpi, d_right (d_right (dc)), options);
 	  d_append_char (dpi, ']');
 	}
       else
 	{
 	  if (strcmp (d_left (dc)->u.s_operator.op->code, "cl") != 0)
-	    d_print_expr_op (dpi, d_left (dc));
-	  d_print_subexpr (dpi, d_right (d_right (dc)));
+	    d_print_expr_op (dpi, d_left (dc), options);
+	  d_print_subexpr (dpi, d_right (d_right (dc)), options);
 	}
 
       if (d_left (dc)->type == DEMANGLE_COMPONENT_OPERATOR
@@ -4165,11 +4162,11 @@ d_print_comp (struct d_print_info *dpi,
 	  d_print_error (dpi);
 	  return;
 	}
-      d_print_subexpr (dpi, d_left (d_right (dc)));
-      d_print_expr_op (dpi, d_left (dc));
-      d_print_subexpr (dpi, d_left (d_right (d_right (dc))));
+      d_print_subexpr (dpi, d_left (d_right (dc)), options);
+      d_print_expr_op (dpi, d_left (dc), options);
+      d_print_subexpr (dpi, d_left (d_right (d_right (dc))), options);
       d_append_string (dpi, " : ");
-      d_print_subexpr (dpi, d_right (d_right (d_right (dc))));
+      d_print_subexpr (dpi, d_right (d_right (d_right (dc))), options);
       return;
 
     case DEMANGLE_COMPONENT_TRINARY_ARG1:
@@ -4200,7 +4197,7 @@ d_print_comp (struct d_print_info *dpi,
 		  {
 		    if (dc->type == DEMANGLE_COMPONENT_LITERAL_NEG)
 		      d_append_char (dpi, '-');
-		    d_print_comp (dpi, d_right (dc));
+		    d_print_comp (dpi, d_right (dc), options);
 		    switch (tp)
 		      {
 		      default:
@@ -4250,13 +4247,13 @@ d_print_comp (struct d_print_info *dpi,
 	  }
 
 	d_append_char (dpi, '(');
-	d_print_comp (dpi, d_left (dc));
+	d_print_comp (dpi, d_left (dc), options);
 	d_append_char (dpi, ')');
 	if (dc->type == DEMANGLE_COMPONENT_LITERAL_NEG)
 	  d_append_char (dpi, '-');
 	if (tp == D_PRINT_FLOAT)
 	  d_append_char (dpi, '[');
-	d_print_comp (dpi, d_right (dc));
+	d_print_comp (dpi, d_right (dc), options);
 	if (tp == D_PRINT_FLOAT)
 	  d_append_char (dpi, ']');
       }
@@ -4268,12 +4265,12 @@ d_print_comp (struct d_print_info *dpi,
 
     case DEMANGLE_COMPONENT_JAVA_RESOURCE:
       d_append_string (dpi, "java resource ");
-      d_print_comp (dpi, d_left (dc));
+      d_print_comp (dpi, d_left (dc), options);
       return;
 
     case DEMANGLE_COMPONENT_COMPOUND_NAME:
-      d_print_comp (dpi, d_left (dc));
-      d_print_comp (dpi, d_right (dc));
+      d_print_comp (dpi, d_left (dc), options);
+      d_print_comp (dpi, d_right (dc), options);
       return;
 
     case DEMANGLE_COMPONENT_CHARACTER:
@@ -4282,7 +4279,7 @@ d_print_comp (struct d_print_info *dpi,
 
     case DEMANGLE_COMPONENT_DECLTYPE:
       d_append_string (dpi, "decltype (");
-      d_print_comp (dpi, d_left (dc));
+      d_print_comp (dpi, d_left (dc), options);
       d_append_char (dpi, ')');
       return;
 
@@ -4296,7 +4293,7 @@ d_print_comp (struct d_print_info *dpi,
 	    /* d_find_pack won't find anything if the only packs involved
 	       in this expansion are function parameter packs; in that
 	       case, just print the pattern and "...".  */
-	    d_print_subexpr (dpi, d_left (dc));
+	    d_print_subexpr (dpi, d_left (dc), options);
 	    d_append_string (dpi, "...");
 	    return;
 	  }
@@ -4306,7 +4303,7 @@ d_print_comp (struct d_print_info *dpi,
 	for (i = 0; i < len; ++i)
 	  {
 	    dpi->pack_index = i;
-	    d_print_comp (dpi, dc);
+	    d_print_comp (dpi, dc, options);
 	    if (i < len-1)
 	      d_append_string (dpi, ", ");
 	  }
@@ -4321,17 +4318,17 @@ d_print_comp (struct d_print_info *dpi,
 
     case DEMANGLE_COMPONENT_GLOBAL_CONSTRUCTORS:
       d_append_string (dpi, "global constructors keyed to ");
-      d_print_comp (dpi, dc->u.s_binary.left);
+      d_print_comp (dpi, dc->u.s_binary.left, options);
       return;
 
     case DEMANGLE_COMPONENT_GLOBAL_DESTRUCTORS:
       d_append_string (dpi, "global destructors keyed to ");
-      d_print_comp (dpi, dc->u.s_binary.left);
+      d_print_comp (dpi, dc->u.s_binary.left, options);
       return;
 
     case DEMANGLE_COMPONENT_LAMBDA:
       d_append_string (dpi, "{lambda(");
-      d_print_comp (dpi, dc->u.s_unary_num.sub);
+      d_print_comp (dpi, dc->u.s_unary_num.sub, options);
       d_append_string (dpi, ")#");
       d_append_num (dpi, dc->u.s_unary_num.num + 1);
       d_append_char (dpi, '}');
@@ -4405,8 +4402,8 @@ d_print_java_identifier (struct d_print_info *dpi, const char *name, int len)
    qualifiers on this after printing a function.  */
 
 static void
-d_print_mod_list (struct d_print_info *dpi,
-                  struct d_print_mod *mods, int suffix)
+d_print_mod_list (struct d_print_info *dpi, struct d_print_mod *mods,
+		  int suffix, int options)
 {
   struct d_print_template *hold_dpt;
 
@@ -4419,7 +4416,7 @@ d_print_mod_list (struct d_print_info *dpi,
 	      || mods->mod->type == DEMANGLE_COMPONENT_VOLATILE_THIS
 	      || mods->mod->type == DEMANGLE_COMPONENT_CONST_THIS)))
     {
-      d_print_mod_list (dpi, mods->next, suffix);
+      d_print_mod_list (dpi, mods->next, suffix, options);
       return;
     }
 
@@ -4430,13 +4427,13 @@ d_print_mod_list (struct d_print_info *dpi,
 
   if (mods->mod->type == DEMANGLE_COMPONENT_FUNCTION_TYPE)
     {
-      d_print_function_type (dpi, mods->mod, mods->next);
+      d_print_function_type (dpi, mods->mod, mods->next, options);
       dpi->templates = hold_dpt;
       return;
     }
   else if (mods->mod->type == DEMANGLE_COMPONENT_ARRAY_TYPE)
     {
-      d_print_array_type (dpi, mods->mod, mods->next);
+      d_print_array_type (dpi, mods->mod, mods->next, options);
       dpi->templates = hold_dpt;
       return;
     }
@@ -4452,10 +4449,10 @@ d_print_mod_list (struct d_print_info *dpi,
 
       hold_modifiers = dpi->modifiers;
       dpi->modifiers = NULL;
-      d_print_comp (dpi, d_left (mods->mod));
+      d_print_comp (dpi, d_left (mods->mod), options);
       dpi->modifiers = hold_modifiers;
 
-      if ((dpi->options & DMGL_JAVA) == 0)
+      if ((options & DMGL_JAVA) == 0)
 	d_append_string (dpi, "::");
       else
 	d_append_char (dpi, '.');
@@ -4475,24 +4472,24 @@ d_print_mod_list (struct d_print_info *dpi,
 	     || dc->type == DEMANGLE_COMPONENT_CONST_THIS)
 	dc = d_left (dc);
 
-      d_print_comp (dpi, dc);
+      d_print_comp (dpi, dc, options);
 
       dpi->templates = hold_dpt;
       return;
     }
 
-  d_print_mod (dpi, mods->mod);
+  d_print_mod (dpi, mods->mod, options);
 
   dpi->templates = hold_dpt;
 
-  d_print_mod_list (dpi, mods->next, suffix);
+  d_print_mod_list (dpi, mods->next, suffix, options);
 }
 
 /* Print a modifier.  */
 
 static void
-d_print_mod (struct d_print_info *dpi,
-             const struct demangle_component *mod)
+d_print_mod (struct d_print_info *dpi, const struct demangle_component *mod,
+	     int options)
 {
   switch (mod->type)
     {
@@ -4510,11 +4507,11 @@ d_print_mod (struct d_print_info *dpi,
       return;
     case DEMANGLE_COMPONENT_VENDOR_TYPE_QUAL:
       d_append_char (dpi, ' ');
-      d_print_comp (dpi, d_right (mod));
+      d_print_comp (dpi, d_right (mod), options);
       return;
     case DEMANGLE_COMPONENT_POINTER:
       /* There is no pointer symbol in Java.  */
-      if ((dpi->options & DMGL_JAVA) == 0)
+      if ((options & DMGL_JAVA) == 0)
 	d_append_char (dpi, '*');
       return;
     case DEMANGLE_COMPONENT_REFERENCE:
@@ -4532,22 +4529,22 @@ d_print_mod (struct d_print_info *dpi,
     case DEMANGLE_COMPONENT_PTRMEM_TYPE:
       if (d_last_char (dpi) != '(')
 	d_append_char (dpi, ' ');
-      d_print_comp (dpi, d_left (mod));
+      d_print_comp (dpi, d_left (mod), options);
       d_append_string (dpi, "::*");
       return;
     case DEMANGLE_COMPONENT_TYPED_NAME:
-      d_print_comp (dpi, d_left (mod));
+      d_print_comp (dpi, d_left (mod), options);
       return;
     case DEMANGLE_COMPONENT_VECTOR_TYPE:
       d_append_string (dpi, " __vector(");
-      d_print_comp (dpi, d_left (mod));
+      d_print_comp (dpi, d_left (mod), options);
       d_append_char (dpi, ')');
       return;
 
     default:
       /* Otherwise, we have something that won't go back on the
 	 modifier stack, so we can just print it.  */
-      d_print_comp (dpi, mod);
+      d_print_comp (dpi, mod, options);
       return;
     }
 }
@@ -4557,7 +4554,7 @@ d_print_mod (struct d_print_info *dpi,
 static void
 d_print_function_type (struct d_print_info *dpi,
                        const struct demangle_component *dc,
-                       struct d_print_mod *mods)
+                       struct d_print_mod *mods, int options)
 {
   int need_paren;
   int need_space;
@@ -4615,7 +4612,7 @@ d_print_function_type (struct d_print_info *dpi,
   hold_modifiers = dpi->modifiers;
   dpi->modifiers = NULL;
 
-  d_print_mod_list (dpi, mods, 0);
+  d_print_mod_list (dpi, mods, 0, options);
 
   if (need_paren)
     d_append_char (dpi, ')');
@@ -4623,11 +4620,11 @@ d_print_function_type (struct d_print_info *dpi,
   d_append_char (dpi, '(');
 
   if (d_right (dc) != NULL)
-    d_print_comp (dpi, d_right (dc));
+    d_print_comp (dpi, d_right (dc), options);
 
   d_append_char (dpi, ')');
 
-  d_print_mod_list (dpi, mods, 1);
+  d_print_mod_list (dpi, mods, 1, options);
 
   dpi->modifiers = hold_modifiers;
 }
@@ -4637,7 +4634,7 @@ d_print_function_type (struct d_print_info *dpi,
 static void
 d_print_array_type (struct d_print_info *dpi,
                     const struct demangle_component *dc,
-                    struct d_print_mod *mods)
+                    struct d_print_mod *mods, int options)
 {
   int need_space;
 
@@ -4669,7 +4666,7 @@ d_print_array_type (struct d_print_info *dpi,
       if (need_paren)
 	d_append_string (dpi, " (");
 
-      d_print_mod_list (dpi, mods, 0);
+      d_print_mod_list (dpi, mods, 0, options);
 
       if (need_paren)
 	d_append_char (dpi, ')');
@@ -4681,7 +4678,7 @@ d_print_array_type (struct d_print_info *dpi,
   d_append_char (dpi, '[');
 
   if (d_left (dc) != NULL)
-    d_print_comp (dpi, d_left (dc));
+    d_print_comp (dpi, d_left (dc), options);
 
   d_append_char (dpi, ']');
 }
@@ -4690,23 +4687,23 @@ d_print_array_type (struct d_print_info *dpi,
 
 static void
 d_print_expr_op (struct d_print_info *dpi,
-                 const struct demangle_component *dc)
+                 const struct demangle_component *dc, int options)
 {
   if (dc->type == DEMANGLE_COMPONENT_OPERATOR)
     d_append_buffer (dpi, dc->u.s_operator.op->name,
 		     dc->u.s_operator.op->len);
   else
-    d_print_comp (dpi, dc);
+    d_print_comp (dpi, dc, options);
 }
 
 /* Print a cast.  */
 
 static void
-d_print_cast (struct d_print_info *dpi,
-              const struct demangle_component *dc)
+d_print_cast (struct d_print_info *dpi, const struct demangle_component *dc,
+	      int options)
 {
   if (d_left (dc)->type != DEMANGLE_COMPONENT_TEMPLATE)
-    d_print_comp (dpi, d_left (dc));
+    d_print_comp (dpi, d_left (dc), options);
   else
     {
       struct d_print_mod *hold_dpm;
@@ -4724,14 +4721,14 @@ d_print_cast (struct d_print_info *dpi,
       dpi->templates = &dpt;
       dpt.template_decl = d_left (dc);
 
-      d_print_comp (dpi, d_left (d_left (dc)));
+      d_print_comp (dpi, d_left (d_left (dc)), options);
 
       dpi->templates = dpt.next;
 
       if (d_last_char (dpi) == '<')
 	d_append_char (dpi, ' ');
       d_append_char (dpi, '<');
-      d_print_comp (dpi, d_right (d_left (dc)));
+      d_print_comp (dpi, d_right (d_left (dc)), options);
       /* Avoid generating two consecutive '>' characters, to avoid
 	 the C++ syntactic ambiguity.  */
       if (d_last_char (dpi) == '>')

[rfc 2/2] Follow DW_AT_linkage_name for methods [Re: The future of dwarf2_physname]

[Jan Kratochvil]

On Mon, 23 May 2011 15:16:59 +0200, Jan Kratochvil wrote:
> problem #1 of post-physname GDB
> 
> (gdb) p 'f<char>()' 
> $1 = {long (void)} 0x4004e4 <f<char>()>
> (gdb) p 'long f<char>()' 
> $2 = {<text variable, no debug info>} 0x4004e4 <f<char>()>
> (gdb) p 'f<short>()' 
> No symbol "f<short>()" in current context.
> (gdb) p 'long f<short>()' 
> $3 = {<text variable, no debug info>} 0x4004fb <long f<short>()>
> 
> ==> dis.h <==
> template <typename T>
> long f () { return 1; }
> ==> dis1.C <==
> #include "dis.h"
> int main () { f<char> (); }
> ==> dis2.C <==
> #include "dis.h"
> void g () { f<short> (); }
> 
> The two different aliases for 0x4004e4 where the $2 one does not have any type
> is a bug, at least both should have the same type.  It can be seen also on the
> f<short>() case ($3) where the return type missing alias is not present because
> only ELF (and not DWARF) symbol is present.

So far I have kept this bug of missing type as GDB currently cannot provide
multiple search names for a single symbol.  And IMO there should exist both
`long f<char>()' as it is its demangled ELF (minimal symbol) name and also the
`f<char>()' as otherwise user will never find such template function by
<tab>-completion from CLI.


> I haven't found how to reproduce the regression
> "Cannot take address of method m." for function templates (which use the
> return type in their linkage) as I haven't found how to reference function
> template externally from a class - this field is missing there in such case:

Therefore I do not find this double-entry for template function such a real
problem, just the return type featured name is missing its real type.


> problem #2 of post-physname GDB
> 
> (gdb) p 'int f<long>()' 
> $1 = {<text variable, no debug info>} 0x40050f <f<long>()>
> (gdb) p 'short f<long>()' 
> $2 = {<text variable, no debug info>} 0x400531 <f<long>()>
> (gdb) p 'f<long>()' 
> $3 = {int (void)} 0x40050f <f<long>()>
> 
> ==> manya.C <==
> template <typename T>
> int f () { return 4; }
> int a () { return f<long> () == 4; }
> extern int b ();
> int main () { return a () && b () ? 0 : 1; }
> ==> manyb.C <==
> template <typename T>
> short f () { return 2; }
> int b () { return f<long> () == 2; }
> 
> As the return type is stripped the name can become ambiguous.  Different
> template functions with same name, same parameter types but different return
> type can be linked in a single program.  It is true one cannot declare both
> such template functions in a single CU (Compilation Unit) so I do not expect
> such case is found in real world.

I do not think such symbols exist in real world cases - when they are
incompatible in a single CU anyway.  Therefore the ambiguity is not a real
world problem.


> I was trying to propose DMGL_RET_POSTFIX but it has other drawbacks

Also after a chat with Tom Tromey I no longer use DMGL_RET_POSTFIX.  That is
for ELF symbol `long m<char>()' GDB with proposed patch uses `m<char>()'
(and not `m<char>()long').  This is compatible with gdb-7.2 (physname)
template functions.


But I have found GDB requires full DWARF symbols for methods anyway so some of
my examples could be a bit unreal, I have to reconsider.

Anyway DW_AT_linkage_name following patch with no regressions on
{x86_64,x86_64-m32,i686}-fedora15-linux-gnu is attached.  It would need GCC
push of the libiberty/ change.


Thanks,
Jan


gdb/
2011-05-24  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Tom Tromey  <tromey@redhat.com>

	* dwarf2read.c (check_physname): New variable.
	(dwarf2_physname): Prefer DW_AT_linkage_name over dwarf2_compute_name.
	(show_check_physname): New function.
	(_initialize_dwarf2_read): Add `check-physname' for check_physname.

gdb/testsuite/
2011-05-24  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/break-interp.exp (reach_1, test_ld): Allow also the prefix
	__GI_.
	* gdb.cp/psymtab-parameter.cc (func): Make it a template function.
	(f): New function.
	* gdb.cp/psymtab-parameter.exp (complete break 'func(): Rename to ...
	(complete p 'func<short>(): ... here.
	* gdb.dwarf2/dw2-linkage-name-trust-main.cc: New file.
	* gdb.dwarf2/dw2-linkage-name-trust.S: New file.
	* gdb.dwarf2/dw2-linkage-name-trust.exp: New file.

include/
2011-05-24  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* demangle.h (DMGL_RET_POSTFIX): Extend the comment.
	(DMGL_RET_DROP): New.
	* cp-demangle.c (d_print_comp) <DEMANGLE_COMPONENT_FUNCTION_TYPE>: Do
	not pass DMGL_RET_POSTFIX or DMGL_RET_DROP.  Suppress d_print_mod for
	DMGL_RET_POSTFIX.  Support DMGL_RET_DROP.
	* testsuite/demangle-expected: New testcases for --ret-postfix and
	--ret-drop.
	* testsuite/test-demangle.c: Document --ret-drop in a comment.
	(main): New variable ret_drop, fill it, call cplus_demangle with it.

--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -119,6 +119,9 @@ _STATEMENT_PROLOGUE;
 /* When non-zero, dump DIEs after they are read in.  */
 static int dwarf2_die_debug = 0;
 
+/* When non-zero, cross-check physname against demangler.  */
+static int check_physname = 0;
+
 static int pagesize;
 
 /* When set, the file that we're processing is known to have debugging
@@ -5147,7 +5150,93 @@ dwarf2_full_name (char *name, struct die_info *die, struct dwarf2_cu *cu)
 static const char *
 dwarf2_physname (char *name, struct die_info *die, struct dwarf2_cu *cu)
 {
-  return dwarf2_compute_name (name, die, cu, 1);
+  struct attribute *attr;
+  const char *retval, *mangled = NULL;
+  char *canon = NULL;
+  struct cleanup *back_to;
+  int need_copy = 1;
+
+  /* In this case dwarf2_compute_name is just a shortcut not building anything
+     on its own.  */
+  if (!die_needs_namespace (die, cu))
+    return dwarf2_compute_name (name, die, cu, 1);
+
+  back_to = make_cleanup (null_cleanup, NULL);
+
+  attr = dwarf2_attr (die, DW_AT_linkage_name, cu);
+  if (!attr)
+    attr = dwarf2_attr (die, DW_AT_MIPS_linkage_name, cu);
+
+  /* DW_AT_linkage_name is missing in some cases - depend on what GDB
+     has computed.  */
+  if (attr && DW_STRING (attr))
+    {
+      char *demangled;
+
+      mangled = DW_STRING (attr);
+
+      /* Use DMGL_RET_DROP for C++ template functions to suppress their return
+	 type.  It is easier for GDB users to search for such functions as
+	 `name(params)' than `long name(params)'.  In such case the minimal
+	 symbol names do not match the full symbol names but for template
+	 functions there is never a need to look up their definition from their
+	 declaration so the only disadvantage remains the minimal symbol
+	 variant `long name(params)' does not have the proper inferior type.
+	 */
+
+      demangled = cplus_demangle (mangled, (DMGL_PARAMS | DMGL_ANSI
+					    | DMGL_VERBOSE
+					    | (cu->language == language_java
+					       ? DMGL_JAVA | DMGL_RET_POSTFIX
+					       : DMGL_RET_DROP)));
+      if (demangled)
+	{
+	  make_cleanup (xfree, demangled);
+	  canon = demangled;
+	}
+      else
+	{
+	  canon = (char *) mangled;
+	  need_copy = 0;
+	}
+    }
+
+  if (canon == NULL || check_physname)
+    {
+      const char *physname = dwarf2_compute_name (name, die, cu, 1);
+
+      if (canon != NULL && strcmp (physname, canon) != 0)
+	{
+	  /* It may not mean a bug in GDB.  The compiler could also
+	     compute DW_AT_linkage_name incorrectly.  But in such case
+	     GDB would need to be bug-to-bug compatible.  */
+
+	  complaint (&symfile_complaints,
+		     _("Computed physname <%s> does not match demangled <%s> "
+		       "(from linkage <%s>) - DIE at 0x%x [in module %s]"),
+		     physname, canon, mangled, die->offset, cu->objfile->name);
+
+	  /* Prefer DW_AT_linkage_name (in the CANON form) - when it
+	     is available here - over computed PHYSNAME.  It is safer
+	     against both buggy GDB and buggy compilers.  */
+
+	  retval = canon;
+	}
+      else
+	{
+	  retval = physname;
+	  need_copy = 0;
+	}
+    }
+  else
+    retval = canon;
+
+  if (need_copy)
+    retval = obsavestring (retval, strlen (retval),
+			   &cu->objfile->objfile_obstack);
+
+  do_cleanups (back_to);
+  return retval;
 }
 
 /* Read the import statement specified by the given die and record it.  */
@@ -16165,6 +16254,15 @@ show_dwarf2_always_disassemble (struct ui_file *file, int from_tty,
 		    value);
 }
 
+static void
+show_check_physname (struct ui_file *file, int from_tty,
+		     struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file,
+		    _("Whether to check \"physname\" is %s.\n"),
+		    value);
+}
+
 void _initialize_dwarf2_read (void);
 
 void
@@ -16220,6 +16318,14 @@ The value is the maximum depth to print."),
 			    NULL,
 			    &setdebuglist, &showdebuglist);
 
+  add_setshow_boolean_cmd ("check-physname", no_class, &check_physname, _("\
+Set cross-checking of \"physname\" code against demangler."), _("\
+Show cross-checking of \"physname\" code against demangler."), _("\
+When enabled, GDB's internal \"physname\" code is checked against\n\
+the demangler."),
+			   NULL, show_check_physname,
+			   &setdebuglist, &showdebuglist);
+
   c = add_cmd ("gdb-index", class_files, save_gdb_index_command,
 	       _("\
 Save a gdb-index file.\n\
--- a/gdb/testsuite/gdb.base/break-interp.exp
+++ b/gdb/testsuite/gdb.base/break-interp.exp
@@ -140,14 +140,14 @@ proc reach_1 {func command displacement} {
 	    }
 	    exp_continue
 	}
-	-re "Breakpoint \[0-9\]+, \\.?$func \\(.*\\) at .*:\[0-9\]+\r\n.*$gdb_prompt $" {
+	-re "Breakpoint \[0-9\]+, \\.?(__GI_)?$func \\(.*\\) at .*:\[0-9\]+\r\n.*$gdb_prompt $" {
 	    if {$func == "_dl_debug_state"} {
 		fail $test
 	    } else {
 		pass $test
 	    }
 	}
-	-re "Breakpoint \[0-9\]+, \[0-9xa-f\]+ in \\.?$func \\(\\).*\r\n$gdb_prompt $" {
+	-re "Breakpoint \[0-9\]+, \[0-9xa-f\]+ in \\.?(__GI_)?$func \\(\\).*\r\n$gdb_prompt $" {
 	    if {$func == "_dl_debug_state"} {
 		fail $test
 	    } else {
@@ -403,7 +403,7 @@ proc test_ld {file ifmain trynosym displacement} {
 
     reach "_dl_debug_state" "run" $displacement
 
-    gdb_test "bt" "#0 +\[^\r\n\]*\\m_dl_debug_state\\M.*" "dl bt"
+    gdb_test "bt" "#0 +\[^\r\n\]*\\m(__GI_)?_dl_debug_state\\M.*" "dl bt"
 
     if $ifmain {
 	reach "main" continue "NONE"
--- a/gdb/testsuite/gdb.cp/psymtab-parameter.cc
+++ b/gdb/testsuite/gdb.cp/psymtab-parameter.cc
@@ -16,7 +16,14 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.
    */
 
-typedef int typedefed;
-void func (typedefed param)
+template <typename T>
+long
+func ()
 {
 }
+
+void
+f ()
+{
+  func<short> ();
+}
--- a/gdb/testsuite/gdb.cp/psymtab-parameter.exp
+++ b/gdb/testsuite/gdb.cp/psymtab-parameter.exp
@@ -35,5 +35,6 @@ gdb_test_no_output "set language c++"
 # XFAIL than FAIL here.  For example -readnow breaks it.
 gdb_test_no_output "maintenance info symtabs"
 
-# GDB has shown only the `func(int)' entry before.
-gdb_test "complete break 'func(" "break 'func\\(int\\)\r\nbreak 'func\\(typedefed\\)"
+# GDB has shown only the `long func<short>()' ELF symbol before, not the DWARF
+# symbol
+gdb_test "complete p 'func<short>(" "p 'func<short>\\(\\)"
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-linkage-name-trust-main.cc
@@ -0,0 +1,41 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* class C
+   {
+   public:
+     static int f ();
+   };  */
+
+asm (".globl cu_text_start");
+asm ("cu_text_start:");
+
+int
+f (void)
+{
+  return 31173;
+}
+
+int
+main (void)
+{
+  f ();
+  return 0;
+}
+
+asm (".globl cu_text_end");
+asm ("cu_text_end:");
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-linkage-name-trust.S
@@ -0,0 +1,134 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Debug information */
+
+	.data
+	.globl	c
+c:	.4byte	0
+
+	.section .debug_info
+.Lcu1_begin:
+	/* CU header */
+	.4byte	.Lcu1_end - .Lcu1_start		/* Length of Compilation Unit */
+.Lcu1_start:
+	.2byte	2				/* DWARF Version */
+	.4byte	.Labbrev1_begin			/* Offset into abbrev section */
+	.byte	4				/* Pointer size */
+
+	/* CU die */
+	.uleb128 1				/* Abbrev: DW_TAG_compile_unit */
+	.4byte	cu_text_start			/* DW_AT_low_pc */
+	.4byte	cu_text_end			/* DW_AT_high_pc */
+	.ascii	"file1.txt\0"			/* DW_AT_name */
+	.ascii	"GNU C 3.3.3\0"			/* DW_AT_producer */
+	.byte	4				/* DW_AT_language (DW_LANG_C_plus_plus) */
+
+.Ltype_class:
+	.uleb128	3			/* Abbrev: DW_TAG_class_type */
+	.ascii		"C\0"			/* DW_AT_name */
+
+	.uleb128	5			/* Abbrev: DW_TAG_subprogram */
+	.byte		1			/* DW_AT_external */
+	.ascii		"membername\0"		/* DW_AT_name */
+	.ascii		"f\0"			/* DW_AT_MIPS_linkage_name */
+	.4byte		.Ltype_int-.Lcu1_begin	/* DW_AT_type */
+
+	.byte		0			/* End of children of DW_TAG_class_type */
+
+	.uleb128	4			/* Abbrev: DW_TAG_variable */
+	.ascii		"c\0"			/* DW_AT_name */
+	.4byte		.Ltype_class-.Lcu1_begin /* DW_AT_type */
+	.byte		1			/* DW_AT_external */
+
+.Ltype_int:
+	.uleb128	2			/* Abbrev: DW_TAG_base_type */
+	.ascii		"int\0"			/* DW_AT_name */
+	.byte		4			/* DW_AT_byte_size */
+	.byte		5			/* DW_AT_encoding */
+
+	.byte		0			/* End of children of CU */
+
+.Lcu1_end:
+
+/* Abbrev table */
+	.section .debug_abbrev
+.Labbrev1_begin:
+	.uleb128	1			/* Abbrev code */
+	.uleb128	0x11			/* DW_TAG_compile_unit */
+	.byte		1			/* has_children */
+	.uleb128	0x11			/* DW_AT_low_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x25			/* DW_AT_producer */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x13			/* DW_AT_language */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	2			/* Abbrev code */
+	.uleb128	0x24			/* DW_TAG_base_type */
+	.byte		0			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0xb			/* DW_AT_byte_size */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x3e			/* DW_AT_encoding */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	3			/* Abbrev code */
+	.uleb128	0x2			/* DW_TAG_class_type */
+	.byte		1			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	4			/* Abbrev code */
+	.uleb128	0x34			/* DW_TAG_variable */
+	.byte		0			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x49			/* DW_AT_type */
+	.uleb128	0x13			/* DW_FORM_ref4 */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	5			/* Abbrev code */
+	.uleb128	0x2e			/* DW_TAG_subprogram */
+	.byte		0			/* has_children */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x2007			/* DW_AT_MIPS_linkage_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x49			/* DW_AT_type */
+	.uleb128	0x13			/* DW_FORM_ref4 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-linkage-name-trust.exp
@@ -0,0 +1,55 @@
+# Copyright 2011 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that GDB can call C++ functions whose parameters or return values have
+# type containing a static member of the same type.
+
+# Still no C++ compiler is used.
+if { [skip_cplus_tests] } { continue }
+
+load_lib dwarf.exp
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0  
+}
+
+set testfile "dw2-linkage-name-trust"
+set executable ${testfile}
+set binfile ${objdir}/${subdir}/${executable}
+if { [gdb_compile ${srcdir}/${subdir}/${testfile}-main.cc "${objdir}/${subdir}/${testfile}-main.o" object {c++ debug}] != ""
+     || [gdb_compile "${srcdir}/${subdir}/${testfile}.S" "${objdir}/${subdir}/${testfile}.o" object {}] != ""
+     || [gdb_compile "${objdir}/${subdir}/${testfile}-main.o ${objdir}/${subdir}/${testfile}.o" "${binfile}" executable {c++}] != "" } {
+    return -1
+}
+
+clean_restart $executable
+
+if ![runto_main] then {
+    return -1
+}
+
+# main is not provided by DWARF.
+gdb_test_no_output "set language c++"
+
+# There are no mangled names in DWARF to suggest the v3 ABI.
+gdb_test_no_output "set cp-abi gnu-v3"
+
+# GDB cannot resolve external member function for which only ELF (and not
+# DWARF) symbol is available.  Therefore the function `f' must have DWARF which
+# confuses it a bit.
+
+gdb_test "p c.membername" " = {.*} 0x\[0-9a-f\]+ <f\\(\\)>"
+gdb_breakpoint "C::membername"
+gdb_test "p c.membername ()" "\r\nBreakpoint \[0-9\]+, .*"
--- a/include/demangle.h
+++ b/include/demangle.h
@@ -45,7 +45,12 @@ extern "C" {
 #define DMGL_VERBOSE	 (1 << 3)	/* Include implementation details.  */
 #define DMGL_TYPES	 (1 << 4)	/* Also try to demangle type encodings.  */
 #define DMGL_RET_POSTFIX (1 << 5)       /* Print function return types (when
-                                           present) after function signature */
+					   present) after function signature.
+					   It applies only for the toplevel
+					   function type.  */
+#define DMGL_RET_DROP	 (1 << 6)       /* Suppress function return types, even
+					   if present.  It applies only for the
+					   toplevel function type.  */
 
 #define DMGL_AUTO	 (1 << 8)
 #define DMGL_GNU	 (1 << 9)
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -3917,10 +3917,14 @@ d_print_comp (struct d_print_info *dpi, const struct demangle_component *dc,
     case DEMANGLE_COMPONENT_FUNCTION_TYPE:
       {
 	if ((options & DMGL_RET_POSTFIX) != 0)
-	  d_print_function_type (dpi, dc, dpi->modifiers, options);
+	  d_print_function_type (dpi, dc, dpi->modifiers,
+				 options & ~(DMGL_RET_POSTFIX | DMGL_RET_DROP));
 
 	/* Print return type if present */
-	if (d_left (dc) != NULL)
+	if (d_left (dc) != NULL && (options & DMGL_RET_POSTFIX) != 0)
+	  d_print_comp (dpi, d_left (dc),
+			options & ~(DMGL_RET_POSTFIX | DMGL_RET_DROP));
+	else if (d_left (dc) != NULL && (options & DMGL_RET_DROP) == 0)
 	  {
 	    struct d_print_mod dpm;
 
@@ -3932,7 +3936,8 @@ d_print_comp (struct d_print_info *dpi, const struct demangle_component *dc,
 	    dpm.printed = 0;
 	    dpm.templates = dpi->templates;
 
-	    d_print_comp (dpi, d_left (dc), options);
+	    d_print_comp (dpi, d_left (dc),
+			  options & ~(DMGL_RET_POSTFIX | DMGL_RET_DROP));
 
 	    dpi->modifiers = dpm.next;
 
@@ -3946,7 +3951,8 @@ d_print_comp (struct d_print_info *dpi, const struct demangle_component *dc,
 	  }
 
 	if ((options & DMGL_RET_POSTFIX) == 0)
-	  d_print_function_type (dpi, dc, dpi->modifiers, options);
+	  d_print_function_type (dpi, dc, dpi->modifiers,
+				 options & ~(DMGL_RET_POSTFIX | DMGL_RET_DROP));
 
 	return;
       }
--- a/libiberty/testsuite/demangle-expected
+++ b/libiberty/testsuite/demangle-expected
@@ -3959,6 +3959,33 @@ f(decltype(nullptr))
 --format=gnu-v3
 _ZN5aaaaa6bbbbbb5cccccIN23ddddddddddddddddddddddd3eeeENS2_4ffff16ggggggggggggggggENS0_9hhhhhhhhhES6_S6_S6_S6_S6_S6_S6_EE
 aaaaa::bbbbbb::ccccc<ddddddddddddddddddddddd::eee, ddddddddddddddddddddddd::ffff::gggggggggggggggg, aaaaa::bbbbbb::hhhhhhhhh, aaaaa::bbbbbb::hhhhhhhhh, aaaaa::bbbbbb::hhhhhhhhh, aaaaa::bbbbbb::hhhhhhhhh, aaaaa::bbbbbb::hhhhhhhhh, aaaaa::bbbbbb::hhhhhhhhh, aaaaa::bbbbbb::hhhhhhhhh, aaaaa::bbbbbb::hhhhhhhhh>
+--format=gnu-v3
+_Z5outerIsEcPFilE
+char outer<short>(int (*)(long))
+--format=gnu-v3
+_Z5outerPFsiEl
+outer(short (*)(int), long)
+--format=gnu-v3
+_Z6outer2IsEPFilES1_
+int (*outer2<short>(int (*)(long)))(long)
+--format=gnu-v3 --ret-postfix
+_Z5outerIsEcPFilE
+outer<short>(int (*)(long))char
+--format=gnu-v3 --ret-postfix
+_Z5outerPFsiEl
+outer(short (*)(int), long)
+--format=gnu-v3 --ret-postfix
+_Z6outer2IsEPFilES1_
+outer2<short>(int (*)(long))int (*)(long)
+--format=gnu-v3 --ret-drop
+_Z5outerIsEcPFilE
+outer<short>(int (*)(long))
+--format=gnu-v3 --ret-drop
+_Z5outerPFsiEl
+outer(short (*)(int), long)
+--format=gnu-v3 --ret-drop
+_Z6outer2IsEPFilES1_
+outer2<short>(int (*)(long))
 #
 # Ada (GNAT) tests.
 #
--- a/libiberty/testsuite/test-demangle.c
+++ b/libiberty/testsuite/test-demangle.c
@@ -159,6 +159,7 @@ exp: %s\n",
                          output is an integer representing ctor_kind.
      --is-v3-dtor        Likewise, but for dtors.
      --ret-postfix       Passes the DMGL_RET_POSTFIX option
+     --ret-drop          Passes the DMGL_RET_DROP option
 
    For compatibility, just in case it matters, the options line may be
    empty, to mean --format=auto.  If it doesn't start with --, then it
@@ -174,7 +175,7 @@ main(argc, argv)
   int no_params;
   int is_v3_ctor;
   int is_v3_dtor;
-  int ret_postfix;
+  int ret_postfix, ret_drop;
   struct line format;
   struct line input;
   struct line expect;
@@ -209,6 +210,7 @@ main(argc, argv)
 
       no_params = 0;
       ret_postfix = 0;
+      ret_drop = 0;
       is_v3_ctor = 0;
       is_v3_dtor = 0;
       if (format.data[0] == '\0')
@@ -265,6 +267,8 @@ main(argc, argv)
 		is_v3_dtor = 1;
 	      else if (strcmp (opt, "--ret-postfix") == 0)
 		ret_postfix = 1;
+	      else if (strcmp (opt, "--ret-drop") == 0)
+		ret_drop = 1;
 	      else
 		{
 		  printf ("FAIL at line %d: unrecognized option %s\n",
@@ -307,9 +311,9 @@ main(argc, argv)
 
       cplus_demangle_set_style (style);
 
-      result = cplus_demangle (inp,
-			       DMGL_PARAMS|DMGL_ANSI|DMGL_TYPES
-			       |(ret_postfix ? DMGL_RET_POSTFIX : 0));
+      result = cplus_demangle (inp, (DMGL_PARAMS | DMGL_ANSI | DMGL_TYPES
+				     | (ret_postfix ? DMGL_RET_POSTFIX : 0)
+				     | (ret_drop ? DMGL_RET_DROP : 0)));
 
       if (result
 	  ? strcmp (result, expect.data)

Re: [rfc 2/2] Follow DW_AT_linkage_name for methods [Re: The future of dwarf2_physname]

[Keith Seitz]

I don't know if you are waiting on me for a reply on this or not, but I 
thought I would throw out my fractional pennies.

On 05/24/2011 02:21 PM, Jan Kratochvil wrote:
> Also after a chat with Tom Tromey I no longer use DMGL_RET_POSTFIX.  That is
> for ELF symbol `long m<char>()' GDB with proposed patch uses `m<char>()'
> (and not `m<char>()long').  This is compatible with gdb-7.2 (physname)
> template functions.

I am relieved to hear this (mis?)feature is staying put. I'm a practical 
kind of person, and I know I'd really *hate* to have to type "break 'int 
blah<int>(blah)'" every time. Yuck.

As for how to address the (remote?) possibility that we will need a 
return type, I'd like to offer the idea that we might introduce some 
sort of qualifier flag, for example, "break blah<int>/int" or "break/int 
blah<int>" or something along those lines. Of course, this is well 
beyond the scope of 7.3. Just thought I'd toss this out there, though, 
as a starting point should this actually become a problem.

> Anyway DW_AT_linkage_name following patch with no regressions on
> {x86_64,x86_64-m32,i686}-fedora15-linux-gnu is attached.  It would need GCC
> push of the libiberty/ change.

I'll reiterate what I wrote to Tom yesterday, "I'm all for it."

Keith

Re: [rfc 2/2] Follow DW_AT_linkage_name for methods [Re: The future of dwarf2_physname]

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> Also after a chat with Tom Tromey I no longer use DMGL_RET_POSTFIX.
Jan> That is for ELF symbol `long m<char>()' GDB with proposed patch
Jan> uses `m<char>()' (and not `m<char>()long').  This is compatible
Jan> with gdb-7.2 (physname) template functions.

This patch looks pretty good to me.

Keith, are there parts of your series which are still needed for 7.3 if
this patch is committed there?

The new command probably needs documentation (it is unclear to me
whether we document, or even whether we should document, "set debug"
commands).

Jan> +	  canon = (char *) mangled;

I think 'canon' can be made const and this cast reomved.

Tom

Re: [rfc 2/2] Follow DW_AT_linkage_name for methods [Re: The future of dwarf2_physname]

[Tom Tromey]

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> As for how to address the (remote?) possibility that we will need a
Keith> return type, I'd like to offer the idea that we might introduce some
Keith> sort of qualifier flag, for example, "break blah<int>/int" or
Keith> "break/int blah<int>" or something along those lines. Of course, this
Keith> is well beyond the scope of 7.3. Just thought I'd toss this out there,
Keith> though, as a starting point should this actually become a problem.

If we are adding linespec syntax, it seems to me we should just
recognize the return type as well: 'break int templatefunction()'.  This
is immediately recognizable to the C++ programmer and preserve the "nm
-C cut-and-paste" property as well.  Only if this cannot be done for
some reason do I think we should consider gdb-specific alternatives.

Tom

Re: [rfc 2/2] Follow DW_AT_linkage_name for methods [Re: The future of dwarf2_physname]

[Keith Seitz]

On 05/25/2011 01:40 PM, Tom Tromey wrote:
>>>>>> "Jan" == Jan Kratochvil<jan.kratochvil@redhat.com>  writes:
>
> Jan>  Also after a chat with Tom Tromey I no longer use DMGL_RET_POSTFIX.
> Jan>  That is for ELF symbol `long m<char>()' GDB with proposed patch
> Jan>  uses `m<char>()' (and not `m<char>()long').  This is compatible
> Jan>  with gdb-7.2 (physname) template functions.
>
> This patch looks pretty good to me.
>
> Keith, are there parts of your series which are still needed for 7.3 if
> this patch is committed there?

If we want to fix 12266, yes, we're still going to need much of the 
patchset: cp_canonicalize_no_typedefs, printname, and the 
cp_demangled_name_parse_free patches. Those patches fix 12266 and a 
bunch of related problems (which are not regressions):

*** Using pre-physname:

(gdb) b calltest(foo)
Function "calltest(foo)" not defined.
Make breakpoint pending on future shared library load? (y or [n]) n
(gdb) b calltest(std::string)
Breakpoint 1 at 0x40075c: file 12266.cc, line 5.
(gdb) b calltest(std::basic_string<char, std::char_traits<char>, 
std::allocator<char> >)
Function "calltest(std::basic_string<char, std::char_traits<char>, 
std::allocator<char> >)" not defined.
Make breakpoint pending on future shared library load? (y or [n]) n


*** Using DW_AT_linkage_name (Jan's patches):

(gdb) b calltest(foo)
Function "calltest(foo)" not defined.
Make breakpoint pending on future shared library load? (y or [n]) n
(gdb) b calltest(std::string)
Function "calltest(std::string)" not defined.
Make breakpoint pending on future shared library load? (y or [n]) n
(gdb) b calltest(std::basic_string<char, std::char_traits<char>, 
std::allocator<char> >)
Breakpoint 1 at 0x40075c: file 12266.cc, line 5.

*** Using 12266 patches (on top of Jan's DW_AT_linkage_name patches):

(gdb) b calltest(foo)
Breakpoint 1 at 0x40075c: file 12266.cc, line 5.
(gdb) b calltest(std::string)
Note: breakpoint 1 also set at pc 0x40075c.
Breakpoint 2 at 0x40075c: file 12266.cc, line 5.
(gdb) b calltest(std::basic_string<char, std::char_traits<char>, 
std::allocator<char> >)
Note: breakpoints 1 and 2 also set at pc 0x40075c.
Breakpoint 3 at 0x40075c: file 12266.cc, line 5.

Mind you, I don't know if I would recommend taking in such a big patch 
this late in the game... Nonetheless, I hope to have this resubmitted today.

Keith

obsolete: [rfc 1/2] libiberty/ options code reshuffle [Re: The future of dwarf2_physname]

[Jan Kratochvil]

obsoleted by:
	[gcc patch 0/3] libiberty: New DMGL_RET_DROP
	http://sourceware.org/ml/gdb-patches/2011-06/msg00019.html

Re: [rfc 2/2] Follow DW_AT_linkage_name for methods [Re: The future of dwarf2_physname]

[Jan Kratochvil]

On Wed, 01 Jun 2011 20:35:15 +0200, Keith Seitz wrote:
> *** Using pre-physname:
[...]
> (gdb) b calltest(std::string)
> Breakpoint 1 at 0x40075c: file 12266.cc, line 5.
[...]
> *** Using DW_AT_linkage_name (Jan's patches):
[...]
> (gdb) b calltest(std::string)
> Function "calltest(std::string)" not defined.
> Make breakpoint pending on future shared library load? (y or [n]) n
[...]
> *** Using 12266 patches (on top of Jan's DW_AT_linkage_name patches):
[...]
> (gdb) b calltest(std::string)
> Note: breakpoint 1 also set at pc 0x40075c.
> Breakpoint 2 at 0x40075c: file 12266.cc, line 5.
[...]

This is a regression by the physname patch which introduced DMGL_VERBOSE in
symbol_find_demangled_name, some other patches of mine being posted are going
to remove DMGL_VERBOSE there.


Thanks,
Jan

Re: [rfc 2/2] Follow DW_AT_linkage_name for methods [Re: The future of dwarf2_physname]

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> This is a regression by the physname patch which introduced
Jan> DMGL_VERBOSE in symbol_find_demangled_name, some other patches of
Jan> mine being posted are going to remove DMGL_VERBOSE there.

I infer this to mean that, with your DW_AT_linkage_name patch, we do not
need Keith's series for 7.3.  Is that correct?

Tom

Re: [rfc 2/2] Follow DW_AT_linkage_name for methods [Re: The future of dwarf2_physname]

[Jan Kratochvil]

On Thu, 02 Jun 2011 20:20:01 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> Jan> This is a regression by the physname patch which introduced
> Jan> DMGL_VERBOSE in symbol_find_demangled_name, some other patches of
> Jan> mine being posted are going to remove DMGL_VERBOSE there.
> 
> I infer this to mean that, with your DW_AT_linkage_name patch, we do not
> need Keith's series for 7.3.  Is that correct?

For ctors/dtors (and for non-GCC compilers) DW_AT_linkage_name does not exist
and it falls back to the physname code.

Some other former code dealing with ctors/dtors in symtabs before physname
code was introduced has been already removed as obsoleted by physname.

Therefore I believe any physname improvements still have effect for
ctors/dtors handling.


Thanks,
Jan

obsolete: [rfc 2/2] Follow DW_AT_linkage_name for methods [Re: The future of dwarf2_physname]

[Jan Kratochvil]

obsoleted/reposted by:
	[patch] Follow DW_AT_linkage_name for methods #2
	http://sourceware.org/ml/gdb-patches/2011-06/msg00034.html