diff --git a/lib/Target/ARM/AsmParser/ARMAsmParser.cpp b/lib/Target/ARM/AsmParser/ARMAsmParser.cpp index a22d03e20be..0e5fd1f6e0c 100644 --- 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, " + // especially when we turn it into a movw and the expression 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(Operands[i]); + const MCConstantExpr *CE = dyn_cast(Op->getImm()); + if (CE) break; + const MCExpr *E = dyn_cast(Op->getImm()); + if (!E) break; + const ARMMCExpr *ARM16Expr = dyn_cast(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 5564e0a8c1a..c0857e128ec 100644 --- 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 bd6906bae77..1f56e128524 100644 --- 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 2a721c4e101..72f8f88d2aa 100644 --- 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 3c26f6d645c..62d7daea28a 100644 --- 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 6ac2db02cca..bb96db05b92 100644 --- 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: ^