[MC][MCParser] Mark local and weak symbols as such when parsing assembly text

[sdardis]

Summary

When parsing assembly text directives such as .local, .weak, the symbols were
not marked as such in the internal symbol table. This lead to different output
depending on whether MCAsmStreamer or MCELFStreamer was. This patch
corrects such behaviour.

[sdardis]

dsanders observed this behaviour in D23497. I've since reposted that patch as D24722.

[sdardis]

Expanded MCSymbol<ELF/COFF/MachO> to dump internal state. Expanded MCContext to be able to dump the symbol table. Added a new hidden option -asm-show-symbol-table that prints out the symbol table just before the AsmParser finishes running.

[vkalintiris]

We should split this in two separate review requests, ie. one that introduces symbol dumping and another that marks local and weak symbols. Can you also add a separate test file just for checking the newly introduced -asm-show-symbol-table option?

include/llvm/MC/MCContext.h

public:
     MCSymbol *lookupSymbol(const Twine &Name) const;
 
     /// getSymbols - Get a reference for the symbol table for clients that
     /// want to, for example, iterate over all symbols. 'const' because we
     /// still want any modifications to the table itself to use the MCContext
     /// APIs.
     const SymbolTable &getSymbols() const { return Symbols; }
 
+    /// dumpSymbols - Dump the symbols to stderr.
+    void dumpSymbols() const;
+
     /// @}
 
     /// \name Section Management
     /// @{
 
     enum : unsigned {
       /// Pass this value as the UniqueID during section creation to get the
       /// generic section with the given name and characteristics. The usual

include/llvm/MC/MCSymbol.h

 #ifndef LLVM_MC_MCSYMBOL_H
 #define LLVM_MC_MCSYMBOL_H
 
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/MC/MCFragment.h"
 #include "llvm/Support/Compiler.h"
+#include "llvm/Support/Debug.h"
 
 namespace llvm {
 class MCAsmInfo;
 class MCExpr;
 class MCSymbol;
 class MCFragment;
 class MCSection;
 class MCContext;

include/llvm/MC/MCSymbolCOFF.h

public:
   bool isSafeSEH() const {
     return getFlags() & SF_SafeSEH;
   }
   void setIsSafeSEH() const {
     modifyFlags(SF_SafeSEH, SF_SafeSEH);
   }
 
   static bool classof(const MCSymbol *S) { return S->isCOFF(); }
+
+  void dump() const {
+    dbgs() << ": Format: COFF, Type: " << Type;
+    if (isSafeSEH())
+      dbgs() << ", SafeSEH";
+    if (isWeakExternal())
+      dbgs() << ", Weak External";
+  }
 };
 }
 
 #endif

include/llvm/MC/MCSymbolELF.h

public:
   void setIsWeakrefUsedInReloc() const;
   bool isWeakrefUsedInReloc() const;
 
   void setIsSignature() const;
   bool isSignature() const;
 
   static bool classof(const MCSymbol *S) { return S->isELF(); }
 
+  void dump() const;
+
 private:
   void setIsBindingSet() const;
 };
 }
 
 #endif

include/llvm/MC/MCSymbolMachO.h

uint16_t getEncodedFlags(bool EncodeAsAltEntry) const {
     }
 
     if (EncodeAsAltEntry)
       Flags |= SF_AltEntry;
 
     return Flags;
   }
 
+  void dump() const {
+    dbgs() << ": Format: MachO";
+  }
+
   static bool classof(const MCSymbol *S) { return S->isMachO(); }
 };
 }
 
 #endif

lib/MC/MCContext.cpp

 }
 
 MCSymbol *MCContext::lookupSymbol(const Twine &Name) const {
   SmallString<128> NameSV;
   StringRef NameRef = Name.toStringRef(NameSV);
   return Symbols.lookup(NameRef);
 }
 
+void MCContext::dumpSymbols() const {
+  dbgs() << "Symbol table:\n";
+  for (const auto &TableEntry : getSymbols()) {
+    MCSymbol *Sym = TableEntry.getValue();
+    Sym->dump();
+    if (Sym->isELF()) {
+      cast<MCSymbolELF>(Sym)->dump();
+    } else if (Sym->isMachO()) {
+      cast<MCSymbolMachO>(Sym)->dump();
+    } else if (Sym->isCOFF()) {
+      cast<MCSymbolCOFF>(Sym)->dump();
+    } else {
+      dbgs() << "Type: Unset?";
+    }
+    dbgs() << "\n";
+  }
+}
 //===----------------------------------------------------------------------===//
 // Section Management
 //===----------------------------------------------------------------------===//
 
 MCSectionMachO *MCContext::getMachOSection(StringRef Segment, StringRef Section,
                                            unsigned TypeAndAttributes,
                                            unsigned Reserved2, SectionKind Kind,
                                            const char *BeginSymName) {

lib/MC/MCParser/AsmParser.cpp

 #include "llvm/MC/MCParser/MCTargetAsmParser.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/MC/MCSection.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/MCValue.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Debug.h"
 #include "llvm/Support/Dwarf.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>

 using namespace llvm;
 
 MCAsmParserSemaCallback::~MCAsmParserSemaCallback() {}
 
 static cl::opt<unsigned> AsmMacroMaxNestingDepth(
      "asm-macro-max-nesting-depth", cl::init(20), cl::Hidden,
      cl::desc("The maximum nesting depth allowed for assembly macros."));
 
+static cl::opt<bool> ShowAsmSymbols("asm-show-symbol-table", cl::init(false),
+      cl::Hidden, cl::desc("Display the internal symbol table after parsing."));
+
 namespace {
 
 /// \brief Helper types for tracking macro definitions.
 typedef std::vector<AsmToken> MCAsmMacroArgument;
 typedef std::vector<MCAsmMacroArgument> MCAsmMacroArguments;
 
 struct MCAsmMacroParameter {
   StringRef Name;
if (!NoFinalize) {
     }
   }
 
   // Finalize the output stream if there are no errors and if the client wants
   // us to.
   if (!HadError && !NoFinalize)
     Out.Finish();
 
+  // Display the internal symbol table if requested.
+  if (ShowAsmSymbols) { getContext().dumpSymbols(); }
+
   return HadError || getContext().hadError();
 }
 
 void AsmParser::checkForValidSection() {
   if (!ParsingInlineAsm && !getStreamer().getCurrentSection().first) {
     printError(getTok().getLoc(),
                "expected section directive before assembly directive");
     Out.InitSections(false);

lib/MC/MCParser/ELFAsmParser.cpp

if (getLexer().isNot(AsmToken::EndOfStatement)) {
     for (;;) {
       StringRef Name;
 
       if (getParser().parseIdentifier(Name))
         return TokError("expected identifier in directive");
 
       MCSymbol *Sym = getContext().getOrCreateSymbol(Name);
 
+      if (Attr == MCSA_Local) {
+        cast<MCSymbolELF>(Sym)->setBinding(ELF::STB_LOCAL);
+      } else if (Attr == MCSA_Weak) {
+        cast<MCSymbolELF>(Sym)->setBinding(ELF::STB_WEAK);
+      }
+
       getStreamer().EmitSymbolAttribute(Sym, Attr);
 
       if (getLexer().is(AsmToken::EndOfStatement))
         break;
 
       if (getLexer().isNot(AsmToken::Comma))
         return TokError("unexpected token in directive");
       Lex();

lib/MC/MCSymbolELF.cpp

 void MCSymbolELF::setIsBindingSet() const {
   uint32_t OtherFlags = getFlags() & ~(0x1 << ELF_BindingSet_Shift);
   setFlags(OtherFlags | (1 << ELF_BindingSet_Shift));
 }
 
 bool MCSymbolELF::isBindingSet() const {
   return getFlags() & (0x1 << ELF_BindingSet_Shift);
 }
+
+LLVM_DUMP_METHOD void MCSymbolELF::dump() const {
+  dbgs() << ": Format: ELF, Type: ";
+  switch (getType()) {
+  case ELF::STT_NOTYPE:
+    dbgs() << "STT_NOTYPE";
+    break;
+  case ELF::STT_OBJECT:
+    dbgs() << "STT_OBJECT";
+    break;
+  case ELF::STT_FUNC:
+    dbgs() << "STT_FUNC";
+    break;
+  case ELF::STT_SECTION:
+    dbgs() << "STT_SECTION";
+    break;
+  case ELF::STT_COMMON:
+    dbgs() << "STT_COMMON";
+    break;
+  case ELF::STT_TLS:
+    dbgs() << "STT_TLS";
+    break;
+  case ELF::STT_GNU_IFUNC:
+    dbgs() << "STT_GNU_IFUNC";
+    break;
+  }
+
+  if (isBindingSet()) {
+    switch (getBinding()) {
+    case ELF::STB_LOCAL:
+      dbgs() << ", Binding: STB_LOCAL";
+      break;
+    case ELF::STB_GLOBAL:
+      dbgs() << ", Binding: STB_GLOBAL";
+      break;
+    case ELF::STB_WEAK:
+      dbgs() << ", Binding: STB_WEAK";
+      break;
+    case ELF::STB_GNU_UNIQUE:
+      dbgs() << ", Binding: STB_GNU_UNIQUE";
+      break;
+    }
+  }
+}
 }

test/MC/ELF/common.s

-// RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - | llvm-readobj -t | FileCheck %s
+// RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - | llvm-readobj -t | FileCheck -check-prefix=OBJ %s
+// RUN: llvm-mc -triple x86_64-pc-linux-gnu %s -o - -asm-show-symbol-table 2>&1 | FileCheck -check-prefix=ASM %s
 
 
   .text
 
 // Test that this produces a regular local symbol.
   .type common1,@object
   .local  common1
   .comm common1,1,1
 
-// CHECK:        Symbol {
-// CHECK:          Name: common1
-// CHECK-NEXT:     Value: 0x0
-// CHECK-NEXT:     Size: 1
-// CHECK-NEXT:     Binding: Local
-// CHECK-NEXT:     Type: Object
-// CHECK-NEXT:     Other: 0
-// CHECK-NEXT:     Section:
-// CHECK-NEXT:   }
+// OBJ:        Symbol {
+// OBJ:          Name: common1
+// OBJ-NEXT:     Value: 0x0
+// OBJ-NEXT:     Size: 1
+// OBJ-NEXT:     Binding: Local
+// OBJ-NEXT:     Type: Object
+// OBJ-NEXT:     Other: 0
+// OBJ-NEXT:     Section:
+// OBJ-NEXT:   }
 
+// ASM: common1: Format: ELF, Type: STT_NOTYPE, Binding: STB_LOCAL
 
 // Same as common1, but with directives in a different order.
   .local  common2
   .type common2,@object
   .comm common2,1,1
 
-// CHECK:        Symbol {
-// CHECK:          Name: common2
-// CHECK-NEXT:     Value: 0x1
-// CHECK-NEXT:     Size: 1
-// CHECK-NEXT:     Binding: Local
-// CHECK-NEXT:     Type: Object
-// CHECK-NEXT:     Other: 0
-// CHECK-NEXT:     Section:
-// CHECK-NEXT:   }
+// OBJ:        Symbol {
+// OBJ:          Name: common2
+// OBJ-NEXT:     Value: 0x1
+// OBJ-NEXT:     Size: 1
+// OBJ-NEXT:     Binding: Local
+// OBJ-NEXT:     Type: Object
+// OBJ-NEXT:     Other: 0
+// OBJ-NEXT:     Section:
+// OBJ-NEXT:   }
 
+// ASM: common2: Format: ELF, Type: STT_NOTYPE, Binding: STB_LOCAL
 
         .local  common6
         .comm common6,8,16
 
-// CHECK:        Symbol {
-// CHECK:          Name: common6
-// CHECK-NEXT:     Value: 0x10
-// CHECK-NEXT:     Size: 8
-// CHECK-NEXT:     Binding: Local
-// CHECK-NEXT:     Type: Object
-// CHECK-NEXT:     Other: 0
-// CHECK-NEXT:     Section: .bss
-// CHECK-NEXT:   }
+// OBJ:        Symbol {
+// OBJ:          Name: common6
+// OBJ-NEXT:     Value: 0x10
+// OBJ-NEXT:     Size: 8
+// OBJ-NEXT:     Binding: Local
+// OBJ-NEXT:     Type: Object
+// OBJ-NEXT:     Other: 0
+// OBJ-NEXT:     Section: .bss
+// OBJ-NEXT:   }
 
+// ASM: common6: Format: ELF, Type: STT_NOTYPE, Binding: STB_LOCAL
 
 // Test that without an explicit .local we produce a global.
   .type common3,@object
   .comm common3,4,4
 
-// CHECK:        Symbol {
-// CHECK:          Name: common3
-// CHECK-NEXT:     Value: 0x4
-// CHECK-NEXT:     Size: 4
-// CHECK-NEXT:     Binding: Global
-// CHECK-NEXT:     Type: Object
-// CHECK-NEXT:     Other: 0
-// CHECK-NEXT:     Section: Common (0xFFF2)
-// CHECK-NEXT:   }
+// OBJ:        Symbol {
+// OBJ:          Name: common3
+// OBJ-NEXT:     Value: 0x4
+// OBJ-NEXT:     Size: 4
+// OBJ-NEXT:     Binding: Global
+// OBJ-NEXT:     Type: Object
+// OBJ-NEXT:     Other: 0
+// OBJ-NEXT:     Section: Common (0xFFF2)
+// OBJ-NEXT:   }
 
 
 // Test that without an explicit .local we produce a global, even if the first
 // occurrence is not in a directive.
   .globl  foo
   .type foo,@function
 foo:
   movsbl  common4+3(%rip), %eax
 
 
   .type common4,@object
   .comm common4,40,16
 
-// CHECK:        Symbol {
-// CHECK:          Name: common4
-// CHECK-NEXT:     Value: 0x10
-// CHECK-NEXT:     Size: 40
-// CHECK-NEXT:     Binding: Global
-// CHECK-NEXT:     Type: Object
-// CHECK-NEXT:     Other: 0
-// CHECK-NEXT:     Section: Common (0xFFF2)
-// CHECK-NEXT:   }
+// OBJ:        Symbol {
+// OBJ:          Name: common4
+// OBJ-NEXT:     Value: 0x10
+// OBJ-NEXT:     Size: 40
+// OBJ-NEXT:     Binding: Global
+// OBJ-NEXT:     Type: Object
+// OBJ-NEXT:     Other: 0
+// OBJ-NEXT:     Section: Common (0xFFF2)
+// OBJ-NEXT:   }
 
 
         .comm common5,4,4
 
-// CHECK:        Symbol {
-// CHECK:          Name: common5
-// CHECK-NEXT:     Value: 0x4
-// CHECK-NEXT:     Size: 4
-// CHECK-NEXT:     Binding: Global
-// CHECK-NEXT:     Type: Object
-// CHECK-NEXT:     Other: 0
-// CHECK-NEXT:     Section: Common (0xFFF2)
-// CHECK-NEXT:   }
+// OBJ:        Symbol {
+// OBJ:          Name: common5
+// OBJ-NEXT:     Value: 0x4
+// OBJ-NEXT:     Size: 4
+// OBJ-NEXT:     Binding: Global
+// OBJ-NEXT:     Type: Object
+// OBJ-NEXT:     Other: 0
+// OBJ-NEXT:     Section: Common (0xFFF2)
+// OBJ-NEXT:   }

test/MC/ELF/common2.s

-// RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - | llvm-readobj -s | FileCheck %s
+// RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - | llvm-readobj -s | FileCheck -check-prefix=OBJ %s
+// RUN: llvm-mc -triple x86_64-pc-linux-gnu %s -o - -asm-show-symbol-table 2>&1 | FileCheck -check-prefix=ASM %s
+
+
 
 // Test local common construction.
 // Unlike gas, common symbols are created when found, not at the end of .bss.
 // In this example it causes .bss to have size 8 instead of 9.
 
   .local  vimvardict
   .comm vimvardict,1,8
   .bss
         .zero 1
   .align  8
 
-// CHECK:        Section {
-// CHECK:          Name: .bss
-// CHECK-NEXT:     Type:
-// CHECK-NEXT:     Flags [
-// CHECK:          ]
-// CHECK-NEXT:     Address:
-// CHECK-NEXT:     Offset:
-// CHECK-NEXT:     Size: 8
-// CHECK-NEXT:     Link:
-// CHECK-NEXT:     Info:
-// CHECK-NEXT:     AddressAlignment:
-// CHECK-NEXT:     EntrySize:
-// CHECK-NEXT:   }
+// OBJ:        Section {
+// OBJ:          Name: .bss
+// OBJ-NEXT:     Type:
+// OBJ-NEXT:     Flags [
+// OBJ:          ]
+// OBJ-NEXT:     Address:
+// OBJ-NEXT:     Offset:
+// OBJ-NEXT:     Size: 8
+// OBJ-NEXT:     Link:
+// OBJ-NEXT:     Info:
+// OBJ-NEXT:     AddressAlignment:
+// OBJ-NEXT:     EntrySize:
+// OBJ-NEXT:   }
+
+// ASM: vimvardict: Format: ELF, Type: STT_NOTYPE, Binding: STB_LOCAL

test/MC/ELF/weak.s

-// RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - | llvm-readobj -t | FileCheck %s
+// RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - | llvm-readobj -t | FileCheck -check-prefix=OBJ %s
+// RUN: llvm-mc -triple x86_64-pc-linux-gnu %s -o - -asm-show-symbol-table 2>&1 | FileCheck -check-prefix=ASM %s
 
 // Test that this produces a weak undefined symbol.
 
   .weak foo
         .long   foo
 
 // And that bar is after all local symbols and has non-zero value.
         .weak bar
 bar:
 
-// CHECK:        Symbol {
-// CHECK:          Name: bar
-// CHECK-NEXT:     Value: 0x4
-// CHECK-NEXT:     Size: 0
-// CHECK-NEXT:     Binding: Weak
-// CHECK-NEXT:     Type: None
-// CHECK-NEXT:     Other: 0
-// CHECK-NEXT:     Section: .text
-// CHECK-NEXT:   }
-// CHECK:        Symbol {
-// CHECK:          Name: foo
-// CHECK-NEXT:     Value: 0x0
-// CHECK-NEXT:     Size: 0
-// CHECK-NEXT:     Binding: Weak
-// CHECK-NEXT:     Type: None
-// CHECK-NEXT:     Other: 0
-// CHECK-NEXT:     Section: Undefined (0x0)
-// CHECK-NEXT:   }
-// CHECK-NEXT:  ]
+// OBJ:        Symbol {
+// OBJ:          Name: bar
+// OBJ-NEXT:     Value: 0x4
+// OBJ-NEXT:     Size: 0
+// OBJ-NEXT:     Binding: Weak
+// OBJ-NEXT:     Type: None
+// OBJ-NEXT:     Other: 0
+// OBJ-NEXT:     Section: .text
+// OBJ-NEXT:   }
+// OBJ:        Symbol {
+// OBJ:          Name: foo
+// OBJ-NEXT:     Value: 0x0
+// OBJ-NEXT:     Size: 0
+// OBJ-NEXT:     Binding: Weak
+// OBJ-NEXT:     Type: None
+// OBJ-NEXT:     Other: 0
+// OBJ-NEXT:     Section: Undefined (0x0)
+// OBJ-NEXT:   }
+// OBJ-NEXT:  ]
+
+// Test that the symbols have the correct bindings.
+
+// ASM: foo: Format: ELF, Type: STT_NOTYPE, Binding: STB_WEAK
+// ASM: bar: Format: ELF, Type: STT_NOTYPE, Binding: STB_WEAK