Patchwork Make cxxfilt demangle internal-linkage templates

login
register
mail settings
Submitter Paul Pluzhnikov
Date Aug. 7, 2013, 6:51 p.m.
Message ID <ye6qpptpqqq0.fsf@elbrus2.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/265592/
State New
Headers show

Comments

Paul Pluzhnikov - Aug. 7, 2013, 6:51 p.m.
Greetings,

I've been redirected here from binutils:
http://sourceware.org/ml/binutils/2013-08/msg00052.html
http://sourceware.org/ml/binutils/2013-08/msg00056.html

The following source:

  template<typename T> static void f();
  void g() { f<int>(); }

results in "_Z1fIiEvv" under g++, but in "_ZL1fIiEvv" under clang.

Richard Smith says:

  The ABI doesn't cover manglings for local symbols ...
  ... and c++filt is not able to cope with the L prefix here.

  I'm having a hard time seeing how this isn't a g++ bug and a matching
  c++filt bug.

It's hard for me to argue that this is a 'g++' bug (since there is no
ABI violation here), but c++filt should be able to handle this, and does
with attached patch.

Ok for trunk?

Thanks,

Google ref: b/10137049
---
Paul Pluzhnikov


2013-08-07  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* cp-demangle.c (d_name): Handle internal-linkage templates.
	* testsuite/demangle-expected: New case.
Paul Pluzhnikov - Aug. 17, 2013, 1:10 a.m.
Ping?

On Wed, Aug 7, 2013 at 11:51 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:

> The following source:
>
>   template<typename T> static void f();
>   void g() { f<int>(); }
>
> results in "_Z1fIiEvv" under g++, but in "_ZL1fIiEvv" under clang.
>
> Richard Smith says:
>
>   The ABI doesn't cover manglings for local symbols ...
>   ... and c++filt is not able to cope with the L prefix here.
>
>   I'm having a hard time seeing how this isn't a g++ bug and a matching
>   c++filt bug.
Paul Pluzhnikov - Sept. 11, 2013, 9:44 p.m.
Ping x2?

Original message:
http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00394.html

On Fri, Aug 16, 2013 at 6:10 PM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> Ping?


--
Paul Pluzhnikov
Paul Pluzhnikov - Sept. 18, 2013, 7:23 p.m.
Ping x3?

2013/9/11 Paul Pluzhnikov <ppluzhnikov@google.com>:
> Ping x2?
>
> Original message:
> http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00394.html

Thanks,
Paolo Carlini - Sept. 18, 2013, 11:24 p.m.
Hi,

On 09/18/2013 09:23 PM, Paul Pluzhnikov wrote:
> Ping x3?
>
> 2013/9/11 Paul Pluzhnikov <ppluzhnikov@google.com>:
>> Ping x2?
>>
>> Original message:
>> http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00394.html
>
I'm adding Ian in CC.

Paolo.
Paul Pluzhnikov - Oct. 10, 2013, 7:13 p.m.
Ian?

Ping x4.

On Wed, Sep 18, 2013 at 4:24 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:

>>> Original message:
>>> http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00394.html
>>
>>
> I'm adding Ian in CC.
Ian Taylor - Oct. 10, 2013, 8:42 p.m.
On Wed, Aug 7, 2013 at 11:51 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
>
> The following source:
>
>   template<typename T> static void f();
>   void g() { f<int>(); }
>
> results in "_Z1fIiEvv" under g++, but in "_ZL1fIiEvv" under clang.
>
> Richard Smith says:
>
>   The ABI doesn't cover manglings for local symbols ...
>   ... and c++filt is not able to cope with the L prefix here.
>
>   I'm having a hard time seeing how this isn't a g++ bug and a matching
>   c++filt bug.
>
> It's hard for me to argue that this is a 'g++' bug (since there is no
> ABI violation here), but c++filt should be able to handle this, and does
> with attached patch.

OK, I finally took a look.

The demangler treates _ZL as a local-source-name.  This is not part of
the C++ ABI (http://codesourcery.com/cxx-abi/).  It was added by Geoff
Keating: http://gcc.gnu.org/PR31775,
http://gcc.gnu.org/ml/gcc-patches/2007-05/msg00309.html .

With Geoff's patch the demangling is

_Z <encoding>
  <encoding> => <(data) name>
_Z <(data) name>
  <name> => <unscoped-name>
_Z <unscoped-name>
  <unscoped-name> => <unqualified-name>
_Z <unqualified-name>
  <unqualified-name> => <local-source-name> (not in ABI)
_Z <local-source-name>
  <local-source-name> => L <source-name> [<discriminator>] (not in ABI)
_ZL <source-name> [<discriminator>]
  <source-name> => <number> <identifier>
_ZL1f [<discriminator>]

The discriminator is not present, and at this point we are in trouble
because IiE is extraneous.

But really the demangling should be

_Z <encoding>
  <encoding> => <(data) name>
_Z <(data) name>
  <name> => <unscoped-template-name> <template-args>
_Z <unscoped-template-name> <template-args>
  <unscoped-template-name> => <unscoped-name>
_Z <unscoped-name> <template-args>
  <unscoped-name> => <unqualified-name>
_Z <unqualified-name> <template-args>
  <unqualified-name> => <local-source-name> (not in ABI)
_Z <local-source-name> <template-args>
  <local-source-name> => L <source-name> [<discriminator>] (not in ABI)
_ZL <source-name> [<discriminator>] <template-args>
  <source-name> => <number> <identifier>
_ZL1f [<discriminator>] <template-args>
  [<discriminator>] =>
_ZL1f <template-args>

etc.

So it looks to me like Geoff's original patch was wrong, and your
patch is correct.

So this is OK.

Thanks.

Sorry for ignoring this for so long.

Ian

Patch

Index: libiberty/testsuite/demangle-expected
===================================================================
--- libiberty/testsuite/demangle-expected	(revision 201577)
+++ libiberty/testsuite/demangle-expected	(working copy)
@@ -4291,3 +4291,6 @@ 
 --format=gnu-v3
 _Z1nIM1AKFvvREEvT_
 void n<void (A::*)() const &>(void (A::*)() const &)
+--format=gnu-v3
+_ZL1fIiEvv
+void f<int>()
Index: libiberty/cp-demangle.c
===================================================================
--- libiberty/cp-demangle.c	(revision 201577)
+++ libiberty/cp-demangle.c	(working copy)
@@ -1276,7 +1276,6 @@ 
     case 'Z':
       return d_local_name (di);
 
-    case 'L':
     case 'U':
       return d_unqualified_name (di);
 
@@ -1323,6 +1322,7 @@ 
 	return dc;
       }
 
+    case 'L':
     default:
       dc = d_unqualified_name (di);
       if (d_peek_char (di) == 'I')