Clang crash targeting ARM or Thumb when assembling a prel31 relocation variant

[simonwallis2]

Summary

In the assembler or inline assembler,
attempting to use an invalid fixup type
gives a crash with a segmentation fault.

attribute((naked))
void foo(void) {
asm("mov r9, :lower16:bar(prel31)");
}

This should give a proper error message when building for ARM or Thumb.
This brings it in line with AARCH64.

This fixes all 8 instances of llvm_unreachable("Unsupported Modifier");
in ARM/MCTargetDesc/ARMELFObjectWriter.cpp.
A test is provided for each instance.

[MarkMurrayARM]

LGTM

llvm/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp

unsigned ARMELFObjectWriter::GetRelocTypeInner(const MCValue &Target,
   if (IsPCRel) {
     switch (Fixup.getTargetKind()) {
     default:
       Ctx.reportFatalError(Fixup.getLoc(), "unsupported relocation on symbol");
       return ELF::R_ARM_NONE;
     case FK_Data_4:
       switch (Modifier) {
       default:
-        llvm_unreachable("Unsupported Modifier");
+        Ctx.reportError(Fixup.getLoc(),
+                        "invalid fixup for 4-byte pc-relative data relocation");
+        return ELF::R_ARM_NONE;
       case MCSymbolRefExpr::VK_None: {
         if (const MCSymbolRefExpr *SymRef = Target.getSymA()) {
           // For GNU AS compatibility expressions such as
           // _GLOBAL_OFFSET_TABLE_ - label emit a R_ARM_BASE_PREL relocation.
           if (SymRef->getSymbol().getName() == "_GLOBAL_OFFSET_TABLE_")
             return ELF::R_ARM_BASE_PREL;
         }
         return ELF::R_ARM_REL32;
unsigned ARMELFObjectWriter::GetRelocTypeInner(const MCValue &Target,
   }
   switch (Kind) {
   default:
     Ctx.reportFatalError(Fixup.getLoc(), "unsupported relocation on symbol");
     return ELF::R_ARM_NONE;
   case FK_Data_1:
     switch (Modifier) {
     default:
-      llvm_unreachable("unsupported Modifier");
+      Ctx.reportError(Fixup.getLoc(),
+                      "invalid fixup for 1-byte data relocation");
+      return ELF::R_ARM_NONE;
     case MCSymbolRefExpr::VK_None:
       return ELF::R_ARM_ABS8;
     }
   case FK_Data_2:
     switch (Modifier) {
     default:
-      llvm_unreachable("unsupported modifier");
+      Ctx.reportError(Fixup.getLoc(),
+                      "invalid fixup for 2-byte data relocation");
+      return ELF::R_ARM_NONE;
     case MCSymbolRefExpr::VK_None:
       return ELF::R_ARM_ABS16;
     }
   case FK_Data_4:
     switch (Modifier) {
     default:
-      llvm_unreachable("Unsupported Modifier");
+      Ctx.reportError(Fixup.getLoc(),
+                      "invalid fixup for 4-byte data relocation");
+      return ELF::R_ARM_NONE;
     case MCSymbolRefExpr::VK_ARM_NONE:
       return ELF::R_ARM_NONE;
     case MCSymbolRefExpr::VK_GOT:
       return ELF::R_ARM_GOT_BREL;
     case MCSymbolRefExpr::VK_TLSGD:
       return ELF::R_ARM_TLS_GD32;
     case MCSymbolRefExpr::VK_TPOFF:
       return ELF::R_ARM_TLS_LE32;
case MCSymbolRefExpr::VK_ARM_TLSDESCSEQ:
       return ELF::R_ARM_TLS_DESCSEQ;
     }
   case ARM::fixup_arm_condbranch:
   case ARM::fixup_arm_uncondbranch:
     return ELF::R_ARM_JUMP24;
   case ARM::fixup_arm_movt_hi16:
     switch (Modifier) {
     default:
-      llvm_unreachable("Unsupported Modifier");
+      Ctx.reportError(Fixup.getLoc(), "invalid fixup for ARM MOVT instruction");
+      return ELF::R_ARM_NONE;
     case MCSymbolRefExpr::VK_None:
       return ELF::R_ARM_MOVT_ABS;
     case MCSymbolRefExpr::VK_ARM_SBREL:
       return ELF::R_ARM_MOVT_BREL;
     }
   case ARM::fixup_arm_movw_lo16:
     switch (Modifier) {
     default:
-      llvm_unreachable("Unsupported Modifier");
+      Ctx.reportError(Fixup.getLoc(), "invalid fixup for ARM MOVW instruction");
+      return ELF::R_ARM_NONE;
     case MCSymbolRefExpr::VK_None:
       return ELF::R_ARM_MOVW_ABS_NC;
     case MCSymbolRefExpr::VK_ARM_SBREL:
       return ELF::R_ARM_MOVW_BREL_NC;
     }
   case ARM::fixup_t2_movt_hi16:
     switch (Modifier) {
     default:
-      llvm_unreachable("Unsupported Modifier");
+      Ctx.reportError(Fixup.getLoc(),
+                      "invalid fixup for Thumb MOVT instruction");
+      return ELF::R_ARM_NONE;
     case MCSymbolRefExpr::VK_None:
       return ELF::R_ARM_THM_MOVT_ABS;
     case MCSymbolRefExpr::VK_ARM_SBREL:
       return ELF::R_ARM_THM_MOVT_BREL;
     }
   case ARM::fixup_t2_movw_lo16:
     switch (Modifier) {
     default:
-      llvm_unreachable("Unsupported Modifier");
+      Ctx.reportError(Fixup.getLoc(),
+                      "invalid fixup for Thumb MOVW instruction");
+      return ELF::R_ARM_NONE;
     case MCSymbolRefExpr::VK_None:
       return ELF::R_ARM_THM_MOVW_ABS_NC;
     case MCSymbolRefExpr::VK_ARM_SBREL:
       return ELF::R_ARM_THM_MOVW_BREL_NC;
     }
   }
 }
 

llvm/test/MC/ARM/error-location.s

 @ RUN: not llvm-mc -triple armv7a--none-eabi -filetype obj < %s -o /dev/null 2>&1 | FileCheck %s
+@ RUN: not llvm-mc -triple thumbv7a--none-eabi -filetype obj < %s -o /dev/null 2>&1 | FileCheck %s
 
 @ Note: These errors are not always emitted in the order in which the relevant
 @ source appears, this file is carefully ordered so that that is the case.
 
   .text
 @ CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: symbol 'undef' can not be undefined in a subtraction expression
   .word (0-undef)
 
 @ CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: expected relocatable expression
   .word -undef
 
 @ CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: symbol 'undef' can not be undefined in a subtraction expression
   adr r0, #a-undef
 
 @ CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: Cannot represent a difference across sections
   .word x_a - y_a
 
+@ CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: invalid fixup for {{ARM|Thumb}} MOVT instruction
+  movt r9, :upper16: bar(PREL31)
 
+@ CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: invalid fixup for {{ARM|Thumb}} MOVW instruction
+  movw r9, :lower16: bar(PREL31)
+
+@ CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: invalid fixup for 1-byte data relocation
+  .byte f30(PLT)
+
+@ CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: invalid fixup for 2-byte data relocation
+  .hword f30(PLT)
+
+@ CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: invalid fixup for 4-byte data relocation
+  .word f30(PLT)
+
+@ CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: invalid fixup for 4-byte pc-relative data relocation
+  .word x_a(PLT) - label1
+label1:
 
 w:
   .word 0
   .weak w
 
 
   .section sec_x
 x_a:
   .word 0
 
 
   .section sec_y
 y_a:
   .word 0