[PATCH] D78138: [MC][ELF] Reject instructions in SHT_NOBITS sections

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: rnk, skan.
Herald added subscribers: llvm-commits, hiraditya, kristof.beyls, Prazek, emaste.
Herald added a reviewer: espindola.
Herald added a project: LLVM.

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 <https://reviews.llvm.org/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, 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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78138

Files:
  llvm/lib/MC/MCAssembler.cpp
  llvm/lib/MC/MCELFStreamer.cpp
  llvm/test/MC/ELF/ARM/bss-non-zero-value.s
  llvm/test/MC/ELF/nobits-non-zero-value.s
  llvm/test/MC/X86/reloc-bss.s


Index: llvm/test/MC/X86/reloc-bss.s
===================================================================
--- 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
Index: llvm/test/MC/ELF/nobits-non-zero-value.s
===================================================================
--- /dev/null
+++ llvm/test/MC/ELF/nobits-non-zero-value.s
@@ -0,0 +1,15 @@
+# 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
+# CHECK: {{.*}}.s:[[#@LINE+1]]:3: error: SHT_NOBITS section cannot have instructions
+  nop
+
+.bss
+# CHECK: {{.*}}.s:[[#@LINE+1]]:3: error: SHT_NOBITS section cannot have instructions
+  addb %al,(%rax)
+
+# CHECK: <unknown>:0: error: non-zero initializer found in section '.bss'
+  .long 1
Index: llvm/test/MC/ELF/ARM/bss-non-zero-value.s
===================================================================
--- 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
Index: llvm/lib/MC/MCELFStreamer.cpp
===================================================================
--- llvm/lib/MC/MCELFStreamer.cpp
+++ llvm/lib/MC/MCELFStreamer.cpp
@@ -511,6 +511,12 @@
 
 void MCELFStreamer::EmitInstToData(const MCInst &Inst,
                                    const MCSubtargetInfo &STI) {
+  MCSection &Sec = *getCurrentSectionOnly();
+  if (Sec.isVirtualSection()) {
+    getContext().reportError(Inst.getLoc(),
+                             "SHT_NOBITS section cannot have instructions");
+    return;
+  }
   MCAssembler &Assembler = getAssembler();
   SmallVector<MCFixup, 4> Fixups;
   SmallString<256> Code;
@@ -538,7 +544,6 @@
   MCDataFragment *DF;
 
   if (Assembler.isBundlingEnabled()) {
-    MCSection &Sec = *getCurrentSectionOnly();
     if (Assembler.getRelaxAll() && isBundleLocked()) {
       // If the -mc-relax-all flag is used and we are bundle-locked, we re-use
       // the current bundle group.
Index: llvm/lib/MC/MCAssembler.cpp
===================================================================
--- llvm/lib/MC/MCAssembler.cpp
+++ llvm/lib/MC/MCAssembler.cpp
@@ -687,10 +687,12 @@
         for (unsigned i = 0, e = DF.getContents().size(); i != e; ++i)
           if (DF.getContents()[i]) {
             if (auto *ELFSec = dyn_cast<const MCSectionELF>(Sec))
-              report_fatal_error("non-zero initializer found in section '" +
-                  ELFSec->getSectionName() + "'");
+              getContext().reportError(
+                  SMLoc(), "non-zero initializer found in section '" +
+                               ELFSec->getSectionName() + "'");
             else
-              report_fatal_error("non-zero initializer found in virtual section");
+              getContext().reportError(
+                  SMLoc(), "non-zero initializer found in virtual section");
           }
         break;
       }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78138.257425.patch
Type: text/x-patch
Size: 3438 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200414/c40e2ebb/attachment.bin>