[llvm] 5ee34ff - MC: Optimize emitInstruction and simplify fragment-in-BSS check

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Sun Jul 20 00:09:29 PDT 2025


Author: Fangrui Song
Date: 2025-07-20T00:09:24-07:00
New Revision: 5ee34ff1e5cc952116f0da943ddaeb1a71db2940

URL: https://github.com/llvm/llvm-project/commit/5ee34ff1e5cc952116f0da943ddaeb1a71db2940
DIFF: https://github.com/llvm/llvm-project/commit/5ee34ff1e5cc952116f0da943ddaeb1a71db2940.diff

LOG: 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.

Added: 
    

Modified: 
    llvm/include/llvm/MC/MCObjectStreamer.h
    llvm/lib/MC/MCAssembler.cpp
    llvm/lib/MC/MCObjectStreamer.cpp
    llvm/lib/MC/WinCOFFObjectWriter.cpp
    llvm/test/MC/COFF/bss-text.s
    llvm/test/MC/COFF/section.s
    llvm/test/MC/ELF/nobits-non-zero-value.s

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/MC/MCObjectStreamer.h b/llvm/include/llvm/MC/MCObjectStreamer.h
index 3ccc1e30222a9..deaabddab3cc7 100644
--- a/llvm/include/llvm/MC/MCObjectStreamer.h
+++ b/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,

diff  --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index 942092457c452..48b222e7ba5a9 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/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;
       }
     }

diff  --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index 285c0f29ca4cc..739e5fec8aa5d 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/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();

diff  --git a/llvm/lib/MC/WinCOFFObjectWriter.cpp b/llvm/lib/MC/WinCOFFObjectWriter.cpp
index ee4d957fe9d87..c69b8d669235f 100644
--- a/llvm/lib/MC/WinCOFFObjectWriter.cpp
+++ b/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;
     }

diff  --git a/llvm/test/MC/COFF/bss-text.s b/llvm/test/MC/COFF/bss-text.s
index ed6890565b9a3..439bd789dff2c 100644
--- a/llvm/test/MC/COFF/bss-text.s
+++ b/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)

diff  --git a/llvm/test/MC/COFF/section.s b/llvm/test/MC/COFF/section.s
index 9c1a11effa341..fdd65701b1050 100644
--- a/llvm/test/MC/COFF/section.s
+++ b/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

diff  --git a/llvm/test/MC/ELF/nobits-non-zero-value.s b/llvm/test/MC/ELF/nobits-non-zero-value.s
index ff43e69baaedc..e9516aa3e835a 100644
--- a/llvm/test/MC/ELF/nobits-non-zero-value.s
+++ b/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", at 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


        


More information about the llvm-commits mailing list