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

llvm-svn: 254183
This commit is contained in:
Peter Collingbourne 2015-11-26 23:29:27 +00:00
parent 4241cedb68
commit 8359a6a83e
2 changed files with 44 additions and 81 deletions

View File

@ -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<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(const MCSymbol &Symbol,
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(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<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::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) {

View File

@ -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