Object: Fix handling of @@@ in .symver directive

[vitalybuka]

Summary

name@@@nodename is going to be replaced with name@@nodename if symbols is
defined in the assembled file, or name@nodename if undefined.
https://sourceware.org/binutils/docs/as/Symver.html

Fixes PR36623

[vitalybuka]

llvm/lib/Object/RecordStreamer.cpp

147

Not sure if I need to lookup in MangledNameMap

[• espindola]

llvm/lib/Object/RecordStreamer.cpp

125–126

Use an explicit type instead of auto.

125–126

What version are you using? I don't see createELFSymverAlias on master.

[vitalybuka]

llvm/lib/Object/RecordStreamer.cpp

125–126

I moved some code in D44276

[vitalybuka]

auto -> explicit types

[vitalybuka]

rebase on D44283

[vitalybuka]

rebase

[pcc]

llvm/lib/Object/RecordStreamer.cpp

130

I think you need to defer this processing until the end to handle the case where a .symver appears before an inline asm definition.

155

This line should probably be IsDefined = GV && !GV->isDeclarationForLinker().

[vitalybuka]

clang-format

[vitalybuka]

Deferred processing.

[vitalybuka]

fix

[• rafael]

LGTM

Thanks,
Rafael

Vitaly Buka via Phabricator via llvm-commits
<[e-mail]> writes:

vitalybuka updated this revision to Diff 137861.
vitalybuka added a comment.

fix

https://reviews.llvm.org/D44274

Files:

llvm/lib/Object/RecordStreamer.cpp
llvm/test/LTO/X86/symver-asm3.ll

Index: llvm/test/LTO/X86/symver-asm3.ll

• /dev/null

+++ llvm/test/LTO/X86/symver-asm3.ll
@@ -0,0 +1,21 @@
+; Test special handling of @@@.
+
+; RUN: llvm-as < %s >%t1
+; RUN: llvm-nm %t1 | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+module asm "_start1:"
+module asm ".symver _start1, foo@@@SOME_VERSION1"
+module asm ".symver _start2, foo@@@SOME_VERSION2"
+module asm ".symver _start3, foo@@@SOME_VERSION3"
+module asm "_start3:"
+module asm ".local _start1"
+module asm ".local _start3"
+
+; CHECK-DAG: t _start1
+; CHECK-DAG: U _start2
+; CHECK-DAG: t foo@@SOME_VERSION1
+; CHECK-DAG: t foo@SOME_VERSION2
+; CHECK-DAG: t foo@@SOME_VERSION3

Index: llvm/lib/Object/RecordStreamer.cpp

• llvm/lib/Object/RecordStreamer.cpp

+++ llvm/lib/Object/RecordStreamer.cpp
@@ -149,6 +149,7 @@

for (auto &Symver : SymverAliasMap) {
  const MCSymbol *Aliasee = Symver.first;
  MCSymbolAttr Attr = MCSA_Invalid;

+ bool IsDefined = false;

// First check if the aliasee binding was recorded in the asm.
RecordStreamer::State state = getSymbolState(Aliasee);

@@ -165,26 +166,53 @@

  break;
}

• // If we don't have a symbol attribute from assembly, then check if
• // the aliasee was defined in the IR.
• if (Attr == MCSA_Invalid) {
• const auto *GV = M.getNamedValue(Aliasee->getName());

+ switch (state) {
+ case RecordStreamer::Defined:
+ case RecordStreamer::DefinedGlobal:
+ case RecordStreamer::DefinedWeak:
+ IsDefined = true;
+ break;
+ case RecordStreamer::NeverSeen:
+ case RecordStreamer::Global:
+ case RecordStreamer::Used:
+ case RecordStreamer::UndefinedWeak:
+ break;
+ }
+
+ const GlobalValue *GV = nullptr;
+ if (Attr == MCSA_Invalid || !IsDefined) {
+ GV = M.getNamedValue(Aliasee->getName());

if (!GV) {
  auto MI = MangledNameMap.find(Aliasee->getName());
  if (MI != MangledNameMap.end())
    GV = MI->second;
}
if (GV) {

• if (GV->hasExternalLinkage())
• Attr = MCSA_Global;
• else if (GV->hasLocalLinkage())
• Attr = MCSA_Local;
• else if (GV->isWeakForLinker())
• Attr = MCSA_Weak;

+ If we don't have a symbol attribute from assembly, then check if
+ the aliasee was defined in the IR.
+ if (Attr == MCSA_Invalid) {
+ if (GV->hasExternalLinkage())
+ Attr = MCSA_Global;
+ else if (GV->hasLocalLinkage())
+ Attr = MCSA_Local;
+ else if (GV->isWeakForLinker())
+ Attr = MCSA_Weak;
+ }
+ IsDefined = IsDefined || !GV->isDeclarationForLinker();

  }
}

+

// Set the detected binding on each alias with this aliasee.
for (auto AliasName : Symver.second) {

+ std::pair<StringRef, StringRef> Split = AliasName.split("@@@");
+ SmallString<128> NewName;
+ if (!Split.second.empty() && !Split.second.startswith("@")) {
+ Special processing for "@@@" according
+ https://sourceware.org/binutils/docs/as/Symver.html
+ const char *Separator = IsDefined ? "@@" : "@";
+ AliasName =
+ (Split.first + Separator + Split.second).toStringRef(NewName);
+ }

MCSymbol *Alias = getContext().getOrCreateSymbol(AliasName);
// TODO: Handle "@@@". Depending on SymbolAttribute value it needs to be
// converted into @ or @@.

---

llvm-commits mailing list
[e-mail]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

[vitalybuka]

Peter, could you please take a look?

[pcc]

llvm/test/LTO/X86/symver-asm3.ll

9

I'd also add a test where the definition is in IR.

21

Is this right? I thought it should be U foo@SOME_VERSION2.

[vitalybuka]

added IR test

[vitalybuka]

llvm/test/LTO/X86/symver-asm3.ll

21

It should be, but I don't change this behavior. That's how EmitAssignment is already implemented.

I'll can try to fix this in another patch.

[pcc]

LGTM

[vitalybuka]

rebase

llvm/lib/Object/RecordStreamer.cpp

 }
 
 RecordStreamer::State RecordStreamer::getSymbolState(const MCSymbol *Sym) {
   auto SI = Symbols.find(Sym->getName());
   if (SI == Symbols.end())
     return NeverSeen;
   return SI->second;
 }
 
 void RecordStreamer::emitELFSymverDirective(StringRef AliasName,
                                             const MCSymbol *Aliasee) {
   SymverAliasMap[Aliasee].push_back(AliasName);
 }
 
 void RecordStreamer::flushSymverDirectives() {
   // Mapping from mangled name to GV.
   StringMap<const GlobalValue *> MangledNameMap;
   // The name in the assembler will be mangled, but the name in the IR
   // might not, so we first compute a mapping from mangled name to GV.
   Mangler Mang;
   SmallString<64> MangledName;
   for (const GlobalValue &GV : M.global_values()) {
     if (!GV.hasName())
       continue;
     MangledName.clear();
     MangledName.reserve(GV.getName().size() + 1);
     Mang.getNameWithPrefix(MangledName, &GV, /*CannotUsePrivateLabel=*/false);
     MangledNameMap[MangledName] = &GV;
   }
 
   // Walk all the recorded .symver aliases, and set up the binding
   // for each alias.
   for (auto &Symver : SymverAliasMap) {
     const MCSymbol *Aliasee = Symver.first;
     MCSymbolAttr Attr = MCSA_Invalid;
+    bool IsDefined = false;
 
     // First check if the aliasee binding was recorded in the asm.
     RecordStreamer::State state = getSymbolState(Aliasee);
     switch (state) {
     case RecordStreamer::Global:
     case RecordStreamer::DefinedGlobal:
       Attr = MCSA_Global;
       break;
     case RecordStreamer::UndefinedWeak:
     case RecordStreamer::DefinedWeak:
       Attr = MCSA_Weak;
       break;
     default:
       break;
     }
 
-    // If we don't have a symbol attribute from assembly, then check if
-    // the aliasee was defined in the IR.
-    if (Attr == MCSA_Invalid) {
-      const auto *GV = M.getNamedValue(Aliasee->getName());
+    switch (state) {
+    case RecordStreamer::Defined:
+    case RecordStreamer::DefinedGlobal:
+    case RecordStreamer::DefinedWeak:
+      IsDefined = true;
+      break;
+    case RecordStreamer::NeverSeen:
+    case RecordStreamer::Global:
+    case RecordStreamer::Used:
+    case RecordStreamer::UndefinedWeak:
+      break;
+    }
+
+    if (Attr == MCSA_Invalid || !IsDefined) {
+      const GlobalValue *GV = M.getNamedValue(Aliasee->getName());
       if (!GV) {
         auto MI = MangledNameMap.find(Aliasee->getName());
         if (MI != MangledNameMap.end())
           GV = MI->second;
       }
       if (GV) {
+        // If we don't have a symbol attribute from assembly, then check if
+        // the aliasee was defined in the IR.
+        if (Attr == MCSA_Invalid) {
           if (GV->hasExternalLinkage())
             Attr = MCSA_Global;
           else if (GV->hasLocalLinkage())
             Attr = MCSA_Local;
           else if (GV->isWeakForLinker())
             Attr = MCSA_Weak;
         }
+        IsDefined = IsDefined || !GV->isDeclarationForLinker();
+      }
     }
+
     // Set the detected binding on each alias with this aliasee.
     for (auto AliasName : Symver.second) {
+      std::pair<StringRef, StringRef> Split = AliasName.split("@@@");
+      SmallString<128> NewName;
+      if (!Split.second.empty() && !Split.second.startswith("@")) {
+        // Special processing for "@@@" according
+        // https://sourceware.org/binutils/docs/as/Symver.html
+        const char *Separator = IsDefined ? "@@" : "@";
+        AliasName =
+            (Split.first + Separator + Split.second).toStringRef(NewName);
+      }
       MCSymbol *Alias = getContext().getOrCreateSymbol(AliasName);
       // TODO: Handle "@@@". Depending on SymbolAttribute value it needs to be
       // converted into @ or @@.
       const MCExpr *Value = MCSymbolRefExpr::create(Aliasee, getContext());
       EmitAssignment(Alias, Value);
       if (Attr != MCSA_Invalid)
         EmitSymbolAttribute(Alias, Attr);
     }
   }
 }

llvm/test/LTO/X86/symver-asm3.ll

+; Test special handling of @@@.
+
+; RUN: llvm-as < %s >%t1
+; RUN: llvm-nm %t1 | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+module asm "foo1:"
+; CHECK-DAG: t foo1
+
+module asm ".symver foo1, foo@@@VER1"
+; CHECK-DAG: t foo@@VER1
+
+module asm ".symver foo2, foo@@@VER2"
+; CHECK-DAG: U foo2
+; CHECK-DAG: t foo@VER2
+
+module asm ".symver foo3, foo@@@VER3"
+; CHECK-DAG: t foo@@VER3
+
+module asm ".symver foo4, foo@@@VER4"
+; CHECK-DAG: T foo@@VER4
+
+module asm ".symver foo5, foo@@@VER5"
+; CHECK-DAG: T foo@VER5
+
+module asm "foo3:"
+; CHECK-DAG: t foo3
+
+module asm ".local foo1"
+module asm ".local foo3"
+
+define void @foo4() {
+; CHECK-DAG: T foo4
+  ret void
+}
+
+declare void @foo5()
+; CHECK-DAG: U foo5