[MC][COFF][ELF] Reject instructions in IMAGE_SCN_CNT_UNINITIALIZED_DATA/SHT_NOBITS...
authorFangrui Song <maskray@google.com>
Tue, 14 Apr 2020 18:05:21 +0000 (11:05 -0700)
committerFangrui Song <maskray@google.com>
Thu, 16 Apr 2020 04:02:47 +0000 (21:02 -0700)
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

12 files changed:
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
llvm/test/MC/COFF/bss-text.s [new file with mode: 0644]
llvm/test/MC/ELF/ARM/bss-non-zero-value.s [deleted file]
llvm/test/MC/ELF/nobits-non-zero-value.s [new file with mode: 0644]
llvm/test/MC/X86/reloc-bss.s [deleted file]

index d5e1f3d..a68e06e 100644 (file)
@@ -189,6 +189,8 @@ public:
   /// 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);
index f52983e..3ece6eb 100644 (file)
@@ -74,6 +74,7 @@ public:
                             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)
index 466fe47..4136ea7 100644 (file)
@@ -78,6 +78,7 @@ public:
                             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; }
index 0d28827..8949a4d 100644 (file)
@@ -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;
       }
index 3549280..be6c478 100644 (file)
@@ -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);
index 6697974..ba25610 100644 (file)
@@ -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));
 }
 
index 9108b0d..387bf2c 100644 (file)
@@ -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";
+}
index ba06737..77c259c 100644 (file)
@@ -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 (file)
index 0000000..ed68905
--- /dev/null
@@ -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 (file)
index da946f1..0000000
+++ /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 (file)
index 0000000..16d386d
--- /dev/null
@@ -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",@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 (file)
index 6463b86..0000000
+++ /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",@nobits
-
-.hidden patatino
-.globl  patatino
-patatino:
-  movl __init_array_start, %eax