Skip to content

V
V8
Commit 44c52f7b authored by Ben Smith's avatar Ben Smith Committed by Commit Bot
Browse files
Options

Enforce restriction on ARM strex{b,h} instruction 

The strex (Store Exclusive) instruction has the form:

    strex rd, rt, [rn]

It stores the value in register rt at the address in register rn. If the
store succeeds, then 0 is stored in rd, otherwise 1 is stored. The ARM
manual says that behavior is "unpredictable" if d == n || d == t (i.e.
those registers are aliased).

We were not checking for this behavior in the assembler or simulator,
and as a result were generating output where it occurred. This didn't
always break; the tests we run on ARM hardware run this instruction and
pass.

BUG: chromium:786168

Change-Id: I57fe3a1db406eac96eb04ef2246f6970548d3cf9
Reviewed-on: https://chromium-review.googlesource.com/777777Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
Reviewed-by: 's avatarMircea Trofin <mtrofin@chromium.org>
Commit-Queue: Ben Smith <binji@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49513}
parent 0cd6166c
Hide whitespace changes
Inline Side-by-side
Showing with 91 additions and 55 deletions
src/arm/assembler-arm.cc
View file @ 44c52f7b
......@@ -2152,6 +2152,8 @@ void Assembler::strd(Register src1, Register src2,
void Assembler::ldrex(Register dst, Register src, Condition cond) {
// Instruction details available in ARM DDI 0406C.b, A8.8.75.
// cond(31-28) | 00011001(27-20) | Rn(19-16) | Rt(15-12) | 111110011111(11-0)
DCHECK(dst != pc);
DCHECK(src != pc);
emit(cond | B24 | B23 | B20 | src.code() * B16 | dst.code() * B12 | 0xf9f);
}
......@@ -2160,6 +2162,11 @@ void Assembler::strex(Register src1, Register src2, Register dst,
// Instruction details available in ARM DDI 0406C.b, A8.8.212.
// cond(31-28) | 00011000(27-20) | Rn(19-16) | Rd(15-12) | 11111001(11-4) |
// Rt(3-0)
DCHECK(dst != pc);
DCHECK(src1 != pc);
DCHECK(src2 != pc);
DCHECK(src1 != dst);
DCHECK(src1 != src2);
emit(cond | B24 | B23 | dst.code() * B16 | src1.code() * B12 | 0xf9 * B4 |
src2.code());
}
......@@ -2167,6 +2174,8 @@ void Assembler::strex(Register src1, Register src2, Register dst,
void Assembler::ldrexb(Register dst, Register src, Condition cond) {
// Instruction details available in ARM DDI 0406C.b, A8.8.76.
// cond(31-28) | 00011101(27-20) | Rn(19-16) | Rt(15-12) | 111110011111(11-0)
DCHECK(dst != pc);
DCHECK(src != pc);
emit(cond | B24 | B23 | B22 | B20 | src.code() * B16 | dst.code() * B12 |
0xf9f);
}
......@@ -2176,6 +2185,11 @@ void Assembler::strexb(Register src1, Register src2, Register dst,
// Instruction details available in ARM DDI 0406C.b, A8.8.213.
// cond(31-28) | 00011100(27-20) | Rn(19-16) | Rd(15-12) | 11111001(11-4) |
// Rt(3-0)
DCHECK(dst != pc);
DCHECK(src1 != pc);
DCHECK(src2 != pc);
DCHECK(src1 != dst);
DCHECK(src1 != src2);
emit(cond | B24 | B23 | B22 | dst.code() * B16 | src1.code() * B12 |
0xf9 * B4 | src2.code());
}
......@@ -2183,6 +2197,8 @@ void Assembler::strexb(Register src1, Register src2, Register dst,
void Assembler::ldrexh(Register dst, Register src, Condition cond) {
// Instruction details available in ARM DDI 0406C.b, A8.8.78.
// cond(31-28) | 00011111(27-20) | Rn(19-16) | Rt(15-12) | 111110011111(11-0)
DCHECK(dst != pc);
DCHECK(src != pc);
emit(cond | B24 | B23 | B22 | B21 | B20 | src.code() * B16 |
dst.code() * B12 | 0xf9f);
}
......@@ -2192,6 +2208,11 @@ void Assembler::strexh(Register src1, Register src2, Register dst,
// Instruction details available in ARM DDI 0406C.b, A8.8.215.
// cond(31-28) | 00011110(27-20) | Rn(19-16) | Rd(15-12) | 11111001(11-4) |
// Rt(3-0)
DCHECK(dst != pc);
DCHECK(src1 != pc);
DCHECK(src2 != pc);
DCHECK(src1 != dst);
DCHECK(src1 != src2);
emit(cond | B24 | B23 | B22 | B21 | dst.code() * B16 | src1.code() * B12 |
0xf9 * B4 | src2.code());
}
......
src/arm/simulator-arm.cc
View file @ 44c52f7b
......@@ -2188,6 +2188,8 @@ void Simulator::DecodeType01(Instruction* instr) {
int rd = instr->RdValue();
int rt = instr->RmValue();
int rn = instr->RnValue();
DCHECK_NE(rd, rn);
DCHECK_NE(rd, rt);
int32_t addr = get_register(rn);
switch (instr->Bits(22, 21)) {
case 0: {
......
src/compiler/arm/code-generator-arm.cc
View file @ 44c52f7b
......@@ -432,51 +432,51 @@ Condition FlagsConditionToCondition(FlagsCondition condition) {
__ dmb(ISH); \
} while (0)
#define ASSEMBLE_ATOMIC_EXCHANGE_INTEGER(load_instr, store_instr) \
do { \
Label exchange; \
__ dmb(ISH); \
__ bind(&exchange); \
__ add(i.TempRegister(0), i.InputRegister(0), i.InputRegister(1)); \
__ load_instr(i.OutputRegister(0), i.TempRegister(0)); \
__ store_instr(i.TempRegister(0), i.InputRegister(2), i.TempRegister(0)); \
__ teq(i.TempRegister(0), Operand(0)); \
__ b(ne, &exchange); \
__ dmb(ISH); \
#define ASSEMBLE_ATOMIC_EXCHANGE_INTEGER(load_instr, store_instr) \
do { \
Label exchange; \
__ add(i.InputRegister(0), i.InputRegister(0), i.InputRegister(1)); \
__ dmb(ISH); \
__ bind(&exchange); \
__ load_instr(i.OutputRegister(0), i.InputRegister(0)); \
__ store_instr(i.TempRegister(0), i.InputRegister(2), i.InputRegister(0)); \
__ teq(i.TempRegister(0), Operand(0)); \
__ b(ne, &exchange); \
__ dmb(ISH); \
} while (0)
#define ASSEMBLE_ATOMIC_COMPARE_EXCHANGE_INTEGER(load_instr, store_instr) \
#define ASSEMBLE_ATOMIC_COMPARE_EXCHANGE_INTEGER(load_instr, store_instr) \
do { \
Label compareExchange; \
Label exit; \
__ add(i.InputRegister(0), i.InputRegister(0), i.InputRegister(1)); \
__ dmb(ISH); \
__ bind(&compareExchange); \
__ load_instr(i.OutputRegister(0), i.InputRegister(0)); \
__ teq(i.InputRegister(2), Operand(i.OutputRegister(0))); \
__ b(ne, &exit); \
__ store_instr(i.TempRegister(0), i.InputRegister(3), i.InputRegister(0)); \
__ teq(i.TempRegister(0), Operand(0)); \
__ b(ne, &compareExchange); \
__ bind(&exit); \
__ dmb(ISH); \
} while (0)
#define ASSEMBLE_ATOMIC_BINOP(load_instr, store_instr, bin_instr) \
do { \
Label compareExchange; \
Label exit; \
Label binop; \
__ add(i.InputRegister(0), i.InputRegister(0), i.InputRegister(1)); \
__ dmb(ISH); \
__ bind(&compareExchange); \
__ add(i.TempRegister(0), i.InputRegister(0), i.InputRegister(1)); \
__ load_instr(i.OutputRegister(0), i.TempRegister(0)); \
__ teq(i.TempRegister(1), Operand(i.OutputRegister(0))); \
__ b(ne, &exit); \
__ store_instr(i.TempRegister(0), i.InputRegister(3), i.TempRegister(0)); \
__ teq(i.TempRegister(0), Operand(0)); \
__ b(ne, &compareExchange); \
__ bind(&exit); \
__ bind(&binop); \
__ load_instr(i.OutputRegister(0), i.InputRegister(0)); \
__ bin_instr(i.TempRegister(0), i.OutputRegister(0), \
Operand(i.InputRegister(2))); \
__ store_instr(i.TempRegister(1), i.TempRegister(0), i.InputRegister(0)); \
__ teq(i.TempRegister(1), Operand(0)); \
__ b(ne, &binop); \
__ dmb(ISH); \
} while (0)
#define ASSEMBLE_ATOMIC_BINOP(load_instr, store_instr, bin_instr) \
do { \
Label binop; \
__ add(i.TempRegister(0), i.InputRegister(0), i.InputRegister(1)); \
__ dmb(ISH); \
__ bind(&binop); \
__ load_instr(i.OutputRegister(0), i.TempRegister(0)); \
__ bin_instr(i.TempRegister(1), i.OutputRegister(0), \
Operand(i.InputRegister(2))); \
__ store_instr(i.TempRegister(1), i.TempRegister(1), i.TempRegister(0)); \
__ teq(i.TempRegister(1), Operand(0)); \
__ b(ne, &binop); \
__ dmb(ISH); \
} while (0)
#define ASSEMBLE_IEEE754_BINOP(name) \
do { \
/* TODO(bmeurer): We should really get rid of this special instruction, */ \
......@@ -2597,25 +2597,24 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
ASSEMBLE_ATOMIC_EXCHANGE_INTEGER(ldrex, strex);
break;
case kAtomicCompareExchangeInt8:
__ uxtb(i.TempRegister(1), i.InputRegister(2));
__ uxtb(i.InputRegister(2), i.InputRegister(2));
ASSEMBLE_ATOMIC_COMPARE_EXCHANGE_INTEGER(ldrexb, strexb);
__ sxtb(i.OutputRegister(0), i.OutputRegister(0));
break;
case kAtomicCompareExchangeUint8:
__ uxtb(i.TempRegister(1), i.InputRegister(2));
__ uxtb(i.InputRegister(2), i.InputRegister(2));
ASSEMBLE_ATOMIC_COMPARE_EXCHANGE_INTEGER(ldrexb, strexb);
break;
case kAtomicCompareExchangeInt16:
__ uxth(i.TempRegister(1), i.InputRegister(2));
__ uxth(i.InputRegister(2), i.InputRegister(2));
ASSEMBLE_ATOMIC_COMPARE_EXCHANGE_INTEGER(ldrexh, strexh);
__ sxth(i.OutputRegister(0), i.OutputRegister(0));
break;
case kAtomicCompareExchangeUint16:
__ uxth(i.TempRegister(1), i.InputRegister(2));
__ uxth(i.InputRegister(2), i.InputRegister(2));
ASSEMBLE_ATOMIC_COMPARE_EXCHANGE_INTEGER(ldrexh, strexh);
break;
case kAtomicCompareExchangeWord32:
__ mov(i.TempRegister(1), i.InputRegister(2));
ASSEMBLE_ATOMIC_COMPARE_EXCHANGE_INTEGER(ldrex, strex);
break;
#define ATOMIC_BINOP_CASE(op, inst) \
......@@ -2642,6 +2641,19 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
ATOMIC_BINOP_CASE(Or, orr)
ATOMIC_BINOP_CASE(Xor, eor)
#undef ATOMIC_BINOP_CASE
#undef ASSEMBLE_CHECKED_LOAD_FP
#undef ASSEMBLE_CHECKED_LOAD_INTEGER
#undef ASSEMBLE_CHECKED_STORE_FP
#undef ASSEMBLE_CHECKED_STORE_INTEGER
#undef ASSEMBLE_ATOMIC_LOAD_INTEGER
#undef ASSEMBLE_ATOMIC_STORE_INTEGER
#undef ASSEMBLE_ATOMIC_EXCHANGE_INTEGER
#undef ASSEMBLE_ATOMIC_COMPARE_EXCHANGE_INTEGER
#undef ASSEMBLE_ATOMIC_BINOP
#undef ASSEMBLE_IEEE754_BINOP
#undef ASSEMBLE_IEEE754_UNOP
#undef ASSEMBLE_NEON_NARROWING_OP
#undef ASSEMBLE_NEON_PAIRWISE_OP
}
return kSuccess;
} // NOLINT(readability/fn_size)
......@@ -3242,6 +3254,7 @@ void CodeGenerator::AssembleJumpTable(Label** targets, size_t target_count) {
}
#undef __
#undef kScratchReg
} // namespace compiler
} // namespace internal
......
src/compiler/arm/instruction-selector-arm.cc
View file @ 44c52f7b
......@@ -2277,10 +2277,10 @@ void InstructionSelector::VisitAtomicExchange(Node* node) {
InstructionOperand inputs[3];
size_t input_count = 0;
inputs[input_count++] = g.UseUniqueRegister(base);
inputs[input_count++] = g.UseUniqueRegister(index);
inputs[input_count++] = g.UseRegister(index);
inputs[input_count++] = g.UseUniqueRegister(value);
InstructionOperand outputs[1];
outputs[0] = g.UseUniqueRegister(node);
outputs[0] = g.DefineAsRegister(node);
InstructionOperand temp[1];
temp[0] = g.TempRegister();
InstructionCode code = opcode | AddressingModeField::encode(addressing_mode);
......@@ -2314,16 +2314,15 @@ void InstructionSelector::VisitAtomicCompareExchange(Node* node) {
InstructionOperand inputs[4];
size_t input_count = 0;
inputs[input_count++] = g.UseUniqueRegister(base);
inputs[input_count++] = g.UseUniqueRegister(index);
inputs[input_count++] = g.UseRegister(index);
inputs[input_count++] = g.UseUniqueRegister(old_value);
inputs[input_count++] = g.UseUniqueRegister(new_value);
InstructionOperand outputs[1];
outputs[0] = g.UseUniqueRegister(node);
InstructionOperand temp[2];
outputs[0] = g.DefineAsRegister(node);
InstructionOperand temp[1];
temp[0] = g.TempRegister();
temp[1] = g.TempRegister();
InstructionCode code = opcode | AddressingModeField::encode(addressing_mode);
Emit(code, 1, outputs, input_count, inputs, 2, temp);
Emit(code, 1, outputs, input_count, inputs, 1, temp);
}
void InstructionSelector::VisitAtomicBinaryOperation(
......@@ -2354,15 +2353,16 @@ void InstructionSelector::VisitAtomicBinaryOperation(
InstructionOperand inputs[3];
size_t input_count = 0;
inputs[input_count++] = g.UseUniqueRegister(base);
inputs[input_count++] = g.UseUniqueRegister(index);
inputs[input_count++] = g.UseRegister(index);
inputs[input_count++] = g.UseUniqueRegister(value);
InstructionOperand outputs[1];
outputs[0] = g.UseUniqueRegister(node);
outputs[0] = g.DefineAsRegister(node);
InstructionOperand temps[2];
temps[0] = g.TempRegister();
temps[1] = g.TempRegister();
size_t temp_count = 0;
temps[temp_count++] = g.TempRegister();
temps[temp_count++] = g.TempRegister();
InstructionCode code = opcode | AddressingModeField::encode(addressing_mode);
Emit(code, 1, outputs, input_count, inputs, 2, temps);
Emit(code, 1, outputs, input_count, inputs, temp_count, temps);
}
#define VISIT_ATOMIC_BINOP(op) \
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment