From a023f79db19dae6c42ce4240e85630510c20ac91 Mon Sep 17 00:00:00 2001 From: Evgeniy Stepanov Date: Mon, 28 Mar 2016 20:36:28 +0000 Subject: [PATCH] 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 --- llvm/include/llvm/MC/MCContext.h | 4 +- llvm/lib/MC/ELFObjectWriter.cpp | 17 ++- llvm/lib/MC/MCContext.cpp | 21 ++-- llvm/test/MC/ELF/section-sym-err.s | 6 -- llvm/test/MC/ELF/section-sym-redefine.s | 138 ++++++++++++++++++++++++ 5 files changed, 164 insertions(+), 22 deletions(-) delete mode 100644 llvm/test/MC/ELF/section-sym-err.s create mode 100644 llvm/test/MC/ELF/section-sym-redefine.s diff --git a/llvm/include/llvm/MC/MCContext.h b/llvm/include/llvm/MC/MCContext.h index a787664de2fb..44d195c1072e 100644 --- a/llvm/include/llvm/MC/MCContext.h +++ b/llvm/include/llvm/MC/MCContext.h @@ -96,7 +96,9 @@ namespace llvm { DenseMap, 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 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 01b7148cea74..68c3cb13a1b8 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(Alias), cast(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(A); // Not an alias. diff --git a/llvm/lib/MC/MCContext.cpp b/llvm/lib/MC/MCContext.cpp index 96e24de6b4db..e326738325af 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(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 789fee7c422c..000000000000 --- 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 000000000000..1f6dd5723af1 --- /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: ]