From 70a1be3f76bdfdfa1ad5789a8f636308e1d16409 Mon Sep 17 00:00:00 2001 From: Rafael Espindola Date: Fri, 17 Oct 2014 01:06:02 +0000 Subject: [PATCH] Revert commit r219835 and r219829. Revert "Correctly handle references to section symbols." Revert "Allow forward references to section symbols." Rui found a regression I am debugging. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@220010 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/MC/MCContext.h | 6 --- lib/MC/ELFObjectWriter.cpp | 74 ++++++++++++++++++++------------ lib/MC/MCContext.cpp | 24 ----------- lib/MC/MCELFStreamer.cpp | 13 +----- test/MC/ELF/comdat.s | 2 +- test/MC/ELF/section-sym-err.s | 6 --- test/MC/ELF/section-sym.s | 91 ---------------------------------------- test/MC/ELF/section-sym2.s | 28 ------------- tools/llvm-readobj/ELFDumper.cpp | 3 +- 9 files changed, 52 insertions(+), 195 deletions(-) delete mode 100644 test/MC/ELF/section-sym-err.s delete mode 100644 test/MC/ELF/section-sym.s delete mode 100644 test/MC/ELF/section-sym2.s diff --git a/include/llvm/MC/MCContext.h b/include/llvm/MC/MCContext.h index f209448d1bf..69365a385a6 100644 --- a/include/llvm/MC/MCContext.h +++ b/include/llvm/MC/MCContext.h @@ -73,10 +73,6 @@ namespace llvm { /// Symbols - Bindings of names to symbols. SymbolTable Symbols; - /// ELF sections can have a corresponding symbol. This maps one to the - /// other. - DenseMap SectionSymbols; - /// A maping from a local label number and an instance count to a symbol. /// For example, in the assembly /// 1: @@ -235,8 +231,6 @@ namespace llvm { MCSymbol *GetOrCreateSymbol(StringRef Name); MCSymbol *GetOrCreateSymbol(const Twine &Name); - MCSymbol *getOrCreateSectionSymbol(const MCSectionELF &Section); - /// LookupSymbol - Get the symbol for \p Name, or null. MCSymbol *LookupSymbol(StringRef Name) const; MCSymbol *LookupSymbol(const Twine &Name) const; diff --git a/lib/MC/ELFObjectWriter.cpp b/lib/MC/ELFObjectWriter.cpp index d77f03f5281..4577f0130ed 100644 --- a/lib/MC/ELFObjectWriter.cpp +++ b/lib/MC/ELFObjectWriter.cpp @@ -81,13 +81,23 @@ public: struct ELFRelocationEntry { uint64_t Offset; // Where is the relocation. - const MCSymbol *Symbol; // The symbol to relocate with. + bool UseSymbol; // Relocate with a symbol, not the section. + union { + const MCSymbol *Symbol; // The symbol to relocate with. + const MCSectionData *Section; // The section to relocate with. + }; unsigned Type; // The type of the relocation. uint64_t Addend; // The addend to use. ELFRelocationEntry(uint64_t Offset, const MCSymbol *Symbol, unsigned Type, uint64_t Addend) - : Offset(Offset), Symbol(Symbol), Type(Type), Addend(Addend) {} + : Offset(Offset), UseSymbol(true), Symbol(Symbol), Type(Type), + Addend(Addend) {} + + ELFRelocationEntry(uint64_t Offset, const MCSectionData *Section, + unsigned Type, uint64_t Addend) + : Offset(Offset), UseSymbol(false), Section(Section), Type(Type), + Addend(Addend) {} }; class ELFObjectWriter : public MCObjectWriter { @@ -127,14 +137,6 @@ class ELFObjectWriter : public MCObjectWriter { // Support lexicographic sorting. bool operator<(const ELFSymbolData &RHS) const { - unsigned LHSType = MCELF::GetType(*SymbolData); - unsigned RHSType = MCELF::GetType(*RHS.SymbolData); - if (LHSType == ELF::STT_SECTION && RHSType != ELF::STT_SECTION) - return false; - if (LHSType != ELF::STT_SECTION && RHSType == ELF::STT_SECTION) - return true; - if (LHSType == ELF::STT_SECTION && RHSType == ELF::STT_SECTION) - return SectionIndex < RHS.SectionIndex; return Name < RHS.Name; } }; @@ -649,6 +651,22 @@ void ELFObjectWriter::WriteSymbolTable(MCDataFragment *SymtabF, WriteSymbol(Writer, MSD, Layout); } + // Write out a symbol table entry for each regular section. + for (MCAssembler::const_iterator i = Asm.begin(), e = Asm.end(); i != e; + ++i) { + const MCSectionELF &Section = + static_cast(i->getSection()); + if (Section.getType() == ELF::SHT_RELA || + Section.getType() == ELF::SHT_REL || + Section.getType() == ELF::SHT_STRTAB || + Section.getType() == ELF::SHT_SYMTAB || + Section.getType() == ELF::SHT_SYMTAB_SHNDX) + continue; + Writer.writeSymbol(0, ELF::STT_SECTION, 0, 0, ELF::STV_DEFAULT, + SectionIndexMap.lookup(&Section), false); + LastLocalSymbolIndex++; + } + for (unsigned i = 0, e = ExternalSymbolData.size(); i != e; ++i) { ELFSymbolData &MSD = ExternalSymbolData[i]; MCSymbolData &Data = *MSD.SymbolData; @@ -864,11 +882,8 @@ void ELFObjectWriter::RecordRelocation(const MCAssembler &Asm, if (!RelocateWithSymbol) { const MCSection *SecA = (SymA && !SymA->isUndefined()) ? &SymA->getSection() : nullptr; - auto *ELFSec = cast_or_null(SecA); - MCSymbol *SectionSymbol = - ELFSec ? Asm.getContext().GetOrCreateSymbol(ELFSec->getSectionName()) - : nullptr; - ELFRelocationEntry Rec(FixupOffset, SectionSymbol, Type, Addend); + const MCSectionData *SecAD = SecA ? &Asm.getSectionData(*SecA) : nullptr; + ELFRelocationEntry Rec(FixupOffset, SecAD, Type, Addend); Relocations[FixupSection].push_back(Rec); return; } @@ -1046,10 +1061,7 @@ ELFObjectWriter::computeSymbolTable(MCAssembler &Asm, const MCAsmLayout &Layout, Buf += Name.substr(Pos + Skip); Name = Buf; } - - // Sections have their own string table - if (MCELF::GetType(SD) != ELF::STT_SECTION) - MSD.Name = StrTabBuilder.add(Name); + MSD.Name = StrTabBuilder.add(Name); if (MSD.SectionIndex == ELF::SHN_UNDEF) UndefinedSymbolData.push_back(MSD); @@ -1067,11 +1079,9 @@ ELFObjectWriter::computeSymbolTable(MCAssembler &Asm, const MCAsmLayout &Layout, for (auto i = Asm.file_names_begin(), e = Asm.file_names_end(); i != e; ++i) FileSymbolData.push_back(StrTabBuilder.getOffset(*i)); - for (ELFSymbolData &MSD : LocalSymbolData) - MSD.StringIndex = MCELF::GetType(*MSD.SymbolData) == ELF::STT_SECTION - ? 0 - : StrTabBuilder.getOffset(MSD.Name); - for (ELFSymbolData &MSD : ExternalSymbolData) + for (ELFSymbolData& MSD : LocalSymbolData) + MSD.StringIndex = StrTabBuilder.getOffset(MSD.Name); + for (ELFSymbolData& MSD : ExternalSymbolData) MSD.StringIndex = StrTabBuilder.getOffset(MSD.Name); for (ELFSymbolData& MSD : UndefinedSymbolData) MSD.StringIndex = StrTabBuilder.getOffset(MSD.Name); @@ -1087,6 +1097,8 @@ ELFObjectWriter::computeSymbolTable(MCAssembler &Asm, const MCAsmLayout &Layout, for (unsigned i = 0, e = LocalSymbolData.size(); i != e; ++i) LocalSymbolData[i].SymbolData->setIndex(Index++); + Index += NumRegularSections; + for (unsigned i = 0, e = ExternalSymbolData.size(); i != e; ++i) ExternalSymbolData[i].SymbolData->setIndex(Index++); for (unsigned i = 0, e = UndefinedSymbolData.size(); i != e; ++i) @@ -1342,8 +1354,18 @@ void ELFObjectWriter::WriteRelocationsFragment(const MCAssembler &Asm, for (unsigned i = 0, e = Relocs.size(); i != e; ++i) { const ELFRelocationEntry &Entry = Relocs[e - i - 1]; - unsigned Index = - Entry.Symbol ? getSymbolIndexInSymbolTable(Asm, Entry.Symbol) : 0; + + unsigned Index; + if (Entry.UseSymbol) { + Index = getSymbolIndexInSymbolTable(Asm, Entry.Symbol); + } else { + const MCSectionData *Sec = Entry.Section; + if (Sec) + Index = Sec->getOrdinal() + FileSymbolData.size() + + LocalSymbolData.size() + 1; + else + Index = 0; + } if (is64Bit()) { write(*F, Entry.Offset); diff --git a/lib/MC/MCContext.cpp b/lib/MC/MCContext.cpp index ea6db142fd7..9d8a57e55f6 100644 --- a/lib/MC/MCContext.cpp +++ b/lib/MC/MCContext.cpp @@ -113,30 +113,6 @@ MCSymbol *MCContext::GetOrCreateSymbol(StringRef Name) { return Sym; } -MCSymbol *MCContext::getOrCreateSectionSymbol(const MCSectionELF &Section) { - MCSymbol *&Sym = SectionSymbols[&Section]; - if (Sym) - return Sym; - - StringRef Name = Section.getSectionName(); - - StringMapEntry &Entry = Symbols.GetOrCreateValue(Name); - MCSymbol *OldSym = Entry.getValue(); - if (OldSym && OldSym->isUndefined()) { - Sym = OldSym; - return OldSym; - } - - StringMapEntry *NameEntry = &UsedNames.GetOrCreateValue(Name); - NameEntry->setValue(true); - Sym = new (*this) MCSymbol(NameEntry->getKey(), /*isTemporary*/ false); - - if (!Entry.getValue()) - Entry.setValue(Sym); - - return Sym; -} - MCSymbol *MCContext::CreateSymbol(StringRef Name) { // Determine whether this is an assembler temporary or normal label, if used. bool isTemporary = false; diff --git a/lib/MC/MCELFStreamer.cpp b/lib/MC/MCELFStreamer.cpp index bdc4a8410fb..4ef22d00224 100644 --- a/lib/MC/MCELFStreamer.cpp +++ b/lib/MC/MCELFStreamer.cpp @@ -92,19 +92,10 @@ void MCELFStreamer::ChangeSection(const MCSection *Section, MCSectionData *CurSection = getCurrentSectionData(); if (CurSection && CurSection->isBundleLocked()) report_fatal_error("Unterminated .bundle_lock when changing a section"); - - MCAssembler &Asm = getAssembler(); - auto *SectionELF = static_cast(Section); - const MCSymbol *Grp = SectionELF->getGroup(); + const MCSymbol *Grp = static_cast(Section)->getGroup(); if (Grp) - Asm.getOrCreateSymbolData(*Grp); - + getAssembler().getOrCreateSymbolData(*Grp); this->MCObjectStreamer::ChangeSection(Section, Subsection); - MCSymbol *SectionSymbol = getContext().getOrCreateSectionSymbol(*SectionELF); - if (SectionSymbol->isUndefined()) { - EmitLabel(SectionSymbol); - MCELF::SetType(Asm.getSymbolData(*SectionSymbol), ELF::STT_SECTION); - } } void MCELFStreamer::EmitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) { diff --git a/test/MC/ELF/comdat.s b/test/MC/ELF/comdat.s index 47966755220..ae2cc6b228b 100644 --- a/test/MC/ELF/comdat.s +++ b/test/MC/ELF/comdat.s @@ -41,7 +41,7 @@ // CHECK-NEXT: Offset: 0x54 // CHECK-NEXT: Size: 12 // CHECK-NEXT: Link: 13 -// CHECK-NEXT: Info: 10 +// CHECK-NEXT: Info: 13 // CHECK-NEXT: AddressAlignment: 4 // CHECK-NEXT: EntrySize: 4 // CHECK-NEXT: } diff --git a/test/MC/ELF/section-sym-err.s b/test/MC/ELF/section-sym-err.s deleted file mode 100644 index 789fee7c422..00000000000 --- a/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/test/MC/ELF/section-sym.s b/test/MC/ELF/section-sym.s deleted file mode 100644 index 3b76d813fe7..00000000000 --- a/test/MC/ELF/section-sym.s +++ /dev/null @@ -1,91 +0,0 @@ -// RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - | llvm-readobj -s -t -r --expand-relocs | FileCheck %s - -.section foo, "aG", @progbits, f1, comdat -.section foo, "G", @progbits, f2, comdat -.section bar -.long foo - -// Test that the relocation points to the first section foo. - -// The first seciton foo has index 6 -// CHECK: Section { -// CHECK: Index: 6 -// CHECK-NEXT: Name: foo (28) -// CHECK-NEXT: Type: SHT_PROGBITS (0x1) -// CHECK-NEXT: Flags [ (0x202) -// CHECK-NEXT: SHF_ALLOC (0x2) -// CHECK-NEXT: SHF_GROUP (0x200) -// CHECK-NEXT: ] -// CHECK-NEXT: Address: 0x0 -// CHECK-NEXT: Offset: 0x50 -// CHECK-NEXT: Size: 0 -// CHECK-NEXT: Link: 0 -// CHECK-NEXT: Info: 0 -// CHECK-NEXT: AddressAlignment: 1 -// CHECK-NEXT: EntrySize: 0 -// CHECK-NEXT: } -// CHECK-NEXT: Section { -// CHECK-NEXT: Index: 7 -// CHECK-NEXT: Name: foo (28) -// CHECK-NEXT: Type: SHT_PROGBITS (0x1) -// CHECK-NEXT: Flags [ (0x200) -// CHECK-NEXT: SHF_GROUP (0x200) -// CHECK-NEXT: ] -// CHECK-NEXT: Address: 0x0 -// CHECK-NEXT: Offset: 0x50 -// CHECK-NEXT: Size: 0 -// CHECK-NEXT: Link: 0 -// CHECK-NEXT: Info: 0 -// CHECK-NEXT: AddressAlignment: 1 -// CHECK-NEXT: EntrySize: 0 -// CHECK-NEXT: } - -// The relocation points to symbol 6 -// CHECK: Relocations [ -// CHECK-NEXT: Section (9) .relabar { -// CHECK-NEXT: Relocation { -// CHECK-NEXT: Offset: 0x0 -// CHECK-NEXT: Type: R_X86_64_32 (10) -// CHECK-NEXT: Symbol: foo (6) -// CHECK-NEXT: Addend: 0x0 -// CHECK-NEXT: } -// CHECK-NEXT: } -// CHECK-NEXT: ] - - -// The symbol 6 corresponds section 6 -// CHECK: Symbols [ - -// symbol 0 -// CHECK-NOT: Name -// CHECK: Name: - -// symbol 1 -// CHECK-NOT: Name -// CHECK: Name: f1 - -// symbol 2 -// CHECK-NOT: Name -// CHECK: Name: f2 - -// symbol 3 -// CHECK-NOT: Name -// CHECK: Name: .text - -// symbol 4 -// CHECK-NOT: Name -// CHECK: Name: .data - -// symbol 5 -// CHECK-NOT: Name -// CHECK: Name: .bss - -// symbol 6 -// CHECK-NOT: Name -// CHECK: Name: foo -// CHECK: Section: foo (0x6) - -// symbol 7 -// CHECK-NOT: Name -// CHECK: Name: foo -// CHECK: Section: foo (0x7) diff --git a/test/MC/ELF/section-sym2.s b/test/MC/ELF/section-sym2.s deleted file mode 100644 index acdb7d9547d..00000000000 --- a/test/MC/ELF/section-sym2.s +++ /dev/null @@ -1,28 +0,0 @@ -// RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - | llvm-readobj -t -r --expand-relocs | FileCheck %s - -// Test that we can forward reference a section. - -mov .rodata, %rsi -.section .rodata - -// CHECK:Relocations [ -// CHECK: Section (2) .rela.text { -// CHECK: Relocation { -// CHECK: Offset: 0x4 -// CHECK: Type: R_X86_64_32S (11) -// CHECK: Symbol: .rodata -// CHECK: Addend: 0x0 -// CHECK: } -// CHECK: } -// CHECK:] - -// There is only one .rodata symbol - -// CHECK:Symbols [ -// CHECK-NOT: Name: .rodata -// CHECK: Name: .rodata -// CHECK-NEXT: Value: 0x0 -// CHECK-NEXT: Size: 0 -// CHECK-NEXT: Binding: Local (0x0) -// CHECK-NEXT: Type: Section (0x3) -// CHECK-NOT: Name: .rodata diff --git a/tools/llvm-readobj/ELFDumper.cpp b/tools/llvm-readobj/ELFDumper.cpp index d68c78682d2..6da3318c931 100644 --- a/tools/llvm-readobj/ELFDumper.cpp +++ b/tools/llvm-readobj/ELFDumper.cpp @@ -676,8 +676,7 @@ void ELFDumper::printRelocation(const Elf_Shdr *Sec, DictScope Group(W, "Relocation"); W.printHex("Offset", Rel.r_offset); W.printNumber("Type", RelocName, (int)Rel.getType(Obj->isMips64EL())); - W.printNumber("Symbol", SymbolName.size() > 0 ? SymbolName : "-", - Rel.getSymbol(Obj->isMips64EL())); + W.printString("Symbol", SymbolName.size() > 0 ? SymbolName : "-"); W.printHex("Addend", Rel.r_addend); } else { raw_ostream& OS = W.startLine(); -- 2.11.0