Handle section vs global name conflict.

This is a fix for PR26941.

When there is both a section and a global definition with the same
name, the global wins.

Section symbols are not added to the symbol table; section references
are left undefined and fixed up in the object writer unless they've
been satisfied by some other definition.

llvm-svn: 264649

5 files changed, 164 insertions, 22 deletions

diff --git a/llvm/include/llvm/MC/MCContext.h b/llvm/include/llvm/MC/MCContext.h
index a787664de2f..44d195c1072 100644
--- a/llvm/include/llvm/MC/MCContext.h
+++ b/llvm/include/llvm/MC/MCContext.h
@@ -96,7 +96,9 @@ namespace llvm {
     DenseMap<std::pair<unsigned, unsigned>, MCSymbol *> LocalSymbols;
 
     /// Keeps tracks of names that were used both for used declared and
-    /// artificial symbols.
+    /// artificial symbols. The value is "true" if the name has been used for a
+    /// non-section symbol (there can be at most one of those, plus an unlimited
+    /// number of section symbols with the same name).
     StringMap<bool, BumpPtrAllocator &> UsedNames;
 
     /// The next ID to dole out to an unnamed assembler temporary symbol with
diff --git a/llvm/lib/MC/ELFObjectWriter.cpp b/llvm/lib/MC/ELFObjectWriter.cpp
index 01b7148cea7..68c3cb13a1b 100644
--- a/llvm/lib/MC/ELFObjectWriter.cpp
+++ b/llvm/lib/MC/ELFObjectWriter.cpp
@@ -375,9 +375,24 @@ uint64_t ELFObjectWriter::SymbolValue(const MCSymbol &Sym,
 
 void ELFObjectWriter::executePostLayoutBinding(MCAssembler &Asm,
                                                const MCAsmLayout &Layout) {
+  // Section symbols are used as definitions for undefined symbols with matching
+  // names. If there are multiple sections with the same name, the first one is
+  // used.
+  for (const MCSection &Sec : Asm) {
+    const MCSymbol *Begin = Sec.getBeginSymbol();
+    if (!Begin)
+      continue;
+
+    const MCSymbol *Alias = Asm.getContext().lookupSymbol(Begin->getName());
+    if (!Alias || !Alias->isUndefined())
+      continue;
+
+    Renames.insert(
+        std::make_pair(cast<MCSymbolELF>(Alias), cast<MCSymbolELF>(Begin)));
+  }
+
   // 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.
diff --git a/llvm/lib/MC/MCContext.cpp b/llvm/lib/MC/MCContext.cpp
index 96e24de6b4d..e326738325a 100644
--- a/llvm/lib/MC/MCContext.cpp
+++ b/llvm/lib/MC/MCContext.cpp
@@ -124,19 +124,9 @@ MCSymbolELF *MCContext::getOrCreateSectionSymbol(const MCSectionELF &Section) {
     return Sym;
 
   StringRef Name = Section.getSectionName();
-
-  MCSymbol *&OldSym = Symbols[Name];
-  if (OldSym && OldSym->isUndefined()) {
-    Sym = cast<MCSymbolELF>(OldSym);
-    return Sym;
-  }
-
-  auto NameIter = UsedNames.insert(std::make_pair(Name, true)).first;
+  auto NameIter = UsedNames.insert(std::make_pair(Name, false)).first;
   Sym = new (&*NameIter, *this) MCSymbolELF(&*NameIter, /*isTemporary*/ false);
 
-  if (!OldSym)
-    OldSym = Sym;
-
   return Sym;
 }
 
@@ -192,9 +182,12 @@ MCSymbol *MCContext::createSymbol(StringRef Name, bool AlwaysAddSuffix,
       raw_svector_ostream(NewName) << NextUniqueID++;
     }
     auto NameEntry = UsedNames.insert(std::make_pair(NewName, true));
-    if (NameEntry.second) {
-      // Ok, we found a name. Have the MCSymbol object itself refer to the copy
-      // of the string that is embedded in the UsedNames entry.
+    if (NameEntry.second || !NameEntry.first->second) {
+      // Ok, we found a name.
+      // Mark it as used for a non-section symbol.
+      NameEntry.first->second = true;
+      // Have the MCSymbol object itself refer to the copy of the string that is
+      // embedded in the UsedNames entry.
       return createSymbolImpl(&*NameEntry.first, IsTemporary);
     }
     assert(IsTemporary && "Cannot rename non-temporary symbols");
diff --git a/llvm/test/MC/ELF/section-sym-err.s b/llvm/test/MC/ELF/section-sym-err.s
deleted file mode 100644
index 789fee7c422..00000000000
--- a/llvm/test/MC/ELF/section-sym-err.s
+++ /dev/null
@@ -1,6 +0,0 @@
-// RUN: not llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o %t.o 2>&1 | FileCheck %s
-
-.section foo
-foo:
-
-// CHECK: error: invalid symbol redefinition
diff --git a/llvm/test/MC/ELF/section-sym-redefine.s b/llvm/test/MC/ELF/section-sym-redefine.s
new file mode 100644
index 00000000000..1f6dd5723af
--- /dev/null
+++ b/llvm/test/MC/ELF/section-sym-redefine.s
@@ -0,0 +1,138 @@
+// RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - | llvm-readobj  -t -r --expand-relocs | FileCheck %s
+
+// Local symbol overriding section.
+.section x1,"a",@progbits
+.local  x1
+.comm   x1,4,4
+.long x1  // reloc: .bss + 0
+
+// Section declared after local. Local symbol wins.
+.local  x2
+.comm   x2,4,4
+.section x2,"a",@progbits
+.long x2  // reloc: .bss + 4
+
+// No overriding symbol.
+.section x3,"a",@progbits
+.long x3  // reloc: x3(section) + 0
+
+// Global vs section.
+.section x4,"a",@progbits
+.long 0
+.globl x4
+.section foo, "a", @progbits
+x4:
+.long 0
+.long x4  // reloc: x4(global) + 0
+
+// Global vs implicit section
+.globl .data
+.data:
+.long 42
+.long .data // reloc: .data(global) + 0
+
+// CHECK: Relocations [
+// CHECK:   Section (4) .relax1 {
+// CHECK:     Relocation {
+// CHECK:       Offset: 0x0
+// CHECK:       Type: R_X86_64_32 (10)
+// CHECK:       Symbol: .bss (3)
+// CHECK:       Addend: 0x0
+// CHECK:     }
+// CHECK:   }
+// CHECK:   Section (7) .relax2 {
+// CHECK:     Relocation {
+// CHECK:       Offset: 0x0
+// CHECK:       Type: R_X86_64_32 (10)
+// CHECK:       Symbol: .bss (3)
+// CHECK:       Addend: 0x4
+// CHECK:     }
+// CHECK:   }
+// CHECK:   Section (9) .relax3 {
+// CHECK:     Relocation {
+// CHECK:       Offset: 0x0
+// CHECK:       Type: R_X86_64_32 (10)
+// CHECK:       Symbol: x3 (4)
+// CHECK:       Addend: 0x0
+// CHECK:     }
+// CHECK:   }
+// CHECK:   Section (12) .relafoo {
+// CHECK:     Relocation {
+// CHECK:       Offset: 0x4
+// CHECK:       Type: R_X86_64_32 (10)
+// CHECK:       Symbol: x4 (6)
+// CHECK:       Addend: 0x0
+// CHECK:     }
+// CHECK:     Relocation {
+// CHECK:       Offset: 0xC
+// CHECK:       Type: R_X86_64_32 (10)
+// CHECK:       Symbol: .data (5)
+// CHECK:       Addend: 0x0
+// CHECK:     }
+// CHECK:   }
+// CHECK: ]
+// CHECK: Symbols [
+// CHECK:   Symbol {
+// CHECK:     Name:  (0)
+// CHECK:     Value: 0x0
+// CHECK:     Size: 0
+// CHECK:     Binding: Local (0x0)
+// CHECK:     Type: None (0x0)
+// CHECK:     Other: 0
+// CHECK:     Section: Undefined (0x0)
+// CHECK:   }
+// CHECK:   Symbol {
+// CHECK:     Name: x1 (67)
+// CHECK:     Value: 0x0
+// CHECK:     Size: 4
+// CHECK:     Binding: Local (0x0)
+// CHECK:     Type: Object (0x1)
+// CHECK:     Other: 0
+// CHECK:     Section: .bss (0x5)
+// CHECK:   }
+// CHECK:   Symbol {
+// CHECK:     Name: x2 (59)
+// CHECK:     Value: 0x4
+// CHECK:     Size: 4
+// CHECK:     Binding: Local (0x0)
+// CHECK:     Type: Object (0x1)
+// CHECK:     Other: 0
+// CHECK:     Section: .bss (0x5)
+// CHECK:   }
+// CHECK:   Symbol {
+// CHECK:     Name:  (0)
+// CHECK:     Value: 0x0
+// CHECK:     Size: 0
+// CHECK:     Binding: Local (0x0)
+// CHECK:     Type: Section (0x3)
+// CHECK:     Other: 0
+// CHECK:     Section: .bss (0x5)
+// CHECK:   }
+// CHECK:   Symbol {
+// CHECK:     Name:  (0)
+// CHECK:     Value: 0x0
+// CHECK:     Size: 0
+// CHECK:     Binding: Local (0x0)
+// CHECK:     Type: Section (0x3)
+// CHECK:     Other: 0
+// CHECK:     Section: x3 (0x8)
+// CHECK:   }
+// CHECK:   Symbol {
+// CHECK:     Name: .data (37)
+// CHECK:     Value: 0x8
+// CHECK:     Size: 0
+// CHECK:     Binding: Global (0x1)
+// CHECK:     Type: None (0x0)
+// CHECK:     Other: 0
+// CHECK:     Section: foo (0xB)
+// CHECK:   }
+// CHECK:   Symbol {
+// CHECK:     Name: x4 (43)
+// CHECK:     Value: 0x0
+// CHECK:     Size: 0
+// CHECK:     Binding: Global (0x1)
+// CHECK:     Type: None (0x0)
+// CHECK:     Other: 0
+// CHECK:     Section: foo (0xB)
+// CHECK:   }
+// CHECK: ]