[llvm] r219829 - Correctly handle references to section symbols.
Rui Ueyama
ruiu at google.com
Thu Oct 16 11:27:30 PDT 2014
This patch seems to have broke out internal test. I'm still investigating,
but it looks like it made global object's constructors to be called twice
in some situation. Do you have any idea why this patch could cause such
behavior?
On Wed, Oct 15, 2014 at 11:55 AM, Rafael Espindola <
rafael.espindola at gmail.com> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141016/661cf740/attachment.html>
More information about the llvm-commits
mailing list