Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Jan Kratochvil <jan.kratochvil@redhat.com>
To: Keith Seitz <keiths@redhat.com>
Cc: gdb-patches@sourceware.org
Subject: Re: The future of dwarf2_physname
Date: Fri, 20 May 2011 20:38:00 -0000	[thread overview]
Message-ID: <20110520203755.GA3662@host1.jankratochvil.net> (raw)
In-Reply-To: <4DD6C6B4.7060406@redhat.com>

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


  reply	other threads:[~2011-05-20 20:38 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-18 22:35 Keith Seitz
2011-05-19 19:23 ` Jan Kratochvil
2011-05-20 19:53   ` Keith Seitz
2011-05-20 20:38     ` Jan Kratochvil [this message]
2011-05-20 20:39     ` Tom Tromey
2011-05-21 20:37       ` Daniel Jacobowitz
2011-05-19 21:00 ` Daniel Jacobowitz
2011-05-20 19:26   ` Tom Tromey
2011-05-21 20:34     ` Daniel Jacobowitz
2011-05-20 19:10 ` Tom Tromey
2011-05-23 13:17 ` Jan Kratochvil
2011-05-23 19:52   ` Tom Tromey
2011-05-23 19:57     ` Keith Seitz
2011-05-24 21:12   ` [rfc 1/2] libiberty/ options code reshuffle [Re: The future of dwarf2_physname] Jan Kratochvil
2011-06-02 15:36     ` obsolete: " Jan Kratochvil
2011-05-24 21:21   ` [rfc 2/2] Follow DW_AT_linkage_name for methods " Jan Kratochvil
2011-05-25 19:40     ` Keith Seitz
2011-05-25 20:42       ` Tom Tromey
2011-05-25 20:40     ` Tom Tromey
2011-06-01 18:35       ` Keith Seitz
2011-06-02 16:26         ` Jan Kratochvil
2011-06-02 18:20           ` Tom Tromey
2011-06-02 18:28             ` Jan Kratochvil
2011-06-02 19:03     ` obsolete: " Jan Kratochvil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110520203755.GA3662@host1.jankratochvil.net \
    --to=jan.kratochvil@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=keiths@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox