[llvm] r219829 - Correctly handle references to section symbols.
Rafael Espíndola
rafael.espindola at gmail.com
Thu Oct 16 18:17:02 PDT 2014
Sure, reverted in r220010.
On 16 October 2014 21:03, Rui Ueyama <ruiu at google.com> wrote:
> Rafael, can I ask you to roll it back? I'll be offline until late night.
>
> On Thu, Oct 16, 2014 at 5:34 PM, Bill Schmidt <wschmidt at linux.vnet.ibm.com>
> wrote:
>>
>> 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
>> >
>>
>>
>> _______________________________________________
>> 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