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

175

Not sure if I need to lookup in MangledNameMap

[• espindola]

llvm/lib/Object/RecordStreamer.cpp

151

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

153

Use an explicit type instead of auto.

[vitalybuka]

llvm/lib/Object/RecordStreamer.cpp

151

I moved some code in D44276

[vitalybuka]

auto -> explicit types

[vitalybuka]

rebase on D44283

[vitalybuka]

rebase

[pcc]

llvm/lib/Object/RecordStreamer.cpp

156

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

181

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;
 }
 
 MCSymbol *RecordStreamer::createELFSymverAlias(StringRef AliasName,
                                                const MCSymbol *Aliasee) {
-  // TODO: Handle "@@@". Depending on SymbolAttribute value it needs to be
-  // converted into @ or @@.
+  auto Split = AliasName.split("@@@");
+  SmallString<64> NewName;
+  if (!Split.second.empty()) {
+    if (Split.second.startswith("@"))
+      return nullptr; // Unexpected 4th @.
+
+    // Special processing for "@@@" according
+    // https://sourceware.org/binutils/docs/as/Symver.html
+    bool IsDefined = false;
+    // First check if the aliasee was recorded in the asm.
+    switch (getSymbolState(Aliasee)) {
+    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 (!IsDefined) {
+      // If we don't have it in assembly, then check if the aliasee was
+      // defined in the IR.
+      const auto *GV = M.getNamedValue(Aliasee->getName());
+      if (!GV) {
+        auto MI = MangledNameMap.find(Aliasee->getName());
+        if (MI != MangledNameMap.end())
+          GV = MI->second;
+      }
+      IsDefined = GV && GV->isDefinitionExact();
+    }
+    const char *Separator = IsDefined ? "@@" : "@";
+    AliasName =
+        (Split.first + Separator + Split.second).toStringRef(NewName);
+  }
   MCSymbol *Alias = MCStreamer::createELFSymverAlias(AliasName, Aliasee);
   SymverAliasMap[Aliasee].push_back(Alias);
   return Alias;
 }
 
 void RecordStreamer::EmitSymverAttributes() {
   // Walk all the recorded .symver aliases, and set up the binding
   // for each alias.

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 "_start1:"
+module asm ".symver _start1, foo@@@SOME_VERSION1"
+module asm ".symver _start2, foo@@@SOME_VERSION2"
+module asm ".local _start1"
+
+; CHECK-DAG: t _start1
+; CHECK-DAG: U _start2
+; CHECK-DAG: t foo@@SOME_VERSION1
+; CHECK-DAG: t foo@SOME_VERSION2