GCC and Clang disagree on symbol mangling

[Quuxplusone]

Bugzilla Link	PR24794
Status	NEW
Importance	P normal
Reported by	Steven Noonan (steven@uplinklabs.net)
Reported on	2015-09-12 05:47:28 -0700
Last modified on	2017-10-03 11:57:30 -0700
Version	3.7
Hardware	PC Linux
CC	anton@korobeynikov.info, dgregor@apple.com, hfinkel@anl.gov, llvm-bugs@lists.llvm.org, paul_robinson@playstation.sony.com, richard-llvm@metafoo.co.uk, rjmccall@apple.com, sylvestre@debian.org, yaghmour.shafik@gmail.com
Fixed by commit(s)
Attachments	test.cpp (1124 bytes, text/x-c++src)
Blocks
Blocked by
See also

Created attachment 14871
test case

Running Clang 3.6.2.

I have some C++ code that fails to link because Clang and GCC are coming up
with different symbol mangling for some specific symbols in some of the
dependent libraries. I managed to come up with a small test case that
illustrates the issue (it's probably not fully reduced). I've attached it to
this bug report.

$ g++ -c test.cpp; nm -C test.o; nm test.o
+ g++ -c test.cpp
+ nm -C test.o
0000000000000000 T bar(foo::V&, int&, int*)
                 U baz::enable_if<!baz::is_enum<int>::value, void>::type baz::Conv<int>(int&, int*)
                 U baz::enable_if<!baz::is_enum<foo::V>::value, void>::type baz::Conv<foo::V>(foo::V&, int*)
+ nm test.o
0000000000000000 T _Z3barRN3foo1VERiPi
                 U _ZN3baz4ConvIiEENS_9enable_ifIXntsrNS_7is_enumIT_EE5valueEvE4typeERS3_Pi
                 U _ZN3baz4ConvIN3foo1VEEENS_9enable_ifIXntsrNS_7is_enumIT_EE5valueEvE4typeERS5_Pi

$ clang++ -c test.cpp; nm -C test.o; nm test.o
+ clang++ -c test.cpp
test.cpp:65:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
1 warning generated.
+ nm -C test.o
0000000000000000 T bar(foo::V&, int&, int*)
                 U _ZN3baz4ConvIiEENS_9enable_ifIXntsr7is_enumIT_EE5valueEvE4typeERS2_Pi
                 U _ZN3baz4ConvIN3foo1VEEENS_9enable_ifIXntsr7is_enumIT_EE5valueEvE4typeERS4_Pi
+ nm test.o
0000000000000000 T _Z3barRN3foo1VERiPi
                 U _ZN3baz4ConvIiEENS_9enable_ifIXntsr7is_enumIT_EE5valueEvE4typeERS2_Pi
                 U _ZN3baz4ConvIN3foo1VEEENS_9enable_ifIXntsr7is_enumIT_EE5valueEvE4typeERS4_Pi

Note that the Clang-mangled symbol cannot be demangled by c++filt or nm.

Any ideas?

[Quuxplusone]

Attached test.cpp (1124 bytes, text/x-c++src): test case

[Quuxplusone]

This change seems to fix this particular case:

diff --git a/lib/AST/ItaniumMangle.cpp b/lib/AST/ItaniumMangle.cpp
index 774ef9a..78647d9 100644
--- a/lib/AST/ItaniumMangle.cpp
+++ b/lib/AST/ItaniumMangle.cpp
@@ -1568,8 +1568,8 @@ bool
CXXNameMangler::mangleUnresolvedTypeOrSimpleId(QualType Ty,
       if (isa<TemplateTemplateParmDecl>(TD))
         goto unresolvedType;

-      mangleSourceName(TD->getIdentifier());
-      break;
+      mangleType(TST);
+      return true;
     }

     case TemplateName::OverloadedTemplate:

This does not pass "make check-all" though:

$ make check-all
llvm[0]: Running test suite
make[1]: Entering directory '/home/snoonan/Development/llvm/build/test'
Making LLVM 'lit.site.cfg' file...
Making LLVM unittest 'lit.site.cfg' file...
make -C /home/snoonan/Development/llvm/build/test/../tools/clang/test
lit.site.cfg Unit/lit.site.cfg
make[2]: Entering directory
'/home/snoonan/Development/llvm/build/tools/clang/test'
Making Clang 'lit.site.cfg' file...
Making Clang 'Unit/lit.site.cfg' file...
make[2]: Leaving directory
'/home/snoonan/Development/llvm/build/tools/clang/test'
( ulimit -t 1200 ; ulimit -d 512000 ; ulimit -m 512000 ; ulimit -s 8192 ; \
  /usr/bin/python2 /home/snoonan/Development/llvm/utils/lit/lit.py -s -v . /home/snoonan/Development/llvm/build/test/../tools/clang/test )
GNU ld (GNU Binutils) 2.25.1
lit.py: lit.cfg:195: note: using clang:
'/home/snoonan/Development/llvm/build/Debug+Asserts/bin/clang'
lit.py: lit.cfg:333: note: Did not find clang-interpreter in
/home/snoonan/Development/llvm/build/Debug+Asserts/bin:/home/snoonan/Development/llvm/build/Debug+Asserts/bin
FAIL: Clang :: CodeGenCXX/mangle-template.cpp (2485 of 23270)
******************** TEST 'Clang :: CodeGenCXX/mangle-template.cpp' FAILED
********************
Script:
--
/home/snoonan/Development/llvm/build/Debug+Asserts/bin/clang -cc1 -internal-
isystem
/home/snoonan/Development/llvm/build/Debug+Asserts/bin/../lib/clang/3.8.0/include
-nostdsysteminc -verify -Wno-return-type -Wno-main -std=c++11 -emit-llvm -
triple x86_64-unknown-linux-gnu -o -
/home/snoonan/Development/llvm/tools/clang/test/CodeGenCXX/mangle-template.cpp
| /home/snoonan/Development/llvm/build/Debug+Asserts/bin/FileCheck
/home/snoonan/Development/llvm/tools/clang/test/CodeGenCXX/mangle-template.cpp
--
Exit Code: 1

Command Output (stderr):
--
/home/snoonan/Development/llvm/tools/clang/test/CodeGenCXX/mangle-
template.cpp:101:18: error: expected string not found in input
 // CHECK-LABEL: define weak_odr {{.*}}void @_ZN5test81fIiEEvNS_5int_cIXsr4metaIT_E4typeE5valueEEE(
                 ^
<stdin>:1:1: note: scanning from here
; ModuleID = '/home/snoonan/Development/llvm/tools/clang/test/CodeGenCXX/mangle-
template.cpp'
^
<stdin>:161:1: note: possible intended match here
define weak_odr void
@_ZN5test71XIiEC2IdEEPT_PNS_5int_cIXplL_ZNS_4metaIiE5valueEEsrNS6_IS3_EE5valueEE4typeE(%"struct.test7::X"*
%this, double*, float*) unnamed_addr #0 comdat align 2 {
^

--

********************
FAIL: Clang :: CodeGenCXX/mangle.cpp (2506 of 23270)
******************** TEST 'Clang :: CodeGenCXX/mangle.cpp' FAILED
********************
Script:
--
/home/snoonan/Development/llvm/build/Debug+Asserts/bin/clang -cc1 -internal-
isystem
/home/snoonan/Development/llvm/build/Debug+Asserts/bin/../lib/clang/3.8.0/include
-nostdsysteminc -emit-llvm
/home/snoonan/Development/llvm/tools/clang/test/CodeGenCXX/mangle.cpp -o - -
triple=x86_64-apple-darwin9 -fblocks -std=c++11 |
/home/snoonan/Development/llvm/build/Debug+Asserts/bin/FileCheck
/home/snoonan/Development/llvm/tools/clang/test/CodeGenCXX/mangle.cpp
--
Exit Code: 1

Command Output (stderr):
--
/home/snoonan/Development/llvm/tools/clang/test/CodeGenCXX/mangle.cpp:655:22:
warning: control reaches end of non-void function
  static char bar() {}
                     ^
/home/snoonan/Development/llvm/tools/clang/test/CodeGenCXX/mangle.cpp:850:54:
warning: control reaches end of non-void function
  auto f1(Types... values) -> A<sizeof...(values)> { }
                                                     ^
/home/snoonan/Development/llvm/tools/clang/test/CodeGenCXX/mangle.cpp:853:17:
note: in instantiation of function template specialization 'test36::f1<int,
float>' requested here
  template A<2> f1(int, float);
                ^
/home/snoonan/Development/llvm/tools/clang/test/CodeGenCXX/mangle.cpp:241:17:
error: expected string not found in input
// CHECK-LABEL: define linkonce_odr void
@_ZN6PR57968__fill_aIiEENS_11__enable_ifIXntsr16__is_scalar_typeIT_EE7__valueEvE6__typeEv
                ^
<stdin>:304:19: note: scanning from here
define void @_Z1fi(i32) #0 {
                  ^
2 warnings generated.
<stdin>:455:1: note: possible intended match here
define weak_odr void
@_Z3ft7IiEN11__enable_ifIXsr16__is_scalar_typeIT_E7__valueEvE6__typeEv() #0 {
^

--

********************
Testing Time: 209.53s
********************
Failing Tests (2):
    Clang :: CodeGenCXX/mangle-template.cpp
    Clang :: CodeGenCXX/mangle.cpp

  Expected Passes    : 19156
  Expected Failures  : 54
  Unsupported Tests  : 4058
  Unexpected Failures: 2
Makefile:105: recipe for target 'check-local-all' failed
make[1]: *** [check-local-all] Error 1
make[1]: Leaving directory '/home/snoonan/Development/llvm/build/test'
/home/snoonan/Development/llvm/Makefile.rules:1807: recipe for target 'check-
all' failed
make: *** [check-all] Error 2

[Quuxplusone]

This is more correct, based on some other tests I've done. Still doesn't pass
the test suite, but I'm finding that a lot of the test suite failures are using
incorrect checks, based on what GCC emits (and what actually passes through
nm/c++filt correctly).

diff --git a/lib/AST/ItaniumMangle.cpp b/lib/AST/ItaniumMangle.cpp
index 774ef9a..7404b95 100644
--- a/lib/AST/ItaniumMangle.cpp
+++ b/lib/AST/ItaniumMangle.cpp
@@ -1568,8 +1568,19 @@ bool
CXXNameMangler::mangleUnresolvedTypeOrSimpleId(QualType Ty,
       if (isa<TemplateTemplateParmDecl>(TD))
         goto unresolvedType;

-      mangleSourceName(TD->getIdentifier());
-      break;
+      Out << "N";
+
+      if (mangleSubstitution(QualType(TST, 0)))
+        return false;
+
+      mangleTemplatePrefix(TST->getTemplateName());
+
+      // FIXME: GCC does not appear to mangle the template arguments when
+      // the template in question is a dependent template name. Should we
+      // emulate that badness?
+      mangleTemplateArgs(TST->getArgs(), TST->getNumArgs());
+      addSubstitution(QualType(TST, 0));
+      return false;
     }

     case TemplateName::OverloadedTemplate:

I'm stumbling a bit blindly here, though. It feels like we should be doing "Out
<< Prefix;" instead of "N" there. Something calling
mangleUnresolvedTypeOrSimpleId probably needs changing to use "N" under these
conditions.

[Quuxplusone]

Alright, last comment for the night. I need some sleep.

Here's what I ended up settling on. The project affected by this issue now
links properly with the GCC-built libraries:

diff --git a/lib/AST/ItaniumMangle.cpp b/lib/AST/ItaniumMangle.cpp
index 774ef9a..6f834a7 100644
--- a/lib/AST/ItaniumMangle.cpp
+++ b/lib/AST/ItaniumMangle.cpp
@@ -1568,8 +1568,14 @@ bool
CXXNameMangler::mangleUnresolvedTypeOrSimpleId(QualType Ty,
       if (isa<TemplateTemplateParmDecl>(TD))
         goto unresolvedType;

-      mangleSourceName(TD->getIdentifier());
-      break;
+      if (mangleSubstitution(QualType(TST, 0)))
+        return false;
+
+      Out << "N";
+      mangleTemplatePrefix(TST->getTemplateName());
+      mangleTemplateArgs(TST->getArgs(), TST->getNumArgs());
+      addSubstitution(QualType(TST, 0));
+      return false;
     }

     case TemplateName::OverloadedTemplate:

Here are the corresponding regression test changes that I'm fairly confident in:

diff --git a/test/CodeGenCXX/mangle-template.cpp b/test/CodeGenCXX/mangle-
template.cpp
index 7fa300a..9af0ebc 100644
--- a/test/CodeGenCXX/mangle-template.cpp
+++ b/test/CodeGenCXX/mangle-template.cpp
@@ -79,7 +79,7 @@ namespace test7 {
     X(U*, typename int_c<(meta<T>::value + meta<U>::value)>::type *) { }
   };

-  // CHECK: define weak_odr {{.*}}
@_ZN5test71XIiEC1IdEEPT_PNS_5int_cIXplL_ZNS_4metaIiE5valueEEsr4metaIS3_EE5valueEE4typeE(
+  // CHECK: define weak_odr {{.*}}
@_ZN5test71XIiEC1IdEEPT_PNS_5int_cIXplL_ZNS_4metaIiE5valueEEsrNS6_IS3_EE5valueEE4typeE(
   template X<int>::X(double*, float*);
 }

@@ -98,7 +98,7 @@ namespace test8 {
   template<typename T>
   void f(int_c<meta<T>::type::value>) { }

-  // CHECK-LABEL: define weak_odr {{.*}}void
@_ZN5test81fIiEEvNS_5int_cIXsr4metaIT_E4typeE5valueEEE(
+  // CHECK-LABEL: define weak_odr {{.*}}void
@_ZN5test81fIiEEvNS_5int_cIXsrNS_4metaIT_E4typeE5valueEEE(
   template void f<int>(int_c<sizeof(int)>);
 }

diff --git a/test/CodeGenCXX/mangle.cpp b/test/CodeGenCXX/mangle.cpp
index 5012c3b..26e1d72 100644
--- a/test/CodeGenCXX/mangle.cpp
+++ b/test/CodeGenCXX/mangle.cpp
@@ -193,9 +193,9 @@ template<typename T> struct __enable_if<true, T> {
 // PR5063
 template<typename T> typename __enable_if<__is_scalar_type<T>::__value, void>::__type ft7() { }

-// CHECK:
@_Z3ft7IiEN11__enable_ifIXsr16__is_scalar_typeIT_EE7__valueEvE6__typeEv
+// CHECK:
@_Z3ft7IiEN11__enable_ifIXsrN16__is_scalar_typeIT_EE7__valueEvE6__typeEv
 template void ft7<int>();
-// CHECK:
@_Z3ft7IPvEN11__enable_ifIXsr16__is_scalar_typeIT_EE7__valueEvE6__typeEv
+// CHECK:
@_Z3ft7IPvEN11__enable_ifIXsrN16__is_scalar_typeIT_EE7__valueEvE6__typeEv
 template void ft7<void*>();

 // PR5144
@@ -223,9 +223,9 @@ S7::S7() {}

 // PR5063
 template<typename T> typename __enable_if<(__is_scalar_type<T>::__value), void>::__type ft8() { }
-// CHECK:
@_Z3ft8IiEN11__enable_ifIXsr16__is_scalar_typeIT_EE7__valueEvE6__typeEv
+// CHECK:
@_Z3ft8IiEN11__enable_ifIXsrN16__is_scalar_typeIT_EE7__valueEvE6__typeEv
 template void ft8<int>();
-// CHECK:
@_Z3ft8IPvEN11__enable_ifIXsr16__is_scalar_typeIT_EE7__valueEvE6__typeEv
+// CHECK:
@_Z3ft8IPvEN11__enable_ifIXsrN16__is_scalar_typeIT_EE7__valueEvE6__typeEv
 template void ft8<void*>();

 // PR5796
@@ -238,7 +238,7 @@ template<bool, typename> struct __enable_if {};
 template<typename T> struct __enable_if<true, T> { typedef T __type; };
 template<typename T>

-// CHECK-LABEL: define linkonce_odr void
@_ZN6PR57968__fill_aIiEENS_11__enable_ifIXntsr16__is_scalar_typeIT_EE7__valueEvE6__typeEv
+// CHECK-LABEL: define linkonce_odr void
@_ZN6PR57968__fill_aIiEENS_11__enable_ifIXntsrNS_16__is_scalar_typeIT_EE7__valueEvE6__typeEv
 typename __enable_if<!__is_scalar_type<T>::__value, void>::__type __fill_a() { };

 void f() { __fill_a<int>(); }
@@ -716,7 +716,7 @@ namespace test28 {

   void test() {
     foo<char>(A<char>::bit);
-    // CHECK: call void @_ZN6test283fooIcEEvDtsr1AIT_E1AE3bitE(
+    // CHECK: call void @_ZN6test283fooIcEEvDtsrNS_1AIT_E1AE3bitE(
   }
 }

@@ -793,7 +793,7 @@ namespace test33 {

   void test() {
     foo<B>();
-    // CHECK: call i32
@_ZN6test333fooINS_1BEEENS_1AIT_Xsr1XIS3_EE5valueEE4typeEv()
+    // CHECK: call i32
@_ZN6test333fooINS_1BEEENS_1AIT_XsrNS_1XIS3_EE5valueEE4typeEv()
   }
 }

Each of the above changes matches the new LLVM output, is successfully
demangled by nm/c++filt, and the demangled output matches what the input
appears to be.

This is the one regression test change that I'm not confident in at all:

diff --git a/test/CodeGenCXX/mangle.cpp b/test/CodeGenCXX/mangle.cpp
index 5012c3b..26e1d72 100644
--- a/test/CodeGenCXX/mangle.cpp
+++ b/test/CodeGenCXX/mangle.cpp
@@ -1055,7 +1055,7 @@ namespace test51 {
   template void fun4<int>();
   // CHECK-LABEL: @_ZN6test514fun4IiEEDTcmcldtcv2S1IT_E_Edn2S1IS2_EEcldtcvS3__Edn2S1IS2_EEEv
   template void fun5<int>();
-  // CHECK-LABEL: @_ZN6test514fun5IiEEDTcldtcv2S1IiE_Edn2S1IT_EEEv
+  // CHECK-LABEL: @_ZN6test514fun5IiEEDTcldtcv2S1IiE_EdnNS1_IT_EEEv
   template void fun6<S1>();
   // CHECK-LABEL: @_ZN6test514fun6I2S1EEDTcldtcvS1_IiE_EdnT_IiEEEv
   template void fun7<E>();

Neither of the two symbol names can be demangled by c++filt, and I also can't
get the test case to compile with GCC, so I cannot cross-check results.

[Quuxplusone]

Changing version to reflect this also occurs on 3.7 (also present on trunk as well).

I've been using with the last patch I mentioned here since I wrote it, building and running multiple fairly complex C++ projects with no issues so far. I'm sure someone can come up with a nicer fix though. ;)

[Quuxplusone]

Richard and/or John, any opinion on this mangling issue?

[Quuxplusone]

GCC is not a single point of truth on symbol mangling. If we disagree about manglings, we should fix things to follow the spec, fixing the spec where necessary.

[Quuxplusone]

In other words, we're not going to accept patches that just blindly emulate the behavior of an unspecified GCC version; you need to make an argument that what we're doing is actually wrong, which will also direct the implementation.

[Quuxplusone]

It sounds like we agree that GCC and Clang outputs should be link compatible,
with respect to the specification.

If GCC is wrong, then it also stands to reason that binutils is wrong as well
(nm and c++filt can't demangle the Clang-generated mangled C++ symbols in my
test case). And if a fix is submitted to gcc/binutils it's likely that they
would be rejected in both projects on the grounds of link compatibility with
previously built libraries. So I'm concerned that this would end up coming back
to clang even *if* gcc/binutils are doing it wrong. Is that a valid concern?

I'm not sure if the Itanium mangling spec is being followed by either compiler,
partially because it seems the mangling spec is meant to be read by higher
beings than myself...

Can you please review my change and vet it against the specification? I'm not
super-attached to the change. It's certainly not "obviously correct". But it
*did* solve the problem for the big C++ project I'm working on at work.

[Quuxplusone]

Could you at least confirm the version of GCC you're using?

[Quuxplusone]

Also, what are the actual mangled symbols that GCC is producing?

[Quuxplusone]

See the original post for the symbol differences for the attached testcase.

I've looked at GCC versions 4.3.6, 4.4.7, 4.5.4, 4.6.4, 4.7.4, 4.8.4, 4.9.2, and 5.2.0. They all generate the same mangling.

[Quuxplusone]

(Also, GCC versions older than 4.3 fail to compile the test case because they don't seem to evaluate __is_enum as a constant expression.)

[Quuxplusone]

More data: The Intel C++ compiler 15.0.2 generates symbols identical to GCC for this test case.

[Quuxplusone]

Hmm.  is_enum<T>::value is an unresolved-name; both compilers agree on that.

GCC appears to be reasoning that, because we know that is_enum is baz::is_enum
and there's an existing substitution for baz, we should mangle it using
  srN <unresolved-type> <unresolved-qualifier-level>+ E <base-unresolved-name>
where the <unresolved-type> is the substitution for baz.

Clang is reasoning that baz is not a type, and that the <substitution> under
<unresolved-type> is intended to match previously-emitted decltypes, not
arbitrary substitutions; and therefore it is using
 [gs] sr <unresolved-qualifier-level>+ E <base-unresolved-name>
which mangles only the source information from the name and therefore omits the
baz prefix.

Clang's decision here seems much more defensible to me; if the spec intended
resolved information to be mangled in the prefix of an unresolved-name, it
would permit it regardless of whether there happened to be a substitution for
it.

[Quuxplusone]

So what's the path forward?

I'm concerned gcc/binutils would reject changes for this issue, as any change to the mangling would be effectively an ABI break.

[Quuxplusone]

I don't think binutils has anything to do with it; they're just using the system demangler for libsupc++, which should definitely accept that mangling, even if it's not the one that GCC is using. There are no binary compatibility problems from demangling more things, anyway.

The binary-compatibility question is up to GCC, and it's something that needs to be raised on cxx-avi-dev if it hasn't already.

[Quuxplusone]

_Bug 34810 has been marked as a duplicate of this bug._

[Quuxplusone]

_Bug 34810 has been marked as a duplicate of this bug._

[Quuxplusone]

ABI issue filed as https://github.com/itanium-cxx-abi/cxx-abi/issues/38, with reference to rjmccall's thread from 2015. FWIW, I think the testcase in PR34810 provides a convincing argument that the ABI and Clang's behavior are wrong -- declarations that declare different function templates (because the result of lookup of a non-dependent name selects a different entity in the prefix of an unresolved-name) get the same mangling.

The resolved initial prefix of an unresolved-name needs to be mangled as denoting an entity, not merely a sequence of source-ids.