[llvm] r254183 - MC: Simplify handling of temporary symbols in COFF writer.
Peter Collingbourne via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 26 15:29:28 PST 2015
Author: pcc
Date: Thu Nov 26 17:29:27 2015
New Revision: 254183
URL: http://llvm.org/viewvc/llvm-project?rev=254183&view=rev
Log:
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
Added:
llvm/trunk/test/MC/COFF/temporary-alias.s
Modified:
llvm/trunk/lib/MC/WinCOFFObjectWriter.cpp
Modified: llvm/trunk/lib/MC/WinCOFFObjectWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/WinCOFFObjectWriter.cpp?rev=254183&r1=254182&r2=254183&view=diff
==============================================================================
--- llvm/trunk/lib/MC/WinCOFFObjectWriter.cpp (original)
+++ llvm/trunk/lib/MC/WinCOFFObjectWriter.cpp Thu Nov 26 17:29:27 2015
@@ -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_
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<MCSymbolCOFF>(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(c
MCAssembler &Assembler,
const MCAsmLayout &Layout) {
COFFSymbol *coff_symbol = GetOrCreateCOFFSymbol(&Symbol);
- SymbolMap[&Symbol] = coff_symbol;
if (cast<MCSymbolCOFF>(Symbol).isWeakExternal()) {
coff_symbol->Data.StorageClass = COFF::IMAGE_SYM_CLASS_WEAK_EXTERNAL;
@@ -517,25 +480,6 @@ void WinCOFFObjectWriter::SetSymbolName(
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::executePostLay
defineSection(static_cast<const MCSectionCOFF &>(Section));
for (const MCSymbol &Symbol : Asm.symbols())
- if (ExportSymbol(Symbol, Asm))
+ if (!Symbol.isTemporary())
DefineSymbol(Symbol, Asm, Layout);
}
@@ -704,8 +648,7 @@ void WinCOFFObjectWriter::recordRelocati
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::recordRelocati
// 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::recordRelocati
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::recordRelocati
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(MC
// 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(MC
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(MC
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) {
Added: llvm/trunk/test/MC/COFF/temporary-alias.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/COFF/temporary-alias.s?rev=254183&view=auto
==============================================================================
--- llvm/trunk/test/MC/COFF/temporary-alias.s (added)
+++ llvm/trunk/test/MC/COFF/temporary-alias.s Thu Nov 26 17:29:27 2015
@@ -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
More information about the llvm-commits
mailing list