Only create an alias for @@@ once we know if it should use @ or @@

[vitalybuka]

LGTM, but this patch does not yet resolves the test from: https://reviews.llvm.org/D44274 . Is this expected?

lib/MC/ELFObjectWriter.cpp

402

= Rest.substr(Symbol.isUndefined() ? 2 : 1);

[• espindola]

In D44310#1033042, @vitalybuka wrote:

LGTM, but this patch does not yet resolves the test from: https://reviews.llvm.org/D44274 . Is this expected?

Yes, the idea was for this to be almost a pure cleanup.

r327160.

include/llvm/MC/MCAssembler.h

private:
 
   /// finishLayout - Finalize a layout, including fragment lowering.
   void finishLayout(MCAsmLayout &Layout);
 
   std::tuple<MCValue, uint64_t, bool>
   handleFixup(const MCAsmLayout &Layout, MCFragment &F, const MCFixup &Fixup);
 
 public:
+  std::vector<std::pair<StringRef, const MCSymbol *>> Symvers;
+
   /// Construct a new assembler instance.
   //
   // FIXME: How are we going to parameterize this? Two obvious options are stay
   // concrete and require clients to pass in a target like object. The other
   // option is to make this abstract, and have targets provide concrete
   // implementations as we do with AsmParser.
   MCAssembler(MCContext &Context, MCAsmBackend &Backend,
               MCCodeEmitter &Emitter, MCObjectWriter &Writer);

include/llvm/MC/MCELFStreamer.h

public:
   void EmitThumbFunc(MCSymbol *Func) override;
   void EmitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) override;
   bool EmitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override;
   void EmitSymbolDesc(MCSymbol *Symbol, unsigned DescValue) override;
   void EmitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                         unsigned ByteAlignment) override;
 
   void emitELFSize(MCSymbol *Symbol, const MCExpr *Value) override;
-  void emitELFSymverDirective(MCSymbol *Alias,
+  void emitELFSymverDirective(StringRef AliasName,
                               const MCSymbol *Aliasee) override;
 
   void EmitLocalCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                              unsigned ByteAlignment) override;
 
   void EmitZerofill(MCSection *Section, MCSymbol *Symbol = nullptr,
                     uint64_t Size = 0, unsigned ByteAlignment = 0) override;
   void EmitTBSSSymbol(MCSection *Section, MCSymbol *Symbol, uint64_t Size,

include/llvm/MC/MCStreamer.h

public:
   /// This corresponds to an assembler statement such as:
   ///  .size symbol, expression
   virtual void emitELFSize(MCSymbol *Symbol, const MCExpr *Value);
 
   /// \brief Emit an ELF .symver directive.
   ///
   /// This corresponds to an assembler statement such as:
   ///  .symver _start, foo@@SOME_VERSION
-  /// \param Alias - The versioned alias (i.e. "foo@@SOME_VERSION")
+  /// \param AliasName - The versioned alias (i.e. "foo@@SOME_VERSION")
   /// \param Aliasee - The aliased symbol (i.e. "_start")
-  virtual void emitELFSymverDirective(MCSymbol *Alias, const MCSymbol *Aliasee);
+  virtual void emitELFSymverDirective(StringRef AliasName,
+                                      const MCSymbol *Aliasee);
 
   /// \brief Emit a Linker Optimization Hint (LOH) directive.
   /// \param Args - Arguments of the LOH.
   virtual void EmitLOHDirective(MCLOHType Kind, const MCLOHArgs &Args) {}
 
   /// \brief Emit a common symbol.
   ///
   /// \param Symbol - The common symbol to emit.

lib/MC/ELFObjectWriter.cpp

class ELFObjectWriter : public MCObjectWriter {
   DenseMap<const MCSymbolELF *, const MCSymbolELF *> Renames;
 
   DenseMap<const MCSectionELF *, std::vector<ELFRelocationEntry>> Relocations;
 
   /// @}
   /// @name Symbol Table Data
   /// @{
 
-  BumpPtrAllocator Alloc;
-  StringSaver VersionSymSaver{Alloc};
   StringTableBuilder StrTabBuilder{StringTableBuilder::ELF};
 
   /// @}
 
   // This holds the symbol table index of the last local symbol.
   unsigned LastLocalSymbolIndex;
   // This holds the .strtab section index.
   unsigned StringTableIndex;
uint64_t ELFObjectWriter::SymbolValue(const MCSymbol &Sym,
 
   return Res;
 }
 
 void ELFObjectWriter::executePostLayoutBinding(MCAssembler &Asm,
                                                const MCAsmLayout &Layout) {
   // The presence of symbol versions causes undefined symbols and
   // versions declared with @@@ to be renamed.
-  for (const MCSymbol &A : Asm.symbols()) {
-    const auto &Alias = cast<MCSymbolELF>(A);
-    // Not an alias.
-    if (!Alias.isVariable())
-      continue;
-    auto *Ref = dyn_cast<MCSymbolRefExpr>(Alias.getVariableValue());
-    if (!Ref)
-      continue;
-    const auto &Symbol = cast<MCSymbolELF>(Ref->getSymbol());
-
-    StringRef AliasName = Alias.getName();
+  for (const std::pair<StringRef, const MCSymbol *> &P : Asm.Symvers) {
+    StringRef AliasName = P.first;
+    const auto &Symbol = cast<MCSymbolELF>(*P.second);
     size_t Pos = AliasName.find('@');
-    if (Pos == StringRef::npos)
-      continue;
+    assert(Pos != StringRef::npos);
+
+    StringRef Prefix = AliasName.substr(0, Pos);
+    StringRef Rest = AliasName.substr(Pos);
+    StringRef Tail = Rest;
+    if (Rest.startswith("@@@"))
+      Tail = Symbol.isUndefined() ? Rest.substr(2) : Rest.substr(1);
+
+    auto *Alias =
+        cast<MCSymbolELF>(Asm.getContext().getOrCreateSymbol(Prefix + Tail));
+    Asm.registerSymbol(*Alias);
+    const MCExpr *Value = MCSymbolRefExpr::create(&Symbol, Asm.getContext());
+    Alias->setVariableValue(Value);
 
     // Aliases defined with .symvar copy the binding from the symbol they alias.
     // This is the first place we are able to copy this information.
-    Alias.setExternal(Symbol.isExternal());
-    Alias.setBinding(Symbol.getBinding());
+    Alias->setExternal(Symbol.isExternal());
+    Alias->setBinding(Symbol.getBinding());
 
-    StringRef Rest = AliasName.substr(Pos);
     if (!Symbol.isUndefined() && !Rest.startswith("@@@"))
       continue;
 
     // FIXME: produce a better error message.
     if (Symbol.isUndefined() && Rest.startswith("@@") &&
         !Rest.startswith("@@@"))
       report_fatal_error("A @@ version cannot be undefined");
 
-    Renames.insert(std::make_pair(&Symbol, &Alias));
+    Renames.insert(std::make_pair(&Symbol, Alias));
   }
 }
 
 static uint8_t mergeTypeForSet(uint8_t origType, uint8_t newType) {
   uint8_t Type = newType;
 
   // Propagation rules:
   // IFUNC > FUNC > OBJECT > NOTYPE
if (Symbol.isAbsolute()) {
       const MCSectionELF &Section =
           static_cast<const MCSectionELF &>(Symbol.getSection());
       MSD.SectionIndex = SectionIndexMap.lookup(&Section);
       assert(MSD.SectionIndex && "Invalid section index!");
       if (MSD.SectionIndex >= ELF::SHN_LORESERVE)
         HasLargeSectionIndex = true;
     }
 
-    // The @@@ in symbol version is replaced with @ in undefined symbols and @@
-    // in defined ones.
-    //
-    // FIXME: All name handling should be done before we get to the writer,
-    // including dealing with GNU-style version suffixes.  Fixing this isn't
-    // trivial.
-    //
-    // We thus have to be careful to not perform the symbol version replacement
-    // blindly:
-    //
-    // The ELF format is used on Windows by the MCJIT engine.  Thus, on
-    // Windows, the ELFObjectWriter can encounter symbols mangled using the MS
-    // Visual Studio C++ name mangling scheme. Symbols mangled using the MSVC
-    // C++ name mangling can legally have "@@@" as a sub-string. In that case,
-    // the EFLObjectWriter should not interpret the "@@@" sub-string as
-    // specifying GNU-style symbol versioning. The ELFObjectWriter therefore
-    // checks for the MSVC C++ name mangling prefix which is either "?", "@?",
-    // "__imp_?" or "__imp_@?".
-    //
-    // It would have been interesting to perform the MS mangling prefix check
-    // only when the target triple is of the form *-pc-windows-elf. But, it
-    // seems that this information is not easily accessible from the
-    // ELFObjectWriter.
     StringRef Name = Symbol.getName();
-    SmallString<32> Buf;
-    if (!Name.startswith("?") && !Name.startswith("@?") &&
-        !Name.startswith("__imp_?") && !Name.startswith("__imp_@?")) {
-      // This symbol isn't following the MSVC C++ name mangling convention. We
-      // can thus safely interpret the @@@ in symbol names as specifying symbol
-      // versioning.
-      size_t Pos = Name.find("@@@");
-      if (Pos != StringRef::npos) {
-        Buf += Name.substr(0, Pos);
-        unsigned Skip = MSD.SectionIndex == ELF::SHN_UNDEF ? 2 : 1;
-        Buf += Name.substr(Pos + Skip);
-        Name = VersionSymSaver.save(Buf.c_str());
-      }
-    }
 
     // Sections have their own string table
     if (Symbol.getType() != ELF::STT_SECTION) {
       MSD.Name = Name;
       StrTabBuilder.add(Name);
     }
 
     if (Local)

lib/MC/MCAsmStreamer.cpp

void AddBlankLine() override {
     EmitEOL();
   }
 
   /// @name MCStreamer Interface
   /// @{
 
   void ChangeSection(MCSection *Section, const MCExpr *Subsection) override;
 
-  void emitELFSymverDirective(MCSymbol *Alias,
+  void emitELFSymverDirective(StringRef AliasName,
                               const MCSymbol *Aliasee) override;
 
   void EmitLOHDirective(MCLOHType Kind, const MCLOHArgs &Args) override;
   void EmitLabel(MCSymbol *Symbol, SMLoc Loc = SMLoc()) override;
 
   void EmitAssemblerFlag(MCAssemblerFlag Flag) override;
   void EmitLinkerOptions(ArrayRef<std::string> Options) override;
   void EmitDataRegion(MCDataRegionType Kind) override;
if (MCTargetStreamer *TS = getTargetStreamer()) {
     TS->changeSection(getCurrentSectionOnly(), Section, Subsection, OS);
   } else {
     Section->PrintSwitchToSection(
         *MAI, getContext().getObjectFileInfo()->getTargetTriple(), OS,
         Subsection);
   }
 }
 
-void MCAsmStreamer::emitELFSymverDirective(MCSymbol *Alias,
+void MCAsmStreamer::emitELFSymverDirective(StringRef AliasName,
                                            const MCSymbol *Aliasee) {
   OS << ".symver ";
   Aliasee->print(OS, MAI);
-  OS << ", ";
-  Alias->print(OS, MAI);
+  OS << ", " << AliasName;
   EmitEOL();
 }
 
 void MCAsmStreamer::EmitLabel(MCSymbol *Symbol, SMLoc Loc) {
   MCStreamer::EmitLabel(Symbol, Loc);
 
   Symbol->print(OS, MAI);
   OS << MAI->getLabelSuffix();

lib/MC/MCELFStreamer.cpp

void MCELFStreamer::EmitCommonSymbol(MCSymbol *S, uint64_t Size,
   cast<MCSymbolELF>(Symbol)
       ->setSize(MCConstantExpr::create(Size, getContext()));
 }
 
 void MCELFStreamer::emitELFSize(MCSymbol *Symbol, const MCExpr *Value) {
   cast<MCSymbolELF>(Symbol)->setSize(Value);
 }
 
-void MCELFStreamer::emitELFSymverDirective(MCSymbol *Alias,
+void MCELFStreamer::emitELFSymverDirective(StringRef AliasName,
                                            const MCSymbol *Aliasee) {
-  const MCExpr *Value = MCSymbolRefExpr::create(Aliasee, getContext());
-  EmitAssignment(Alias, Value);
+  getAssembler().Symvers.push_back({AliasName, Aliasee});
 }
 
 void MCELFStreamer::EmitLocalCommonSymbol(MCSymbol *S, uint64_t Size,
                                           unsigned ByteAlignment) {
   auto *Symbol = cast<MCSymbolELF>(S);
   // FIXME: Should this be caught and done earlier?
   getAssembler().registerSymbol(*Symbol);
   Symbol->setBinding(ELF::STB_LOCAL);

lib/MC/MCParser/ELFAsmParser.cpp

bool ELFAsmParser::ParseDirectiveSymver(StringRef, SMLoc) {
 
   StringRef AliasName;
   if (getParser().parseIdentifier(AliasName))
     return TokError("expected identifier in directive");
 
   if (AliasName.find('@') == StringRef::npos)
     return TokError("expected a '@' in the name");
 
-  MCSymbol *Alias = getContext().getOrCreateSymbol(AliasName);
   MCSymbol *Sym = getContext().getOrCreateSymbol(Name);
-  getStreamer().emitELFSymverDirective(Alias, Sym);
+  getStreamer().emitELFSymverDirective(AliasName, Sym);
   return false;
 }
 
 /// ParseDirectiveVersion
 ///  ::= .version string
 bool ELFAsmParser::ParseDirectiveVersion(StringRef, SMLoc) {
   if (getLexer().isNot(AsmToken::String))
     return TokError("unexpected token in '.version' directive");

lib/MC/MCStreamer.cpp

 void MCStreamer::EmitFileDirective(StringRef Filename) {}
 void MCStreamer::EmitCOFFSymbolStorageClass(int StorageClass) {
   llvm_unreachable("this directive only supported on COFF targets");
 }
 void MCStreamer::EmitCOFFSymbolType(int Type) {
   llvm_unreachable("this directive only supported on COFF targets");
 }
 void MCStreamer::emitELFSize(MCSymbol *Symbol, const MCExpr *Value) {}
-void MCStreamer::emitELFSymverDirective(MCSymbol *Alias,
+void MCStreamer::emitELFSymverDirective(StringRef AliasName,
                                         const MCSymbol *Aliasee) {}
 void MCStreamer::EmitLocalCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                                        unsigned ByteAlignment) {}
 void MCStreamer::EmitTBSSSymbol(MCSection *Section, MCSymbol *Symbol,
                                 uint64_t Size, unsigned ByteAlignment) {}
 void MCStreamer::ChangeSection(MCSection *, const MCExpr *) {}
 void MCStreamer::EmitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) {}
 void MCStreamer::EmitBytes(StringRef Data) {}

lib/Object/RecordStreamer.h

public:
   void EmitLabel(MCSymbol *Symbol, SMLoc Loc = SMLoc()) override;
   void EmitAssignment(MCSymbol *Symbol, const MCExpr *Value) override;
   bool EmitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override;
   void EmitZerofill(MCSection *Section, MCSymbol *Symbol, uint64_t Size,
                     unsigned ByteAlignment) override;
   void EmitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                         unsigned ByteAlignment) override;
   /// Record .symver aliases for later processing.
-  void emitELFSymverDirective(MCSymbol *Alias,
+  void emitELFSymverDirective(StringRef AliasName,
                               const MCSymbol *Aliasee) override;
   /// Return the map of .symver aliasee to associated aliases.
   DenseMap<const MCSymbol *, std::vector<MCSymbol *>> &symverAliases() {
     return SymverAliasMap;
   }
 
   /// Get the state recorded for the given symbol.
   State getSymbolState(const MCSymbol *Sym) {

lib/Object/RecordStreamer.cpp

 //===-- RecordStreamer.cpp - Record asm defined and used symbols ----------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "RecordStreamer.h"
+#include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCSymbol.h"
 
 using namespace llvm;
 
 void RecordStreamer::markDefined(const MCSymbol &Symbol) {
   State &S = Symbols[Symbol.getName()];
   switch (S) {
   case DefinedGlobal:
void RecordStreamer::EmitZerofill(MCSection *Section, MCSymbol *Symbol,
   markDefined(*Symbol);
 }
 
 void RecordStreamer::EmitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
                                       unsigned ByteAlignment) {
   markDefined(*Symbol);
 }
 
-void RecordStreamer::emitELFSymverDirective(MCSymbol *Alias,
+void RecordStreamer::emitELFSymverDirective(StringRef AliasName,
                                             const MCSymbol *Aliasee) {
+  MCSymbol *Alias = getContext().getOrCreateSymbol(AliasName);
   const MCExpr *Value = MCSymbolRefExpr::create(Aliasee, getContext());
   EmitAssignment(Alias, Value);
   SymverAliasMap[Aliasee].push_back(Alias);
 }