[bug] Properly validate ADD[S]/SUB[S]/CMP/CMN with extend option

Extend option in ADD, ADDS, SUB, SUBS, CMP, and CMN instructions
doesn't always use the same second register type. For example when
extending from a BYTE the second source register must be W and not
X.

This change makes sure that the assembler accepts the correct
combination and refuses the incorrect one.

IMPORTANT: Although this is not an ABI change, the new behavior
can break AArch64 code that used the incorrect signatures.
This commit is contained in:
kobalicek
2024-11-15 22:02:48 +01:00
parent 439febb13a
commit d28c4be2e7
3 changed files with 54 additions and 17 deletions

View File

@@ -53,6 +53,21 @@ static constexpr uint32_t kWX = InstDB::kWX;
static const uint8_t armShiftOpToLdStOptMap[] = { ASMJIT_LOOKUP_TABLE_16(VALUE, 0) };
#undef VALUE
// a64::Assembler - ExtendOpToRegType
// ==================================
static inline RegType extendOptionToRegType(uint32_t option) noexcept {
uint32_t pred = (uint32_t(RegType::kARM_GpW) << (0x0 * 4)) | // 0b000 - UXTB.
(uint32_t(RegType::kARM_GpW) << (0x1 * 4)) | // 0b001 - UXTH.
(uint32_t(RegType::kARM_GpW) << (0x2 * 4)) | // 0b010 - UXTW.
(uint32_t(RegType::kARM_GpX) << (0x3 * 4)) | // 0b011 - UXTX|LSL.
(uint32_t(RegType::kARM_GpW) << (0x4 * 4)) | // 0b100 - SXTB.
(uint32_t(RegType::kARM_GpW) << (0x5 * 4)) | // 0b101 - SXTH.
(uint32_t(RegType::kARM_GpW) << (0x6 * 4)) | // 0b110 - SXTW.
(uint32_t(RegType::kARM_GpX) << (0x7 * 4)) ; // 0b111 - SXTX.
return RegType((pred >> (option * 4u)) & 0xFu);
}
// asmjit::a64::Assembler - SizeOp
// ===============================
@@ -1228,9 +1243,6 @@ Error Assembler::_emit(InstId instId, const Operand_& o0, const Operand_& o1, co
}
if (isign4 == ENC_OPS3(Reg, Reg, Reg) || isign4 == ENC_OPS4(Reg, Reg, Reg, Imm)) {
if (!checkSignature(o1, o2))
goto InvalidInstruction;
uint32_t opSize = x ? 64 : 32;
uint64_t shift = 0;
uint32_t sType = uint32_t(ShiftOp::kLSL);
@@ -1247,11 +1259,17 @@ Error Assembler::_emit(InstId instId, const Operand_& o0, const Operand_& o1, co
if (sType <= uint32_t(ShiftOp::kASR)) {
bool hasSP = o0.as<Gp>().isSP() || o1.as<Gp>().isSP();
if (!hasSP) {
if (!checkGpId(o0, o1, kZR))
goto InvalidPhysId;
if (!checkSignature(o1, o2)) {
goto InvalidInstruction;
}
if (shift >= opSize)
if (!checkGpId(o0, o1, kZR)) {
goto InvalidPhysId;
}
if (shift >= opSize) {
goto InvalidImmediate;
}
opcode.reset(uint32_t(opData.shiftedOp) << 21);
opcode.addImm(x, 31);
@@ -1264,8 +1282,10 @@ Error Assembler::_emit(InstId instId, const Operand_& o0, const Operand_& o1, co
}
// SP register can only be used with LSL or Extend.
if (sType != uint32_t(ShiftOp::kLSL))
if (sType != uint32_t(ShiftOp::kLSL)) {
goto InvalidImmediate;
}
sType = x ? uint32_t(ShiftOp::kUXTX) : uint32_t(ShiftOp::kUXTW);
}
@@ -1273,8 +1293,9 @@ Error Assembler::_emit(InstId instId, const Operand_& o0, const Operand_& o1, co
opcode.reset(uint32_t(opData.extendedOp) << 21);
sType -= uint32_t(ShiftOp::kUXTB);
if (sType > 7 || shift > 4)
if (sType > 7 || shift > 4) {
goto InvalidImmediate;
}
if (!(opcode.get() & B(29))) {
// ADD|SUB (extend) - ZR is not allowed.
@@ -1287,6 +1308,11 @@ Error Assembler::_emit(InstId instId, const Operand_& o0, const Operand_& o1, co
goto InvalidPhysId;
}
// Validate whether the register operands match extend option.
if (o2.as<Reg>().type() != extendOptionToRegType(sType) || o1.as<Reg>().type() < o2.as<Reg>().type()) {
goto InvalidInstruction;
}
opcode.addImm(x, 31);
opcode.addReg(o2, 16);
opcode.addImm(sType, 13);
@@ -1412,9 +1438,6 @@ Error Assembler::_emit(InstId instId, const Operand_& o0, const Operand_& o1, co
}
if (isign4 == ENC_OPS2(Reg, Reg) || isign4 == ENC_OPS3(Reg, Reg, Imm)) {
if (!checkSignature(o0, o1))
goto InvalidInstruction;
uint32_t opSize = x ? 64 : 32;
uint32_t sType = 0;
uint64_t shift = 0;
@@ -1429,8 +1452,13 @@ Error Assembler::_emit(InstId instId, const Operand_& o0, const Operand_& o1, co
// Shift operation - LSL, LSR, ASR.
if (sType <= uint32_t(ShiftOp::kASR)) {
if (!hasSP) {
if (shift >= opSize)
if (!checkSignature(o0, o1)) {
goto InvalidInstruction;
}
if (shift >= opSize) {
goto InvalidImmediate;
}
opcode.reset(uint32_t(opData.shiftedOp) << 21);
opcode.addImm(x, 31);
@@ -1451,8 +1479,14 @@ Error Assembler::_emit(InstId instId, const Operand_& o0, const Operand_& o1, co
// Extend operation - UXTB, UXTH, UXTW, UXTX, SXTB, SXTH, SXTW, SXTX.
sType -= uint32_t(ShiftOp::kUXTB);
if (sType > 7 || shift > 4)
if (sType > 7 || shift > 4) {
goto InvalidImmediate;
}
// Validate whether the register operands match extend option.
if (o1.as<Reg>().type() != extendOptionToRegType(sType) || o0.as<Reg>().type() < o1.as<Reg>().type()) {
goto InvalidInstruction;
}
opcode.reset(uint32_t(opData.extendedOp) << 21);
opcode.addImm(x, 31);

View File

@@ -42,6 +42,7 @@ static void ASMJIT_NOINLINE testA64AssemblerBase(AssemblerTester<a64::Assembler>
TEST_INSTRUCTION("E103038B", add(x1, xzr, x3));
TEST_INSTRUCTION("5F00030B", add(wzr, w2, w3));
TEST_INSTRUCTION("5F00038B", add(xzr, x2, x3));
TEST_INSTRUCTION("4140238B", add(x1, x2, w3, uxtw(0)));
TEST_INSTRUCTION("83004011", add(w3, w4, 0, lsl(12)));
TEST_INSTRUCTION("83004091", add(x3, x4, 0, lsl(12)));
TEST_INSTRUCTION("83005011", add(w3, w4, 1024, lsl(12)));
@@ -210,7 +211,8 @@ static void ASMJIT_NOINLINE testA64AssemblerBase(AssemblerTester<a64::Assembler>
TEST_INSTRUCTION("3F00022B", cmn(w1, w2));
TEST_INSTRUCTION("3F0002AB", cmn(x1, x2));
TEST_INSTRUCTION("3F08222B", cmn(w1, w2, uxtb(2)));
TEST_INSTRUCTION("3F0822AB", cmn(x1, x2, uxtb(2)));
TEST_INSTRUCTION("3F0822AB", cmn(x1, w2, uxtb(2)));
TEST_INSTRUCTION("5F4023AB", cmn(x2, w3, uxtw(0)));
TEST_INSTRUCTION("FF43212B", cmn(wsp, w1));
TEST_INSTRUCTION("FF07212B", cmn(wsp, w1, uxtb(1)));
TEST_INSTRUCTION("FF6321AB", cmn(sp, x1));
@@ -224,7 +226,8 @@ static void ASMJIT_NOINLINE testA64AssemblerBase(AssemblerTester<a64::Assembler>
TEST_INSTRUCTION("3F00026B", cmp(w1, w2));
TEST_INSTRUCTION("3F0002EB", cmp(x1, x2));
TEST_INSTRUCTION("3F08226B", cmp(w1, w2, uxtb(2)));
TEST_INSTRUCTION("3F0822EB", cmp(x1, x2, uxtb(2)));
TEST_INSTRUCTION("3F0822EB", cmp(x1, w2, uxtb(2)));
TEST_INSTRUCTION("5F4023EB", cmp(x2, w3, uxtw(0)));
TEST_INSTRUCTION("FF43216B", cmp(wsp, w1));
TEST_INSTRUCTION("FF07216B", cmp(wsp, w1, uxtb(1)));
TEST_INSTRUCTION("FF6321EB", cmp(sp, x1));

View File

@@ -177,13 +177,13 @@ static void generateGpSequenceInternal(
cc.cmn(wA, wB);
cc.cmn(xA, xB);
cc.cmn(wA, wB, uxtb(2));
cc.cmn(xA, xB, uxtb(2));
cc.cmn(xA, wB, uxtb(2));
cc.cmp(wA, 33);
cc.cmp(xA, 33);
cc.cmp(wA, wB);
cc.cmp(xA, xB);
cc.cmp(wA, wB, uxtb(2));
cc.cmp(xA, xB, uxtb(2));
cc.cmp(xA, wB, uxtb(2));
cc.crc32b(wA, wB, wC);
cc.crc32b(wzr, wB, wC);
cc.crc32b(wA, wzr, wC);