MC: Optimize emitInstruction and simplify fragment-in-BSS check

Move the FT_Relaxable-in-BSS check from frequently-called
MCObjectStreamer::emitInstruction to MCAssembler::writeSectionData,
along with existing checks for other fragment types. For the uncommon
diagnostics, losing the location information is acceptable.

Author: MaskRay

llvm/include/llvm/MC/MCObjectStreamer.h

@@ -54,7 +54,6 @@ class MCObjectStreamer : public MCStreamer {
   void emitInstToData(const MCInst &Inst, const MCSubtargetInfo &);
   void emitCFIStartProcImpl(MCDwarfFrameInfo &Frame) override;
   void emitCFIEndProcImpl(MCDwarfFrameInfo &Frame) override;
-  void emitInstructionImpl(const MCInst &Inst, const MCSubtargetInfo &STI);
 
 protected:
   MCObjectStreamer(MCContext &Context, std::unique_ptr<MCAsmBackend> TAB,

llvm/lib/MC/MCAssembler.cpp

@@ -583,42 +583,23 @@ void MCAssembler::writeSectionData(raw_ostream &OS,
                                    const MCSection *Sec) const {
   assert(getBackendPtr() && "Expected assembler backend");
 
-  // Ignore virtual sections.
   if (Sec->isVirtualSection()) {
     assert(getSectionFileSize(*Sec) == 0 && "Invalid size for section!");
 
-    // Check that contents are only things legal inside a virtual section.
+    // Ensure no fixups or non-zero bytes are written to BSS sections, catching
+    // errors in both input assembly code and MCStreamer API usage. Location is
+    // not tracked for efficiency.
+    auto Fn = [](char c) { return c != 0; };
     for (const MCFragment &F : *Sec) {
-      switch (F.getKind()) {
-      default: llvm_unreachable("Invalid fragment in virtual section!");
-      case MCFragment::FT_Data: {
-        // Check that we aren't trying to write a non-zero contents (or fixups)
-        // into a virtual section. This is to support clients which use standard
-        // directives to fill the contents of virtual sections.
-        if (F.getFixups().size() || F.getVarFixups().size())
-          reportError(SMLoc(), Sec->getVirtualSectionKind() + " section '" +
-                                   Sec->getName() + "' cannot have fixups");
-        for (char C : F.getContents())
-          if (C) {
-            reportError(SMLoc(), Sec->getVirtualSectionKind() + " section '" +
-                                     Sec->getName() +
-                                     "' cannot have non-zero initializers");
-            break;
-          }
+      if (any_of(F.getContents(), Fn) || any_of(F.getVarContents(), Fn)) {
+        reportError(SMLoc(), Sec->getVirtualSectionKind() + " section '" +
+                                 Sec->getName() +
+                                 "' cannot have non-zero bytes");
         break;
       }
-      case MCFragment::FT_Align:
-        // Check that we aren't trying to write a non-zero value into a virtual
-        // section.
-        assert((cast<MCAlignFragment>(F).getFillLen() == 0 ||
-                cast<MCAlignFragment>(F).getFill() == 0) &&
-               "Invalid align in virtual section!");
-        break;
-      case MCFragment::FT_Fill:
-        assert((cast<MCFillFragment>(F).getValue() == 0) &&
-               "Invalid fill in virtual section!");
-        break;
-      case MCFragment::FT_Org:
+      if (F.getFixups().size() || F.getVarFixups().size()) {
+        reportError(SMLoc(), Sec->getVirtualSectionKind() + " section '" +
+                                 Sec->getName() + "' cannot have fixups");
         break;
       }
     }

llvm/lib/MC/MCObjectStreamer.cpp

@@ -279,18 +279,6 @@ bool MCObjectStreamer::mayHaveInstructions(MCSection &Sec) const {
 
 void MCObjectStreamer::emitInstruction(const MCInst &Inst,
                                        const MCSubtargetInfo &STI) {
-  const MCSection &Sec = *getCurrentSectionOnly();
-  if (Sec.isVirtualSection()) {
-    getContext().reportError(Inst.getLoc(), Twine(Sec.getVirtualSectionKind()) +
-                                                " section '" + Sec.getName() +
-                                                "' cannot have instructions");
-    return;
-  }
-  emitInstructionImpl(Inst, STI);
-}
-
-void MCObjectStreamer::emitInstructionImpl(const MCInst &Inst,
-                                           const MCSubtargetInfo &STI) {
   MCStreamer::emitInstruction(Inst, STI);
 
   MCSection *Sec = getCurrentSectionOnly();

llvm/lib/MC/WinCOFFObjectWriter.cpp

@@ -179,7 +179,7 @@ class llvm::WinCOFFWriter {
   void SetSymbolName(COFFSymbol &S);
   void SetSectionName(COFFSection &S);
 
-  bool IsPhysicalSection(COFFSection *S);
+  bool isUninitializedData(const COFFSection &S);
 
   // Entity writing methods.
   void WriteFileHeader(const COFF::header &Header);
@@ -453,8 +453,8 @@ void WinCOFFWriter::SetSymbolName(COFFSymbol &S) {
     std::memcpy(S.Data.Name, S.Name.c_str(), S.Name.size());
 }
 
-bool WinCOFFWriter::IsPhysicalSection(COFFSection *S) {
-  return (S->Header.Characteristics & COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA) ==
+bool WinCOFFWriter::isUninitializedData(const COFFSection &S) {
+  return (S.Header.Characteristics & COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA) !=
          0;
 }
 
@@ -606,6 +606,9 @@ void WinCOFFWriter::writeSection(const COFFSection &Sec) {
     assert(AuxSyms.size() == 1 && AuxSyms[0].AuxType == ATSectionDefinition);
     AuxSymbol &SecDef = AuxSyms[0];
     SecDef.Aux.SectionDefinition.CheckSum = CRC;
+  } else if (isUninitializedData(Sec)) {
+    // Error if fixups or non-zero bytes are present.
+    writeSectionContents(*Sec.MCSection);
   }
 
   // Write relocations for this section.
@@ -745,7 +748,7 @@ void WinCOFFWriter::assignFileOffsets() {
 
     Sec->Header.SizeOfRawData = Asm->getSectionAddressSize(Section);
 
-    if (IsPhysicalSection(Sec)) {
+    if (!isUninitializedData(*Sec)) {
       Sec->Header.PointerToRawData = Offset;
       Offset += Sec->Header.SizeOfRawData;
     }

llvm/test/MC/COFF/bss-text.s

@@ -1,13 +1,15 @@
-# RUN: not llvm-mc -filetype=obj -triple=x86_64-pc-win32 %s -o /dev/null 2>&1 | FileCheck %s
+# RUN: not llvm-mc -filetype=obj -triple=x86_64-pc-win32 %s -o /dev/null 2>&1 | FileCheck %s --implicit-check-not=error:
 
 ## -filetype=asm does not check the error.
 # RUN: llvm-mc -triple=x86_64-pc-win32 %s
 
+.bss
+# CHECK: <unknown>:0: error: IMAGE_SCN_CNT_UNINITIALIZED_DATA section '.bss' cannot have non-zero bytes
+  addb %bl,(%rax)
+
 .section uninitialized,"b"
-# MCRelaxableFragment
-# CHECK: {{.*}}.s:[[#@LINE+1]]:3: error: IMAGE_SCN_CNT_UNINITIALIZED_DATA section 'uninitialized' cannot have instructions
+# CHECK: <unknown>:0: error: IMAGE_SCN_CNT_UNINITIALIZED_DATA section 'uninitialized' cannot have non-zero bytes
   jmp foo
 
-.bss
-# CHECK: {{.*}}.s:[[#@LINE+1]]:3: error: IMAGE_SCN_CNT_UNINITIALIZED_DATA section '.bss' cannot have instructions
+.section bss0,"b"
   addb %al,(%rax)

llvm/test/MC/COFF/section.s

@@ -29,7 +29,7 @@
 .section s      ; .long 1
 .section s_, "" ; .long 1
 .section s_a,"a"; .long 1
-.section s_b,"b"; .long 1
+.section s_b,"b"; .long 0
 .section s_d,"d"; .long 1
 .section s_D,"D"; .long 1
 .section s_n,"n"; .long 1

llvm/test/MC/ELF/nobits-non-zero-value.s

@@ -1,26 +1,28 @@
-# RUN: not llvm-mc -filetype=obj -triple=x86_64 %s -o /dev/null 2>&1 | FileCheck %s --implicit-check-not=error:
+# RUN: not llvm-mc -filetype=obj -triple=x86_64 %s -o /dev/null 2>&1 | FileCheck %s --implicit-check-not=error: --implicit-check-not=warning:
 
 ## -filetype=asm does not check the error.
 # RUN: llvm-mc -triple=x86_64 %s
 
 .section .tbss,"aw",@nobits
-# MCRelaxableFragment
-# CHECK: {{.*}}.s:[[#@LINE+1]]:3: error: SHT_NOBITS section '.tbss' cannot have instructions
   jmp foo
 
 .bss
-# CHECK: {{.*}}.s:[[#@LINE+1]]:3: error: SHT_NOBITS section '.bss' cannot have instructions
   addb %al,(%rax)
 
 # CHECK: {{.*}}.s:[[#@LINE+1]]:11: warning: ignoring non-zero fill value in SHT_NOBITS section '.bss'
 .align 4, 42
 
-# CHECK-NOT: {{.*}}.s:[[#@LINE+1]]:11: warning: ignoring non-zero fill value in SHT_NOBITS section '.bss'
 .align 4, 0
 
-# CHECK: <unknown>:0: error: SHT_NOBITS section '.bss' cannot have non-zero initializers
   .long 1
 
+.section .bss0,"aw",%nobits
+addb %al,(%rax)
+
 .section .bss1,"aw",%nobits
-# CHECK: <unknown>:0: error: SHT_NOBITS section '.bss1' cannot have fixups
 .quad foo
+
+## Location is not tracked for efficiency.
+# CHECK: <unknown>:0: error: SHT_NOBITS section '.tbss' cannot have non-zero bytes
+# CHECK: <unknown>:0: error: SHT_NOBITS section '.bss' cannot have non-zero bytes
+# CHECK: <unknown>:0: error: SHT_NOBITS section '.bss1' cannot have fixups