Change the ARM assembler to require a :lower16: or :upper16 on non-constant

expressions for mov instructions instead of silently truncating by default.

For the ARM assembler, we want to avoid misleadingly allowing something
like "mov r0, <symbol>" especially when we turn it into a movw and the
expression <symbol> does not have a :lower16: or :upper16" as part of the
expression.  We don't want the behavior of silently truncating, which can be
unexpected and lead to bugs that are difficult to find since this is an easy
mistake to make.

This does change the previous behavior of llvm but actually matches an
older gnu assembler that would not allow this but print less useful errors
of like “invalid constant (0x927c0) after fixup” and “unsupported relocation on
symbol foo”.  The error for llvm is "immediate expression for mov requires
:lower16: or :upper16" with correct location information on the operand
as shown in the added test cases.

rdar://12342160

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@206669 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Target/ARM/AsmParser/ARMAsmParser.cpp b/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
index a22d03e20beb2d1013abd6775d27e3d2ce27e691..0e5fd1f6e0c398964cb93e072ccf96eb6ad51932 100644 (file)
--- a/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ b/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -5863,6 +5863,30 @@ validateInstruction(MCInst &Inst,
       return Error(Operands[Op]->getStartLoc(), "branch target out of range");
     break;
   }
+  case ARM::MOVi16:
+  case ARM::t2MOVi16:
+  case ARM::t2MOVTi16:
+    {
+    // We want to avoid misleadingly allowing something like "mov r0, <symbol>"
+    // especially when we turn it into a movw and the expression <symbol> does
+    // not have a :lower16: or :upper16 as part of the expression.  We don't
+    // want the behavior of silently truncating, which can be unexpected and
+    // lead to bugs that are difficult to find since this is an easy mistake
+    // to make.
+    int i = (Operands[3]->isImm()) ? 3 : 4;
+    ARMOperand *Op = static_cast<ARMOperand*>(Operands[i]);
+    const MCConstantExpr *CE = dyn_cast<MCConstantExpr>(Op->getImm());
+    if (CE) break;
+    const MCExpr *E = dyn_cast<MCExpr>(Op->getImm());
+    if (!E) break;
+    const ARMMCExpr *ARM16Expr = dyn_cast<ARMMCExpr>(E);
+    if (!ARM16Expr || (ARM16Expr->getKind() != ARMMCExpr::VK_ARM_HI16 &&
+                       ARM16Expr->getKind() != ARMMCExpr::VK_ARM_LO16)) {
+      return Error(Op->getStartLoc(),
+            "immediate expression for mov requires :lower16: or :upper16");
+      break;
+    }
+    }
   }
 
   return false;

diff --git a/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp b/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
index 5564e0a8c1a84ba88345bf9ff746f5c95fd125e5..c0857e128ec52f1ea0889e6d98631b70f760a619 100644 (file)
--- a/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
+++ b/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
@@ -1040,12 +1040,12 @@ ARMMCCodeEmitter::getHiLo16ImmOpValue(const MCInst &MI, unsigned OpIdx,
     return 0;
   }
   // If the expression doesn't have :upper16: or :lower16: on it,
-  // it's just a plain immediate expression, and those evaluate to
+  // it's just a plain immediate expression, previously those evaluated to
   // the lower 16 bits of the expression regardless of whether
-  // we have a movt or a movw.
-  Kind = MCFixupKind(isThumb2(STI) ? ARM::fixup_t2_movw_lo16
-                                   : ARM::fixup_arm_movw_lo16);
-  Fixups.push_back(MCFixup::Create(0, E, Kind, MI.getLoc()));
+  // we have a movt or a movw, but that led to misleadingly results.
+  // This is now disallowed in the the AsmParser in validateInstruction()
+  // so this should never happen.
+  assert(0 && "expression without :upper16: or :lower16:");
   return 0;
 }
 

diff --git a/test/MC/ARM/arm_fixups.s b/test/MC/ARM/arm_fixups.s
index bd6906bae77f591a91065027fcc0354639e98e1a..1f56e1285245ecfa9df0b869a85ec3086f8197b4 100644 (file)
--- a/test/MC/ARM/arm_fixups.s
+++ b/test/MC/ARM/arm_fixups.s
@@ -26,9 +26,9 @@
 @ CHECK-BE: movt       r9, :upper16:_foo       @ encoding: [0xe3,0b0100AAAA,0x90'A',A]
 @ CHECK-BE: @   fixup A - offset: 0, value: _foo, kind: fixup_arm_movt_hi16
 
-    mov r2, fred
+    mov r2, :lower16:fred
 
-@ CHECK: movw  r2, fred                 @ encoding: [A,0x20'A',0b0000AAAA,0xe3]
+@ CHECK: movw  r2, :lower16:fred                 @ encoding: [A,0x20'A',0b0000AAAA,0xe3]
 @ CHECK: @   fixup A - offset: 0, value: fred, kind: fixup_arm_movw_lo16
-@ CHECK-BE: movw  r2, fred                 @ encoding: [0xe3,0b0000AAAA,0x20'A',A]
+@ CHECK-BE: movw  r2, :lower16:fred                 @ encoding: [0xe3,0b0000AAAA,0x20'A',A]
 @ CHECK-BE: @   fixup A - offset: 0, value: fred, kind: fixup_arm_movw_lo16

diff --git a/test/MC/ARM/complex-operands.s b/test/MC/ARM/complex-operands.s
index 2a721c4e101e0b99126c084f622642cbcc1c3f92..72f8f88d2aa41b6d25b03c5202839590ea8a97fd 100644 (file)
--- a/test/MC/ARM/complex-operands.s
+++ b/test/MC/ARM/complex-operands.s
@@ -21,20 +21,20 @@ return:
        .global arm_function
        .type arm_function,%function
 arm_function:
-       mov r0, #(.L_table_end - .L_table_begin) >> 2
+       mov r0, #:lower16:((.L_table_end - .L_table_begin) >> 2)
        blx return
 
 @ CHECK-LABEL: arm_function
-@ CHECK:       movw r0, #(.L_table_end-.L_table_begin)>>2
+@ CHECK:       movw r0, :lower16:((.L_table_end-.L_table_begin)>>2)
 @ CHECK:       blx return
 
        .global thumb_function
        .type thumb_function,%function
 thumb_function:
-       mov r0, #(.L_table_end - .L_table_begin) >> 2
+       mov r0, #:lower16:((.L_table_end - .L_table_begin) >> 2)
        blx return
 
 @ CHECK-LABEL: thumb_function
-@ CHECK:       movw r0, #(.L_table_end-.L_table_begin)>>2
+@ CHECK:       movw r0, :lower16:((.L_table_end-.L_table_begin)>>2)
 @ CHECK:       blx return
 

diff --git a/test/MC/ARM/diagnostics.s b/test/MC/ARM/diagnostics.s
index 3c26f6d645c83009e18787874133da904c470da2..62d7daea28a146d5a4e40f19038a1dd8eae46696 100644 (file)
--- a/test/MC/ARM/diagnostics.s
+++ b/test/MC/ARM/diagnostics.s
@@ -465,3 +465,11 @@
         ldm sp!, {r0}^
 @ CHECK-ERRORS: error: system STM cannot have writeback register
 @ CHECK-ERRORS: error: writeback register only allowed on system LDM if PC in register-list
+
+foo2:
+        mov r0, foo2
+        movw r0, foo2
+@ CHECK-ERRORS: error: immediate expression for mov requires :lower16: or :upper16
+@ CHECK-ERRORS:                 ^
+@ CHECK-ERRORS: error: immediate expression for mov requires :lower16: or :upper16
+@ CHECK-ERRORS:                  ^

diff --git a/test/MC/ARM/thumb2-diagnostics.s b/test/MC/ARM/thumb2-diagnostics.s
index 6ac2db02cca75cb1c3a8846ef622642434426b1d..bb96db05b92982b82f231bc0c47a5a5aef5b99d3 100644 (file)
--- a/test/MC/ARM/thumb2-diagnostics.s
+++ b/test/MC/ARM/thumb2-diagnostics.s
@@ -70,3 +70,14 @@
 @ CHECK-ERRORS: error: branch target out of range
 @ CHECK-ERRORS: error: branch target out of range
 @ CHECK-ERRORS: error: branch target out of range
+
+foo2:
+        mov r0, foo2
+        movw r0, foo2
+        movt r0, foo2
+@ CHECK-ERRORS: error: immediate expression for mov requires :lower16: or :upper16
+@ CHECK-ERRORS:                 ^
+@ CHECK-ERRORS: error: immediate expression for mov requires :lower16: or :upper16
+@ CHECK-ERRORS:                  ^
+@ CHECK-ERRORS: error: immediate expression for mov requires :lower16: or :upper16
+@ CHECK-ERRORS:                  ^