[demangler] Fix undocumented Local encoding

[urnathan]

Summary

GCC emits [some] static symbols with an 'L' mangling, which we attempt to demangle. But the module mangling changes have exposed that we were doing so at the wrong level. AFAICT this isn't documented in the ABI. This adjusts the demangler along the same lines as the existing gcc demangler (which is not yet module-aware). 'L' is part of an unqualified name. As before we merely parse the 'L', and then ignore it.

@iains discovered this problem when playing with gcc's module implementation.

[I have GCC demangler patches, but GCC is currently stabilizing for release.]

[iains]

just to clarify the intent for:

export module A;

static int addone (int y) { return y + 1; }
static int x = 5;

export int addsix (int z) { return x + addone (z); }

GCC mangles addone => _ZW1AL6addonei
and x as _ZW1AL1x

clang mangles them as _ZL6addonei and _ZL1x respectively.

Whilst this makes no practical difference (since the symbols are TU-local in either case) - I was expecting that decls in the purview of A would be attached to it (and thus mangled as such). I guess I missed some subtlety.

[urnathan]

In D123138#3432207, @iains wrote:

just to clarify the intent for:

export module A;

static int addone (int y) { return y + 1; }
static int x = 5;

export int addsix (int z) { return x + addone (z); }

GCC mangles addone => _ZW1AL6addonei
and x as _ZW1AL1x

clang mangles them as _ZL6addonei and _ZL1x respectively.

Whilst this makes no practical difference (since the symbols are TU-local in either case) - I was expecting that decls in the purview of A would be attached to it (and thus mangled as such). I guess I missed some subtlety.

The ABI just specifies the mangling for cross-TU-visible symbols. Different compilers may choose different manglings for internal-linkage symbols.

That said, it seems I made different choices in GCC and Clang manglers, which isn't the best ($urnathan[GCC] != $urnathan[Clang]). I guess if the elf-linkage on a template instantiation over some internal-linkage entity gets the wrong linkage, the GCC approach is better?

[iains]

In D123138#3432417, @urnathan wrote:

In D123138#3432207, @iains wrote:

just to clarify the intent for:

export module A;

static int addone (int y) { return y + 1; }
static int x = 5;

export int addsix (int z) { return x + addone (z); }

GCC mangles addone => _ZW1AL6addonei
and x as _ZW1AL1x

clang mangles them as _ZL6addonei and _ZL1x respectively.

Whilst this makes no practical difference (since the symbols are TU-local in either case) - I was expecting that decls in the purview of A would be attached to it (and thus mangled as such). I guess I missed some subtlety.

The ABI just specifies the mangling for cross-TU-visible symbols. Different compilers may choose different manglings for internal-linkage symbols.

That said, it seems I made different choices in GCC and Clang manglers, which isn't the best ($urnathan[GCC] != $urnathan[Clang]). I guess if the elf-linkage on a template instantiation over some internal-linkage entity gets the wrong linkage, the GCC approach is better?

That would be my thinking too (also a little more helpful to poor souls who sometimes need to look at .ll and .s files).

[urnathan]

thinking further, adding module attachment makes the symbols longer (probably not much, but still). I also discovered in GCC it was intentionally mangling module attachment to anonymous namespaces. I will change GCC to match Clang's behaviour here -- post GCC12. Meanwhile there's D123220 to verify the attachment is not being added.

ETA: I tested Clang to add this mangling, and it generated the same as GCC, so that's more evidence that this demangler change is correct.

[iains]

this LGTM, at least it demangles the symbols from GCC (presumably those will still be emitted for GCC11 and 12 even if the change is made to 13+ to match clang)

... hopefully the CI test fails are spurious?

libcxxabi/src/demangle/ItaniumDemangle.h

Node *AbstractManglingParser<Derived, Alloc>::parseLocalName(NameState *State) {
 
   Node *Entity = getDerived().parseName(State);
   if (Entity == nullptr)
     return nullptr;
   First = parse_discriminator(First, Last);
   return make<LocalName>(Encoding, Entity);
 }
 
-// <unscoped-name> ::= [L]* <unqualified-name>
-//                 ::= St [L]* <unqualified-name>   # ::std::
+// <unscoped-name> ::= <unqualified-name>
+//                 ::= St <unqualified-name>   # ::std::
 // [*] extension
 template <typename Derived, typename Alloc>
 Node *
 AbstractManglingParser<Derived, Alloc>::parseUnscopedName(NameState *State,
                                                           bool *IsSubst) {
 
   Node *Std = nullptr;
   if (consumeIf("St")) {
     Std = make<NameType>("std");
     if (Std == nullptr)
       return nullptr;
   }
-  consumeIf('L');
 
   Node *Res = nullptr;
   ModuleName *Module = nullptr;
   if (look() == 'S') {
     Node *S = getDerived().parseSubstitution();
     if (!S)
       return nullptr;
     if (S->getKind() == Node::KModuleName)
       Module = static_cast<ModuleName *>(S);
     else if (IsSubst && Std == nullptr) {
       Res = S;
       *IsSubst = true;
     } else {
       return nullptr;
     }
   }
 
-  if (Res == nullptr)
+  if (Res == nullptr || Std != nullptr) {
     Res = getDerived().parseUnqualifiedName(State, Std, Module);
+  }
 
   return Res;
 }
 
-// <unqualified-name> ::= [<module-name>] <operator-name> [<abi-tags>]
+// <unqualified-name> ::= [<module-name>] L? <operator-name> [<abi-tags>]
 //                    ::= [<module-name>] <ctor-dtor-name> [<abi-tags>]
-//                    ::= [<module-name>] <source-name> [<abi-tags>]
-//                    ::= [<module-name>] <unnamed-type-name> [<abi-tags>]
+//                    ::= [<module-name>] L? <source-name> [<abi-tags>]
+//                    ::= [<module-name>] L? <unnamed-type-name> [<abi-tags>]
 //      # structured binding declaration
-//                    ::= [<module-name>] DC <source-name>+ E
+//                    ::= [<module-name>] L? DC <source-name>+ E
 template <typename Derived, typename Alloc>
 Node *AbstractManglingParser<Derived, Alloc>::parseUnqualifiedName(
     NameState *State, Node *Scope, ModuleName *Module) {
   if (getDerived().parseModuleNameOpt(Module))
     return nullptr;
 
+  consumeIf('L');
+
   Node *Result;
-  if (look() == 'U') {
-    Result = getDerived().parseUnnamedTypeName(State);
-  } else if (look() >= '1' && look() <= '9') {
+  if (look() >= '1' && look() <= '9') {
     Result = getDerived().parseSourceName(State);
+  } else if (look() == 'U') {
+    Result = getDerived().parseUnnamedTypeName(State);
   } else if (consumeIf("DC")) {
     // Structured binding
     size_t BindingsBegin = Names.size();
     do {
       Node *Binding = getDerived().parseSourceName(State);
       if (Binding == nullptr)
         return nullptr;
       Names.push_back(Binding);
if (look() == 'D' && (look(1) == '0' || look(1) == '1' || look(1) == '2' ||
     First += 2;
     if (State) State->CtorDtorConversion = true;
     return make<CtorDtorName>(SoFar, /*IsDtor=*/true, Variant);
   }
 
   return nullptr;
 }
 
-// <nested-name> ::= N [<CV-Qualifiers>] [<ref-qualifier>] <prefix> <unqualified-name> E
-//               ::= N [<CV-Qualifiers>] [<ref-qualifier>] <template-prefix> <template-args> E
+// <nested-name> ::= N [<CV-Qualifiers>] [<ref-qualifier>] <prefix>
+//      <unqualified-name> E
+//               ::= N [<CV-Qualifiers>] [<ref-qualifier>] <template-prefix>
+//                <template-args> E
 //
-// <prefix> ::= <prefix> [L]* <unqualified-name>
+// <prefix> ::= <prefix> <unqualified-name>
 //          ::= <template-prefix> <template-args>
 //          ::= <template-param>
 //          ::= <decltype>
 //          ::= # empty
 //          ::= <substitution>
 //          ::= <prefix> <data-member-prefix>
 // [*] extension
 //
if (look() == 'T') {
       SoFar = make<NameWithTemplateArgs>(SoFar, TA);
     } else if (look() == 'D' && (look(1) == 't' || look(1) == 'T')) {
       //          ::= <decltype>
       if (SoFar != nullptr)
         return nullptr; // Cannot have a prefix.
       SoFar = getDerived().parseDecltype();
     } else {
       ModuleName *Module = nullptr;
-      bool IsLocal = consumeIf('L'); // extension
 
       if (look() == 'S') {
         //          ::= <substitution>
         Node *S = nullptr;
         if (look(1) == 't') {
           First += 2;
           S = make<NameType>("std");
         } else {
           S = getDerived().parseSubstitution();
         }
         if (!S)
           return nullptr;
         if (S->getKind() == Node::KModuleName) {
           Module = static_cast<ModuleName *>(S);
-        } else if (SoFar != nullptr || IsLocal) {
+        } else if (SoFar != nullptr) {
           return nullptr; // Cannot have a prefix.
         } else {
           SoFar = S;
           continue; // Do not push a new substitution.
         }
       }
 
       //          ::= [<prefix>] <unqualified-name>

libcxxabi/test/test_demangle.pass.cpp

//    {"_Z1fM1XRFivEMS_OFivEMS_KOFivE", "f(int (X::*)() &, int (X::*)() &&, int (X::*)() const &&)"},
     {"_ZNStW3STD9allocatorI4PoohE1MEPS1_", "std::allocator@STD<Pooh>::M(Pooh*)"},
     {"_ZNStW3STD9allocatorI4PoohE1NI6PigletEEPT_PS1_", "Piglet* std::allocator@STD<Pooh>::N<Piglet>(Pooh*)"},
     {"_ZW3FooDC1a1bE", "[a, b]@Foo"},
     {"_ZN1NW3FooDC1a1bEE", "N::[a, b]@Foo"},
     {"_ZN3NMSW3MOD3FooB3ABIEv", "NMS::Foo@MOD[abi:ABI]()"},
     {"_ZGIW3Foo", "initializer for module Foo"},
     {"_ZGIW3FooW3Bar", "initializer for module Foo.Bar"},
     {"_ZGIW3FooWP3BarW3Baz", "initializer for module Foo:Bar.Baz"},
+    {"_ZW1ML4Oink", "Oink@M"},
+    {"_ZW1ML1fi", "f@M(int)"},
 };
 
 const unsigned N = sizeof(cases) / sizeof(cases[0]);
 
 struct FPLiteralCase {
   const char* mangled;
   // There are four possible demanglings of a given float.
   std::string expecting[4];

llvm/include/llvm/Demangle/ItaniumDemangle.h

Node *AbstractManglingParser<Derived, Alloc>::parseLocalName(NameState *State) {
 
   Node *Entity = getDerived().parseName(State);
   if (Entity == nullptr)
     return nullptr;
   First = parse_discriminator(First, Last);
   return make<LocalName>(Encoding, Entity);
 }
 
-// <unscoped-name> ::= [L]* <unqualified-name>
-//                 ::= St [L]* <unqualified-name>   # ::std::
+// <unscoped-name> ::= <unqualified-name>
+//                 ::= St <unqualified-name>   # ::std::
 // [*] extension
 template <typename Derived, typename Alloc>
 Node *
 AbstractManglingParser<Derived, Alloc>::parseUnscopedName(NameState *State,
                                                           bool *IsSubst) {
 
   Node *Std = nullptr;
   if (consumeIf("St")) {
     Std = make<NameType>("std");
     if (Std == nullptr)
       return nullptr;
   }
-  consumeIf('L');
 
   Node *Res = nullptr;
   ModuleName *Module = nullptr;
   if (look() == 'S') {
     Node *S = getDerived().parseSubstitution();
     if (!S)
       return nullptr;
     if (S->getKind() == Node::KModuleName)
       Module = static_cast<ModuleName *>(S);
     else if (IsSubst && Std == nullptr) {
       Res = S;
       *IsSubst = true;
     } else {
       return nullptr;
     }
   }
 
-  if (Res == nullptr)
+  if (Res == nullptr || Std != nullptr) {
     Res = getDerived().parseUnqualifiedName(State, Std, Module);
+  }
 
   return Res;
 }
 
-// <unqualified-name> ::= [<module-name>] <operator-name> [<abi-tags>]
+// <unqualified-name> ::= [<module-name>] L? <operator-name> [<abi-tags>]
 //                    ::= [<module-name>] <ctor-dtor-name> [<abi-tags>]
-//                    ::= [<module-name>] <source-name> [<abi-tags>]
-//                    ::= [<module-name>] <unnamed-type-name> [<abi-tags>]
+//                    ::= [<module-name>] L? <source-name> [<abi-tags>]
+//                    ::= [<module-name>] L? <unnamed-type-name> [<abi-tags>]
 //      # structured binding declaration
-//                    ::= [<module-name>] DC <source-name>+ E
+//                    ::= [<module-name>] L? DC <source-name>+ E
 template <typename Derived, typename Alloc>
 Node *AbstractManglingParser<Derived, Alloc>::parseUnqualifiedName(
     NameState *State, Node *Scope, ModuleName *Module) {
   if (getDerived().parseModuleNameOpt(Module))
     return nullptr;
 
+  consumeIf('L');
+
   Node *Result;
-  if (look() == 'U') {
-    Result = getDerived().parseUnnamedTypeName(State);
-  } else if (look() >= '1' && look() <= '9') {
+  if (look() >= '1' && look() <= '9') {
     Result = getDerived().parseSourceName(State);
+  } else if (look() == 'U') {
+    Result = getDerived().parseUnnamedTypeName(State);
   } else if (consumeIf("DC")) {
     // Structured binding
     size_t BindingsBegin = Names.size();
     do {
       Node *Binding = getDerived().parseSourceName(State);
       if (Binding == nullptr)
         return nullptr;
       Names.push_back(Binding);
if (look() == 'D' && (look(1) == '0' || look(1) == '1' || look(1) == '2' ||
     First += 2;
     if (State) State->CtorDtorConversion = true;
     return make<CtorDtorName>(SoFar, /*IsDtor=*/true, Variant);
   }
 
   return nullptr;
 }
 
-// <nested-name> ::= N [<CV-Qualifiers>] [<ref-qualifier>] <prefix> <unqualified-name> E
-//               ::= N [<CV-Qualifiers>] [<ref-qualifier>] <template-prefix> <template-args> E
+// <nested-name> ::= N [<CV-Qualifiers>] [<ref-qualifier>] <prefix>
+//      <unqualified-name> E
+//               ::= N [<CV-Qualifiers>] [<ref-qualifier>] <template-prefix>
+//                <template-args> E
 //
-// <prefix> ::= <prefix> [L]* <unqualified-name>
+// <prefix> ::= <prefix> <unqualified-name>
 //          ::= <template-prefix> <template-args>
 //          ::= <template-param>
 //          ::= <decltype>
 //          ::= # empty
 //          ::= <substitution>
 //          ::= <prefix> <data-member-prefix>
 // [*] extension
 //
if (look() == 'T') {
       SoFar = make<NameWithTemplateArgs>(SoFar, TA);
     } else if (look() == 'D' && (look(1) == 't' || look(1) == 'T')) {
       //          ::= <decltype>
       if (SoFar != nullptr)
         return nullptr; // Cannot have a prefix.
       SoFar = getDerived().parseDecltype();
     } else {
       ModuleName *Module = nullptr;
-      bool IsLocal = consumeIf('L'); // extension
 
       if (look() == 'S') {
         //          ::= <substitution>
         Node *S = nullptr;
         if (look(1) == 't') {
           First += 2;
           S = make<NameType>("std");
         } else {
           S = getDerived().parseSubstitution();
         }
         if (!S)
           return nullptr;
         if (S->getKind() == Node::KModuleName) {
           Module = static_cast<ModuleName *>(S);
-        } else if (SoFar != nullptr || IsLocal) {
+        } else if (SoFar != nullptr) {
           return nullptr; // Cannot have a prefix.
         } else {
           SoFar = S;
           continue; // Do not push a new substitution.
         }
       }
 
       //          ::= [<prefix>] <unqualified-name>