From e4d89ec8de33c9ed3554dcc2b2391b7698dba1e3 Mon Sep 17 00:00:00 2001 From: David Majnemer Date: Tue, 8 Apr 2014 22:33:40 +0000 Subject: [PATCH] WinCOFF: Emit common symbols as specified in the COFF spec Summary: Local common symbols were properly inserted into the .bss section. However, putting external common symbols in the .bss section would give them a strong definition. Instead, encode them as undefined, external symbols who's symbol value is equivalent to their size. Reviewers: Bigcheese, rafael, rnk CC: llvm-commits Differential Revision: http://reviews.llvm.org/D3324 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@205811 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/TargetLoweringObjectFileImpl.cpp | 9 ++-- lib/MC/MCObjectFileInfo.cpp | 4 ++ lib/MC/WinCOFFObjectWriter.cpp | 4 +- lib/MC/WinCOFFStreamer.cpp | 56 ++++++++++---------- test/MC/COFF/comm.ll | 4 +- test/MC/COFF/comm.s | 4 +- 6 files changed, 45 insertions(+), 36 deletions(-) diff --git a/lib/CodeGen/TargetLoweringObjectFileImpl.cpp b/lib/CodeGen/TargetLoweringObjectFileImpl.cpp index e41fbfc6d36..523d219daab 100644 --- a/lib/CodeGen/TargetLoweringObjectFileImpl.cpp +++ b/lib/CodeGen/TargetLoweringObjectFileImpl.cpp @@ -755,7 +755,7 @@ const MCSection *TargetLoweringObjectFileCOFF::getExplicitSectionGlobal( static const char *getCOFFSectionNameForUniqueGlobal(SectionKind Kind) { if (Kind.isText()) return ".text"; - if (Kind.isBSS ()) + if (Kind.isBSS()) return ".bss"; if (Kind.isThreadLocal()) return ".tls$"; @@ -781,7 +781,7 @@ SelectSectionForGlobal(const GlobalValue *GV, SectionKind Kind, // Section names depend on the name of the symbol which is not feasible if the // symbol has private linkage. if ((GV->isWeakForLinker() || EmitUniquedSection) && - !GV->hasPrivateLinkage()) { + !GV->hasPrivateLinkage() && !Kind.isCommon()) { const char *Name = getCOFFSectionNameForUniqueGlobal(Kind); unsigned Characteristics = getCOFFSectionFlags(Kind); @@ -802,7 +802,10 @@ SelectSectionForGlobal(const GlobalValue *GV, SectionKind Kind, if (Kind.isReadOnly()) return ReadOnlySection; - if (Kind.isBSS()) + // Note: we claim that common symbols are put in BSSSection, but they are + // really emitted with the magic .comm directive, which creates a symbol table + // entry but not a section. + if (Kind.isBSS() || Kind.isCommon()) return BSSSection; return DataSection; diff --git a/lib/MC/MCObjectFileInfo.cpp b/lib/MC/MCObjectFileInfo.cpp index 3b011c8bc5a..75147bfb489 100644 --- a/lib/MC/MCObjectFileInfo.cpp +++ b/lib/MC/MCObjectFileInfo.cpp @@ -548,6 +548,10 @@ void MCObjectFileInfo::InitELFMCObjectFileInfo(Triple T) { void MCObjectFileInfo::InitCOFFMCObjectFileInfo(Triple T) { + // The object file format cannot represent common symbols with explicit + // alignments. + CommDirectiveSupportsAlignment = false; + // COFF BSSSection = Ctx->getCOFFSection(".bss", diff --git a/lib/MC/WinCOFFObjectWriter.cpp b/lib/MC/WinCOFFObjectWriter.cpp index 500acd8f14e..f9426c61bde 100644 --- a/lib/MC/WinCOFFObjectWriter.cpp +++ b/lib/MC/WinCOFFObjectWriter.cpp @@ -394,7 +394,7 @@ void WinCOFFObjectWriter::DefineSection(MCSectionData const &SectionData) { SectionMap[&SectionData.getSection()] = coff_section; } -/// This function takes a section data object from the assembler +/// This function takes a symbol data object from the assembler /// and creates the associated COFF symbol staging object. void WinCOFFObjectWriter::DefineSymbol(MCSymbolData const &SymbolData, MCAssembler &Assembler, @@ -443,6 +443,8 @@ void WinCOFFObjectWriter::DefineSymbol(MCSymbolData const &SymbolData, int64_t Addr; if (Symbol.getVariableValue()->EvaluateAsAbsolute(Addr, Layout)) coff_symbol->Data.Value = Addr; + } else if (SymbolData.isExternal() && SymbolData.isCommon()) { + coff_symbol->Data.Value = SymbolData.getCommonSize(); } coff_symbol->Data.Type = (ResSymData.getFlags() & 0x0000FFFF) >> 0; diff --git a/lib/MC/WinCOFFStreamer.cpp b/lib/MC/WinCOFFStreamer.cpp index 5bd7b8f0c63..6981bb7982d 100644 --- a/lib/MC/WinCOFFStreamer.cpp +++ b/lib/MC/WinCOFFStreamer.cpp @@ -45,9 +45,6 @@ public: MCCodeEmitter &CE, raw_ostream &OS); - void AddCommonSymbol(MCSymbol *Symbol, uint64_t Size, - unsigned ByteAlignment, bool External); - // MCStreamer interface void InitSections() override; @@ -101,26 +98,6 @@ WinCOFFStreamer::WinCOFFStreamer(MCContext &Context, MCAsmBackend &MAB, MCCodeEmitter &CE, raw_ostream &OS) : MCObjectStreamer(Context, MAB, OS, &CE), CurSymbol(NULL) {} -void WinCOFFStreamer::AddCommonSymbol(MCSymbol *Symbol, uint64_t Size, - unsigned ByteAlignment, bool External) { - assert(!Symbol->isInSection() && "Symbol must not already have a section!"); - - const MCSection *Section = getContext().getObjectFileInfo()->getBSSSection(); - MCSectionData &SectionData = getAssembler().getOrCreateSectionData(*Section); - if (SectionData.getAlignment() < ByteAlignment) - SectionData.setAlignment(ByteAlignment); - - MCSymbolData &SymbolData = getAssembler().getOrCreateSymbolData(*Symbol); - SymbolData.setExternal(External); - - AssignSection(Symbol, Section); - - if (ByteAlignment != 1) - new MCAlignFragment(ByteAlignment, 0, 0, ByteAlignment, &SectionData); - - SymbolData.setFragment(new MCFillFragment(0, 0, Size, &SectionData)); -} - // MCStreamer interface void WinCOFFStreamer::InitSections() { @@ -244,15 +221,38 @@ void WinCOFFStreamer::EmitCommonSymbol(MCSymbol *Symbol, uint64_t Size, assert((Symbol->isInSection() ? Symbol->getSection().getVariant() == MCSection::SV_COFF : true) && "Got non-COFF section in the COFF backend!"); - AddCommonSymbol(Symbol, Size, ByteAlignment, true); + + if (ByteAlignment > 32) + report_fatal_error( + "The linker won't align common symbols beyond 32 bytes."); + + AssignSection(Symbol, NULL); + + MCSymbolData &SD = getAssembler().getOrCreateSymbolData(*Symbol); + SD.setExternal(true); + SD.setCommon(Size, ByteAlignment); } void WinCOFFStreamer::EmitLocalCommonSymbol(MCSymbol *Symbol, uint64_t Size, unsigned ByteAlignment) { - assert((Symbol->isInSection() - ? Symbol->getSection().getVariant() == MCSection::SV_COFF - : true) && "Got non-COFF section in the COFF backend!"); - AddCommonSymbol(Symbol, Size, ByteAlignment, false); + assert(!Symbol->isInSection() && "Symbol must not already have a section!"); + + const MCSection *Section = getContext().getObjectFileInfo()->getBSSSection(); + MCSectionData &SectionData = getAssembler().getOrCreateSectionData(*Section); + if (SectionData.getAlignment() < ByteAlignment) + SectionData.setAlignment(ByteAlignment); + + MCSymbolData &SD = getAssembler().getOrCreateSymbolData(*Symbol); + SD.setExternal(false); + + AssignSection(Symbol, Section); + + if (ByteAlignment != 1) + new MCAlignFragment(ByteAlignment, /*_Value=*/0, /*_ValueSize=*/0, + ByteAlignment, &SectionData); + + SD.setFragment( + new MCFillFragment(/*_Value=*/0, /*_ValueSize=*/0, Size, &SectionData)); } void WinCOFFStreamer::EmitZerofill(const MCSection *Section, MCSymbol *Symbol, diff --git a/test/MC/COFF/comm.ll b/test/MC/COFF/comm.ll index 74da557fb5c..6fe122ef1d9 100644 --- a/test/MC/COFF/comm.ll +++ b/test/MC/COFF/comm.ll @@ -9,5 +9,5 @@ ; CHECK: .lcomm _a,1 ; CHECK: .lcomm _b,8,8 ; .comm uses log2 alignment -; CHECK: .comm _c,1,0 -; CHECK: .comm _d,8,3 +; CHECK: .comm _c,1 +; CHECK: .comm _d,8 diff --git a/test/MC/COFF/comm.s b/test/MC/COFF/comm.s index 21ae5d25dda..37db75f9cc4 100644 --- a/test/MC/COFF/comm.s +++ b/test/MC/COFF/comm.s @@ -1,7 +1,7 @@ // RUN: llvm-mc -filetype=obj -triple i686-pc-win32 %s | llvm-readobj -t | FileCheck %s .lcomm _a,4,4 -.comm _b, 4, 2 +.comm _b, 4 // CHECK: Symbol { @@ -17,7 +17,7 @@ // CHECK: Symbol { // CHECK: Name: _b // CHECK-NEXT: Value: 4 -// CHECK-NEXT: Section: .bss +// CHECK-NEXT: Section: (0) // CHECK-NEXT: BaseType: Null // CHECK-NEXT: ComplexType: Null // CHECK-NEXT: StorageClass: External -- 2.34.1