[llvm] e13a8a1 - [MC][COFF][ELF] Reject instructions in IMAGE_SCN_CNT_UNINITIALIZED_DATA/SHT_NOBITS sections

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 15 21:02:59 PDT 2020


Author: Fangrui Song
Date: 2020-04-15T21:02:47-07:00
New Revision: e13a8a1fc56837e2f21b85b89a445fb4f21500d6

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

LOG: [MC][COFF][ELF] Reject instructions in IMAGE_SCN_CNT_UNINITIALIZED_DATA/SHT_NOBITS sections

For `.bss; nop`, MC inappropriately calls abort() (via report_fatal_error()) with a message
`cannot have fixups in virtual section!`
It is a bug to crash for invalid user input. Fix it by erroring out early in EmitInstToData().

Similarly, emitIntValue() in a virtual section (SHT_NOBITS in ELF) can crash with the mssage
`non-zero initializer found in section '.bss'` (see D4199)
It'd be nice to report the location but so many directives can call emitIntValue()
and it is difficult to track every location.
Note, COFF does not crash because MCAssembler::writeSectionData() is not
called for an IMAGE_SCN_CNT_UNINITIALIZED_DATA section.

Note, GNU as' arm64 backend reports ``Error: attempt to store non-zero value in section `.bss'``
for a non-zero .inst but fails to do so for other instructions.
We simply reject all instructions, even if the encoding is all zeros.

The Mach-O counterpart is D48517 (see `test/MC/MachO/zerofill-text.s`)

Reviewed By: rnk, skan

Differential Revision: https://reviews.llvm.org/D78138

Added: 
    llvm/test/MC/COFF/bss-text.s
    llvm/test/MC/ELF/nobits-non-zero-value.s

Modified: 
    llvm/include/llvm/MC/MCSection.h
    llvm/include/llvm/MC/MCSectionCOFF.h
    llvm/include/llvm/MC/MCSectionELF.h
    llvm/lib/MC/MCAssembler.cpp
    llvm/lib/MC/MCObjectStreamer.cpp
    llvm/lib/MC/MCSection.cpp
    llvm/lib/MC/MCSectionCOFF.cpp
    llvm/lib/MC/MCSectionELF.cpp

Removed: 
    llvm/test/MC/ELF/ARM/bss-non-zero-value.s
    llvm/test/MC/X86/reloc-bss.s


################################################################################
diff  --git a/llvm/include/llvm/MC/MCSection.h b/llvm/include/llvm/MC/MCSection.h
index d5e1f3dbd4a6..a68e06e661be 100644
--- a/llvm/include/llvm/MC/MCSection.h
+++ b/llvm/include/llvm/MC/MCSection.h
@@ -189,6 +189,8 @@ class MCSection {
   /// file contents.
   virtual bool isVirtualSection() const = 0;
 
+  virtual StringRef getVirtualSectionKind() const;
+
   /// Add a pending label for the requested subsection. This label will be
   /// associated with a fragment in flushPendingLabels()
   void addPendingLabel(MCSymbol* label, unsigned Subsection = 0);

diff  --git a/llvm/include/llvm/MC/MCSectionCOFF.h b/llvm/include/llvm/MC/MCSectionCOFF.h
index f52983e8590d..3ece6eb904bc 100644
--- a/llvm/include/llvm/MC/MCSectionCOFF.h
+++ b/llvm/include/llvm/MC/MCSectionCOFF.h
@@ -74,6 +74,7 @@ class MCSectionCOFF final : public MCSection {
                             const MCExpr *Subsection) const override;
   bool UseCodeAlign() const override;
   bool isVirtualSection() const override;
+  StringRef getVirtualSectionKind() const override;
 
   unsigned getOrAssignWinCFISectionID(unsigned *NextID) const {
     if (WinCFISectionID == ~0U)

diff  --git a/llvm/include/llvm/MC/MCSectionELF.h b/llvm/include/llvm/MC/MCSectionELF.h
index 466fe470c7bf..4136ea79de41 100644
--- a/llvm/include/llvm/MC/MCSectionELF.h
+++ b/llvm/include/llvm/MC/MCSectionELF.h
@@ -78,6 +78,7 @@ class MCSectionELF final : public MCSection {
                             const MCExpr *Subsection) const override;
   bool UseCodeAlign() const override;
   bool isVirtualSection() const override;
+  StringRef getVirtualSectionKind() const override;
 
   bool isUnique() const { return UniqueID != NonUniqueID; }
   unsigned getUniqueID() const { return UniqueID; }

diff  --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp
index 0d28827e1b1d..8949a4d9ba87 100644
--- a/llvm/lib/MC/MCAssembler.cpp
+++ b/llvm/lib/MC/MCAssembler.cpp
@@ -683,11 +683,16 @@ void MCAssembler::writeSectionData(raw_ostream &OS, const MCSection *Sec,
         // directives to fill the contents of virtual sections.
         const MCDataFragment &DF = cast<MCDataFragment>(F);
         if (DF.fixup_begin() != DF.fixup_end())
-          report_fatal_error("cannot have fixups in virtual section!");
+          getContext().reportError(SMLoc(), Sec->getVirtualSectionKind() +
+                                                " section '" + Sec->getName() +
+                                                "' cannot have fixups");
         for (unsigned i = 0, e = DF.getContents().size(); i != e; ++i)
           if (DF.getContents()[i]) {
-            report_fatal_error("non-zero initializer found in section '" +
-                               Sec->getName() + "'");
+            getContext().reportError(SMLoc(),
+                                     Sec->getVirtualSectionKind() +
+                                         " section '" + Sec->getName() +
+                                         "' cannot have non-zero initializers");
+            break;
           }
         break;
       }

diff  --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index 3549280b8e88..be6c47847c92 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -367,6 +367,13 @@ 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;
+  }
   getAssembler().getBackend().emitInstructionBegin(*this, Inst);
   emitInstructionImpl(Inst, STI);
   getAssembler().getBackend().emitInstructionEnd(*this, Inst);

diff  --git a/llvm/lib/MC/MCSection.cpp b/llvm/lib/MC/MCSection.cpp
index 669797468b1e..ba256102080a 100644
--- a/llvm/lib/MC/MCSection.cpp
+++ b/llvm/lib/MC/MCSection.cpp
@@ -87,7 +87,9 @@ MCSection::getSubsectionInsertionPoint(unsigned Subsection) {
   return IP;
 }
 
-void MCSection::addPendingLabel(MCSymbol* label, unsigned Subsection) {
+StringRef MCSection::getVirtualSectionKind() const { return "virtual"; }
+
+void MCSection::addPendingLabel(MCSymbol *label, unsigned Subsection) {
   PendingLabels.push_back(PendingLabel(label, Subsection));
 }
 

diff  --git a/llvm/lib/MC/MCSectionCOFF.cpp b/llvm/lib/MC/MCSectionCOFF.cpp
index 9108b0dee81e..387bf2c884e5 100644
--- a/llvm/lib/MC/MCSectionCOFF.cpp
+++ b/llvm/lib/MC/MCSectionCOFF.cpp
@@ -111,3 +111,7 @@ bool MCSectionCOFF::UseCodeAlign() const {
 bool MCSectionCOFF::isVirtualSection() const {
   return getCharacteristics() & COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA;
 }
+
+StringRef MCSectionCOFF::getVirtualSectionKind() const {
+  return "IMAGE_SCN_CNT_UNINITIALIZED_DATA";
+}

diff  --git a/llvm/lib/MC/MCSectionELF.cpp b/llvm/lib/MC/MCSectionELF.cpp
index ba06737a9df5..77c259c27a04 100644
--- a/llvm/lib/MC/MCSectionELF.cpp
+++ b/llvm/lib/MC/MCSectionELF.cpp
@@ -196,3 +196,5 @@ bool MCSectionELF::UseCodeAlign() const {
 bool MCSectionELF::isVirtualSection() const {
   return getType() == ELF::SHT_NOBITS;
 }
+
+StringRef MCSectionELF::getVirtualSectionKind() const { return "SHT_NOBITS"; }

diff  --git a/llvm/test/MC/COFF/bss-text.s b/llvm/test/MC/COFF/bss-text.s
new file mode 100644
index 000000000000..ed6890565b9a
--- /dev/null
+++ b/llvm/test/MC/COFF/bss-text.s
@@ -0,0 +1,13 @@
+# RUN: not llvm-mc -filetype=obj -triple=x86_64-pc-win32 %s -o /dev/null 2>&1 | FileCheck %s
+
+## -filetype=asm does not check the error.
+# RUN: llvm-mc -triple=x86_64-pc-win32 %s
+
+.section uninitialized,"b"
+# MCRelaxableFragment
+# CHECK: {{.*}}.s:[[#@LINE+1]]:3: error: IMAGE_SCN_CNT_UNINITIALIZED_DATA section 'uninitialized' cannot have instructions
+  jmp foo
+
+.bss
+# CHECK: {{.*}}.s:[[#@LINE+1]]:3: error: IMAGE_SCN_CNT_UNINITIALIZED_DATA section '.bss' cannot have instructions
+  addb %al,(%rax)

diff  --git a/llvm/test/MC/ELF/ARM/bss-non-zero-value.s b/llvm/test/MC/ELF/ARM/bss-non-zero-value.s
deleted file mode 100644
index da946f1c95d9..000000000000
--- a/llvm/test/MC/ELF/ARM/bss-non-zero-value.s
+++ /dev/null
@@ -1,9 +0,0 @@
-// RUN: not --crash llvm-mc -filetype=obj -triple arm-linux-gnu %s -o %t 2>%t.out
-// RUN: FileCheck --input-file=%t.out %s
-// CHECK: non-zero initializer found in section '.bss'
-	.bss
-	.globl	a
-	.align	2
-a:
-	.long	1
-	.size	a, 4

diff  --git a/llvm/test/MC/ELF/nobits-non-zero-value.s b/llvm/test/MC/ELF/nobits-non-zero-value.s
new file mode 100644
index 000000000000..16d386dc62ad
--- /dev/null
+++ b/llvm/test/MC/ELF/nobits-non-zero-value.s
@@ -0,0 +1,16 @@
+# RUN: not llvm-mc -filetype=obj -triple=x86_64 %s -o /dev/null 2>&1 | FileCheck %s
+
+## -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: <unknown>:0: error: SHT_NOBITS section '.bss' cannot have non-zero initializers
+  .long 1

diff  --git a/llvm/test/MC/X86/reloc-bss.s b/llvm/test/MC/X86/reloc-bss.s
deleted file mode 100644
index 6463b866f095..000000000000
--- a/llvm/test/MC/X86/reloc-bss.s
+++ /dev/null
@@ -1,9 +0,0 @@
-# RUN: not --crash llvm-mc -filetype=obj -triple=x86_64-linux-gnu %s 2>&1 | FileCheck %s
-# CHECK: LLVM ERROR: cannot have fixups in virtual section!
-
-.section        .init_array,"awT", at nobits
-
-.hidden patatino
-.globl  patatino
-patatino:
-  movl __init_array_start, %eax


        


More information about the llvm-commits mailing list