From: Peter Collingbourne Date: Thu, 26 Nov 2015 23:29:27 +0000 (+0000) Subject: MC: Simplify handling of temporary symbols in COFF writer. X-Git-Url: http://plrg.eecs.uci.edu/git/?p=oota-llvm.git;a=commitdiff_plain;h=41984d3288cfbc1ad14510ac3a87f0f398b551da MC: Simplify handling of temporary symbols in COFF writer. The COFF object writer was previously adding unnecessary symbols to its temporary data structures and cleaning them up later. This made the code harder to understand and caused a bug (aliases classed as temporary symbols would cause an assertion failure). A much simpler way of handling such symbols is to ask the layout for their section-relative position when needed. Tested with a bootstrap on Windows and by building Chrome. Differential Revision: http://reviews.llvm.org/D14975 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@254183 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/MC/WinCOFFObjectWriter.cpp b/lib/MC/WinCOFFObjectWriter.cpp index 875a61425f1..59f06dcd113 100644 --- a/lib/MC/WinCOFFObjectWriter.cpp +++ b/lib/MC/WinCOFFObjectWriter.cpp @@ -78,8 +78,6 @@ public: COFFSymbol(StringRef name); void set_name_offset(uint32_t Offset); - bool should_keep() const; - int64_t getIndex() const { return Index; } void setIndex(int Value) { Index = Value; @@ -162,8 +160,6 @@ public: void SetSymbolName(COFFSymbol &S); void SetSectionName(COFFSection &S); - bool ExportSymbol(const MCSymbol &Symbol, MCAssembler &Asm); - bool IsPhysicalSection(COFFSection *S); // Entity writing methods. @@ -217,38 +213,6 @@ void COFFSymbol::set_name_offset(uint32_t Offset) { write_uint32_le(Data.Name + 4, Offset); } -/// logic to decide if the symbol should be reported in the symbol table -bool COFFSymbol::should_keep() const { - // no section means its external, keep it - if (!Section) - return true; - - // if it has relocations pointing at it, keep it - if (Relocations > 0) { - assert(Section->Number != -1 && "Sections with relocations must be real!"); - return true; - } - - // if this is a safeseh handler, keep it - if (MC && (cast(MC)->isSafeSEH())) - return true; - - // if the section its in is being droped, drop it - if (Section->Number == -1) - return false; - - // if it is the section symbol, keep it - if (Section->Symbol == this) - return true; - - // if its temporary, drop it - if (MC && MC->isTemporary()) - return false; - - // otherwise, keep it - return true; -} - //------------------------------------------------------------------------------ // Section class implementation @@ -394,7 +358,6 @@ void WinCOFFObjectWriter::DefineSymbol(const MCSymbol &Symbol, MCAssembler &Assembler, const MCAsmLayout &Layout) { COFFSymbol *coff_symbol = GetOrCreateCOFFSymbol(&Symbol); - SymbolMap[&Symbol] = coff_symbol; if (cast(Symbol).isWeakExternal()) { coff_symbol->Data.StorageClass = COFF::IMAGE_SYM_CLASS_WEAK_EXTERNAL; @@ -517,25 +480,6 @@ void WinCOFFObjectWriter::SetSymbolName(COFFSymbol &S) { std::memcpy(S.Data.Name, S.Name.c_str(), S.Name.size()); } -bool WinCOFFObjectWriter::ExportSymbol(const MCSymbol &Symbol, - MCAssembler &Asm) { - // This doesn't seem to be right. Strings referred to from the .data section - // need symbols so they can be linked to code in the .text section right? - - // return Asm.isSymbolLinkerVisible(Symbol); - - // Non-temporary labels should always be visible to the linker. - if (!Symbol.isTemporary()) - return true; - - // Temporary variable symbols are invisible. - if (Symbol.isVariable()) - return false; - - // Absolute temporary labels are never visible. - return !Symbol.isAbsolute(); -} - bool WinCOFFObjectWriter::IsPhysicalSection(COFFSection *S) { return (S->Header.Characteristics & COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA) == 0; @@ -665,7 +609,7 @@ void WinCOFFObjectWriter::executePostLayoutBinding(MCAssembler &Asm, defineSection(static_cast(Section)); for (const MCSymbol &Symbol : Asm.symbols()) - if (ExportSymbol(Symbol, Asm)) + if (!Symbol.isTemporary()) DefineSymbol(Symbol, Asm, Layout); } @@ -704,8 +648,7 @@ void WinCOFFObjectWriter::recordRelocation( const MCFixup &Fixup, MCValue Target, bool &IsPCRel, uint64_t &FixedValue) { assert(Target.getSymA() && "Relocation must reference a symbol!"); - const MCSymbol &Symbol = Target.getSymA()->getSymbol(); - const MCSymbol &A = Symbol; + const MCSymbol &A = Target.getSymA()->getSymbol(); if (!A.isRegistered()) { Asm.getContext().reportError(Fixup.getLoc(), Twine("symbol '") + A.getName() + @@ -724,11 +667,8 @@ void WinCOFFObjectWriter::recordRelocation( // Mark this symbol as requiring an entry in the symbol table. assert(SectionMap.find(Section) != SectionMap.end() && "Section must already have been defined in executePostLayoutBinding!"); - assert(SymbolMap.find(&A) != SymbolMap.end() && - "Symbol must already have been defined in executePostLayoutBinding!"); COFFSection *coff_section = SectionMap[Section]; - COFFSymbol *coff_symbol = SymbolMap[&A]; const MCSymbolRefExpr *SymB = Target.getSymB(); bool CrossSection = false; @@ -745,12 +685,12 @@ void WinCOFFObjectWriter::recordRelocation( if (!A.getFragment()) { Asm.getContext().reportError( Fixup.getLoc(), - Twine("symbol '") + Symbol.getName() + + Twine("symbol '") + A.getName() + "' can not be undefined in a subtraction expression"); return; } - CrossSection = &Symbol.getSection() != &B->getSection(); + CrossSection = &A.getSection() != &B->getSection(); // Offset of the symbol in the section int64_t OffsetOfB = Layout.getSymbolOffset(*B); @@ -779,12 +719,19 @@ void WinCOFFObjectWriter::recordRelocation( Reloc.Data.VirtualAddress = Layout.getFragmentOffset(Fragment); // Turn relocations for temporary symbols into section relocations. - if (coff_symbol->MC->isTemporary() || CrossSection) { - Reloc.Symb = coff_symbol->Section->Symbol; - FixedValue += Layout.getFragmentOffset(coff_symbol->MC->getFragment()) + - coff_symbol->MC->getOffset(); - } else - Reloc.Symb = coff_symbol; + if (A.isTemporary() || CrossSection) { + MCSection *TargetSection = &A.getSection(); + assert( + SectionMap.find(TargetSection) != SectionMap.end() && + "Section must already have been defined in executePostLayoutBinding!"); + Reloc.Symb = SectionMap[TargetSection]->Symbol; + FixedValue += Layout.getSymbolOffset(A); + } else { + assert( + SymbolMap.find(&A) != SymbolMap.end() && + "Symbol must already have been defined in executePostLayoutBinding!"); + Reloc.Symb = SymbolMap[&A]; + } ++Reloc.Symb->Relocations; @@ -898,14 +845,10 @@ void WinCOFFObjectWriter::writeObject(MCAssembler &Asm, // Update section number & offset for symbols that have them. if (Symbol->Section) Symbol->Data.SectionNumber = Symbol->Section->Number; - if (Symbol->should_keep()) { - Symbol->setIndex(Header.NumberOfSymbols++); - // Update auxiliary symbol info. - Symbol->Data.NumberOfAuxSymbols = Symbol->Aux.size(); - Header.NumberOfSymbols += Symbol->Data.NumberOfAuxSymbols; - } else { - Symbol->setIndex(-1); - } + Symbol->setIndex(Header.NumberOfSymbols++); + // Update auxiliary symbol info. + Symbol->Data.NumberOfAuxSymbols = Symbol->Aux.size(); + Header.NumberOfSymbols += Symbol->Data.NumberOfAuxSymbols; } // Build string table. @@ -913,7 +856,7 @@ void WinCOFFObjectWriter::writeObject(MCAssembler &Asm, if (S->Name.size() > COFF::NameSize) Strings.add(S->Name); for (const auto &S : Symbols) - if (S->should_keep() && S->Name.size() > COFF::NameSize) + if (S->Name.size() > COFF::NameSize) Strings.add(S->Name); Strings.finalize(); @@ -921,8 +864,7 @@ void WinCOFFObjectWriter::writeObject(MCAssembler &Asm, for (const auto &S : Sections) SetSectionName(*S); for (auto &S : Symbols) - if (S->should_keep()) - SetSymbolName(*S); + SetSymbolName(*S); // Fixup weak external references. for (auto &Symbol : Symbols) { diff --git a/test/MC/COFF/temporary-alias.s b/test/MC/COFF/temporary-alias.s new file mode 100644 index 00000000000..be4297024af --- /dev/null +++ b/test/MC/COFF/temporary-alias.s @@ -0,0 +1,21 @@ +// RUN: llvm-mc -triple=i686-pc-windows -filetype=obj -o %t %s +// RUN: llvm-objdump -d -r %t | FileCheck %s + +.globl _main +_main: +// CHECK: 00 00 00 00 +// CHECK-NEXT: 00000002: IMAGE_REL_I386_DIR32 .rdata +movb L_alias1(%eax), %al +// CHECK: 01 00 00 00 +// CHECK-NEXT: 00000008: IMAGE_REL_I386_DIR32 .rdata +movb L_alias2(%eax), %al +retl + +.section .rdata,"dr" +L_sym1: +.ascii "\001" +L_sym2: +.ascii "\002" + +L_alias1 = L_sym1 +L_alias2 = L_sym2