[llvm] r219829 - Correctly handle references to section symbols.
Bill Schmidt
wschmidt at linux.vnet.ibm.com
Thu Oct 16 17:34:21 PDT 2014
Hi Rafael,
We've had two tests failing on the clang-ppc64-elf-linux buildbot since
this patch went in.
******************** TEST (simple) 'fftbench' FAILED! ********************
******************** TEST (simple) 'tramp3d-v4' FAILED! ********************
Both appear to be segfaulting at run time.
Thanks,
Bill
On Wed, 2014-10-15 at 18:55 +0000, Rafael Espindola wrote:
> Author: rafael
> Date: Wed Oct 15 13:55:30 2014
> New Revision: 219829
>
> URL: http://llvm.org/viewvc/llvm-project?rev=219829&view=rev
> Log:
> Correctly handle references to section symbols.
>
> When processing assembly like
>
> .long .text
>
> we were creating a new undefined symbol .text. GAS on the other hand would
> handle that as a reference to the .text section.
>
> This patch implements that by creating the section symbols earlier so that
> they are visible during asm parsing.
>
> The patch also updates llvm-readobj to print the symbol number in the relocation
> dump so that the test can differentiate between two sections with the same name.
>
> Added:
> llvm/trunk/test/MC/ELF/section-sym-err.s
> llvm/trunk/test/MC/ELF/section-sym.s
> Modified:
> llvm/trunk/include/llvm/MC/MCContext.h
> llvm/trunk/lib/MC/ELFObjectWriter.cpp
> llvm/trunk/lib/MC/MCContext.cpp
> llvm/trunk/lib/MC/MCELFStreamer.cpp
> llvm/trunk/test/MC/ELF/comdat.s
> llvm/trunk/tools/llvm-readobj/ELFDumper.cpp
>
> Modified: llvm/trunk/include/llvm/MC/MCContext.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCContext.h?rev=219829&r1=219828&r2=219829&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/MC/MCContext.h (original)
> +++ llvm/trunk/include/llvm/MC/MCContext.h Wed Oct 15 13:55:30 2014
> @@ -73,6 +73,10 @@ namespace llvm {
> /// Symbols - Bindings of names to symbols.
> SymbolTable Symbols;
>
> + /// ELF sections can have a corresponding symbol. This maps one to the
> + /// other.
> + DenseMap<const MCSectionELF*, MCSymbol*> SectionSymbols;
> +
> /// A maping from a local label number and an instance count to a symbol.
> /// For example, in the assembly
> /// 1:
> @@ -231,6 +235,8 @@ 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;
>
> Modified: llvm/trunk/lib/MC/ELFObjectWriter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/ELFObjectWriter.cpp?rev=219829&r1=219828&r2=219829&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/ELFObjectWriter.cpp (original)
> +++ llvm/trunk/lib/MC/ELFObjectWriter.cpp Wed Oct 15 13:55:30 2014
> @@ -81,23 +81,13 @@ public:
>
> struct ELFRelocationEntry {
> uint64_t Offset; // Where is the relocation.
> - 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.
> - };
> + const MCSymbol *Symbol; // The symbol 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), 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) {}
> + : Offset(Offset), Symbol(Symbol), Type(Type), Addend(Addend) {}
> };
>
> class ELFObjectWriter : public MCObjectWriter {
> @@ -137,6 +127,14 @@ class ELFObjectWriter : public MCObjectW
>
> // 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;
> }
> };
> @@ -651,22 +649,6 @@ void ELFObjectWriter::WriteSymbolTable(M
> 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<const MCSectionELF&>(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;
> @@ -882,8 +864,11 @@ void ELFObjectWriter::RecordRelocation(c
> if (!RelocateWithSymbol) {
> const MCSection *SecA =
> (SymA && !SymA->isUndefined()) ? &SymA->getSection() : nullptr;
> - const MCSectionData *SecAD = SecA ? &Asm.getSectionData(*SecA) : nullptr;
> - ELFRelocationEntry Rec(FixupOffset, SecAD, Type, Addend);
> + auto *ELFSec = cast_or_null<MCSectionELF>(SecA);
> + MCSymbol *SectionSymbol =
> + ELFSec ? Asm.getContext().GetOrCreateSymbol(ELFSec->getSectionName())
> + : nullptr;
> + ELFRelocationEntry Rec(FixupOffset, SectionSymbol, Type, Addend);
> Relocations[FixupSection].push_back(Rec);
> return;
> }
> @@ -1061,7 +1046,10 @@ ELFObjectWriter::computeSymbolTable(MCAs
> Buf += Name.substr(Pos + Skip);
> Name = Buf;
> }
> - MSD.Name = StrTabBuilder.add(Name);
> +
> + // Sections have their own string table
> + if (MCELF::GetType(SD) != ELF::STT_SECTION)
> + MSD.Name = StrTabBuilder.add(Name);
>
> if (MSD.SectionIndex == ELF::SHN_UNDEF)
> UndefinedSymbolData.push_back(MSD);
> @@ -1079,9 +1067,11 @@ ELFObjectWriter::computeSymbolTable(MCAs
> 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 = StrTabBuilder.getOffset(MSD.Name);
> - for (ELFSymbolData& MSD : ExternalSymbolData)
> + for (ELFSymbolData &MSD : LocalSymbolData)
> + MSD.StringIndex = MCELF::GetType(*MSD.SymbolData) == ELF::STT_SECTION
> + ? 0
> + : StrTabBuilder.getOffset(MSD.Name);
> + for (ELFSymbolData &MSD : ExternalSymbolData)
> MSD.StringIndex = StrTabBuilder.getOffset(MSD.Name);
> for (ELFSymbolData& MSD : UndefinedSymbolData)
> MSD.StringIndex = StrTabBuilder.getOffset(MSD.Name);
> @@ -1097,8 +1087,6 @@ ELFObjectWriter::computeSymbolTable(MCAs
> 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)
> @@ -1354,18 +1342,8 @@ void ELFObjectWriter::WriteRelocationsFr
>
> for (unsigned i = 0, e = Relocs.size(); i != e; ++i) {
> const ELFRelocationEntry &Entry = Relocs[e - i - 1];
> -
> - 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;
> - }
> + unsigned Index =
> + Entry.Symbol ? getSymbolIndexInSymbolTable(Asm, Entry.Symbol) : 0;
>
> if (is64Bit()) {
> write(*F, Entry.Offset);
>
> Modified: llvm/trunk/lib/MC/MCContext.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCContext.cpp?rev=219829&r1=219828&r2=219829&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/MCContext.cpp (original)
> +++ llvm/trunk/lib/MC/MCContext.cpp Wed Oct 15 13:55:30 2014
> @@ -113,6 +113,23 @@ MCSymbol *MCContext::GetOrCreateSymbol(S
> return Sym;
> }
>
> +MCSymbol *MCContext::getOrCreateSectionSymbol(const MCSectionELF &Section) {
> + MCSymbol *&Sym = SectionSymbols[&Section];
> + if (Sym)
> + return Sym;
> +
> + StringRef Name = Section.getSectionName();
> + StringMapEntry<bool> *NameEntry = &UsedNames.GetOrCreateValue(Name);
> + NameEntry->setValue(true);
> + Sym = new (*this) MCSymbol(NameEntry->getKey(), /*isTemporary*/ false);
> +
> + StringMapEntry<MCSymbol*> &Entry = Symbols.GetOrCreateValue(Name);
> + 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;
>
> Modified: llvm/trunk/lib/MC/MCELFStreamer.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCELFStreamer.cpp?rev=219829&r1=219828&r2=219829&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/MCELFStreamer.cpp (original)
> +++ llvm/trunk/lib/MC/MCELFStreamer.cpp Wed Oct 15 13:55:30 2014
> @@ -92,10 +92,19 @@ void MCELFStreamer::ChangeSection(const
> MCSectionData *CurSection = getCurrentSectionData();
> if (CurSection && CurSection->isBundleLocked())
> report_fatal_error("Unterminated .bundle_lock when changing a section");
> - const MCSymbol *Grp = static_cast<const MCSectionELF *>(Section)->getGroup();
> +
> + MCAssembler &Asm = getAssembler();
> + auto *SectionELF = static_cast<const MCSectionELF *>(Section);
> + const MCSymbol *Grp = SectionELF->getGroup();
> if (Grp)
> - getAssembler().getOrCreateSymbolData(*Grp);
> + Asm.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) {
>
> Modified: llvm/trunk/test/MC/ELF/comdat.s
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ELF/comdat.s?rev=219829&r1=219828&r2=219829&view=diff
> ==============================================================================
> --- llvm/trunk/test/MC/ELF/comdat.s (original)
> +++ llvm/trunk/test/MC/ELF/comdat.s Wed Oct 15 13:55:30 2014
> @@ -41,7 +41,7 @@
> // CHECK-NEXT: Offset: 0x54
> // CHECK-NEXT: Size: 12
> // CHECK-NEXT: Link: 13
> -// CHECK-NEXT: Info: 13
> +// CHECK-NEXT: Info: 10
> // CHECK-NEXT: AddressAlignment: 4
> // CHECK-NEXT: EntrySize: 4
> // CHECK-NEXT: }
>
> Added: llvm/trunk/test/MC/ELF/section-sym-err.s
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ELF/section-sym-err.s?rev=219829&view=auto
> ==============================================================================
> --- llvm/trunk/test/MC/ELF/section-sym-err.s (added)
> +++ llvm/trunk/test/MC/ELF/section-sym-err.s Wed Oct 15 13:55:30 2014
> @@ -0,0 +1,6 @@
> +// 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
>
> Added: llvm/trunk/test/MC/ELF/section-sym.s
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ELF/section-sym.s?rev=219829&view=auto
> ==============================================================================
> --- llvm/trunk/test/MC/ELF/section-sym.s (added)
> +++ llvm/trunk/test/MC/ELF/section-sym.s Wed Oct 15 13:55:30 2014
> @@ -0,0 +1,91 @@
> +// 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)
>
> Modified: llvm/trunk/tools/llvm-readobj/ELFDumper.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-readobj/ELFDumper.cpp?rev=219829&r1=219828&r2=219829&view=diff
> ==============================================================================
> --- llvm/trunk/tools/llvm-readobj/ELFDumper.cpp (original)
> +++ llvm/trunk/tools/llvm-readobj/ELFDumper.cpp Wed Oct 15 13:55:30 2014
> @@ -676,7 +676,8 @@ void ELFDumper<ELFT>::printRelocation(co
> DictScope Group(W, "Relocation");
> W.printHex("Offset", Rel.r_offset);
> W.printNumber("Type", RelocName, (int)Rel.getType(Obj->isMips64EL()));
> - W.printString("Symbol", SymbolName.size() > 0 ? SymbolName : "-");
> + W.printNumber("Symbol", SymbolName.size() > 0 ? SymbolName : "-",
> + Rel.getSymbol(Obj->isMips64EL()));
> W.printHex("Addend", Rel.r_addend);
> } else {
> raw_ostream& OS = W.startLine();
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list