From 1d13a12fbc2d33f1ff50d96a49935416cc5d0f6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=A4rkl?= Date: Sun, 6 Oct 2024 21:13:14 +0200 Subject: [PATCH] AArch64: Replace vararg add_cs_detail by multiple concrete functions Fixes UB caused by various mismatches on how these arguments are passed and read. This became visible when running on PowerPC hosts with e.g. `cstool -d aarch64 204862f8`. Apart from the UB fix, this is meant to be a pure refactor. Partially addresses #2458 --- arch/AArch64/AArch64InstPrinter.c | 169 ++++++------ arch/AArch64/AArch64InstPrinter.h | 6 +- arch/AArch64/AArch64Mapping.c | 244 ++---------------- arch/AArch64/AArch64Mapping.h | 24 +- .../cpptranslator/Tests/test_patches.py | 2 +- .../cpptranslator/patches/AddCSDetail.py | 20 +- tests/issues/issues.yaml | 25 ++ 7 files changed, 167 insertions(+), 323 deletions(-) diff --git a/arch/AArch64/AArch64InstPrinter.c b/arch/AArch64/AArch64InstPrinter.c index 6fe5ca9f1..8a5cd0b1c 100644 --- a/arch/AArch64/AArch64InstPrinter.c +++ b/arch/AArch64/AArch64InstPrinter.c @@ -905,8 +905,9 @@ bool printSyspAlias(MCInst *MI, SStream *O) void CONCAT(printMatrix, EltSize)(MCInst * MI, unsigned OpNum, \ SStream *O) \ { \ - add_cs_detail(MI, CONCAT(AArch64_OP_GROUP_Matrix, EltSize), \ - OpNum, EltSize); \ + AArch64_add_cs_detail_1( \ + MI, CONCAT(AArch64_OP_GROUP_Matrix, EltSize), OpNum, \ + EltSize); \ MCOperand *RegOp = MCInst_getOperand(MI, (OpNum)); \ \ printRegName(O, MCOperand_getReg(RegOp)); \ @@ -941,10 +942,10 @@ DEFINE_printMatrix(0); void CONCAT(printMatrixTileVector, \ IsVertical)(MCInst * MI, unsigned OpNum, SStream *O) \ { \ - add_cs_detail(MI, \ - CONCAT(AArch64_OP_GROUP_MatrixTileVector, \ - IsVertical), \ - OpNum, IsVertical); \ + AArch64_add_cs_detail_1( \ + MI, \ + CONCAT(AArch64_OP_GROUP_MatrixTileVector, IsVertical), \ + OpNum, IsVertical); \ MCOperand *RegOp = MCInst_getOperand(MI, (OpNum)); \ \ const char *RegName = getRegisterName(MCOperand_getReg(RegOp), \ @@ -970,7 +971,7 @@ DEFINE_printMatrixTileVector(1); void printMatrixTile(MCInst *MI, unsigned OpNum, SStream *O) { - add_cs_detail(MI, AArch64_OP_GROUP_MatrixTile, OpNum); + AArch64_add_cs_detail_0(MI, AArch64_OP_GROUP_MatrixTile, OpNum); MCOperand *RegOp = MCInst_getOperand(MI, (OpNum)); printRegName(O, MCOperand_getReg(RegOp)); @@ -978,7 +979,7 @@ void printMatrixTile(MCInst *MI, unsigned OpNum, SStream *O) void printSVCROp(MCInst *MI, unsigned OpNum, SStream *O) { - add_cs_detail(MI, AArch64_OP_GROUP_SVCROp, OpNum); + AArch64_add_cs_detail_0(MI, AArch64_OP_GROUP_SVCROp, OpNum); MCOperand *MO = MCInst_getOperand(MI, (OpNum)); unsigned svcrop = MCOperand_getImm(MO); @@ -989,7 +990,7 @@ void printSVCROp(MCInst *MI, unsigned OpNum, SStream *O) void printOperand(MCInst *MI, unsigned OpNo, SStream *O) { - add_cs_detail(MI, AArch64_OP_GROUP_Operand, OpNo); + AArch64_add_cs_detail_0(MI, AArch64_OP_GROUP_Operand, OpNo); MCOperand *Op = MCInst_getOperand(MI, (OpNo)); if (MCOperand_isReg(Op)) { unsigned Reg = MCOperand_getReg(Op); @@ -1006,7 +1007,7 @@ void printOperand(MCInst *MI, unsigned OpNo, SStream *O) void printImm(MCInst *MI, unsigned OpNo, SStream *O) { - add_cs_detail(MI, AArch64_OP_GROUP_Imm, OpNo); + AArch64_add_cs_detail_0(MI, AArch64_OP_GROUP_Imm, OpNo); MCOperand *Op = MCInst_getOperand(MI, (OpNo)); SStream_concat(O, "%s", markup(" AArch64_PN15) \ @@ -1316,7 +1317,7 @@ DEFINE_printPredicateAsCounter(0); void printCondCode(MCInst *MI, unsigned OpNum, SStream *O) { - add_cs_detail(MI, AArch64_OP_GROUP_CondCode, OpNum); + AArch64_add_cs_detail_0(MI, AArch64_OP_GROUP_CondCode, OpNum); AArch64CC_CondCode CC = (AArch64CC_CondCode)MCOperand_getImm( MCInst_getOperand(MI, (OpNum))); SStream_concat0(O, AArch64CC_getCondCodeName(CC)); @@ -1324,7 +1325,7 @@ void printCondCode(MCInst *MI, unsigned OpNum, SStream *O) void printInverseCondCode(MCInst *MI, unsigned OpNum, SStream *O) { - add_cs_detail(MI, AArch64_OP_GROUP_InverseCondCode, OpNum); + AArch64_add_cs_detail_0(MI, AArch64_OP_GROUP_InverseCondCode, OpNum); AArch64CC_CondCode CC = (AArch64CC_CondCode)MCOperand_getImm( MCInst_getOperand(MI, (OpNum))); SStream_concat0(O, AArch64CC_getCondCodeName( @@ -1333,7 +1334,7 @@ void printInverseCondCode(MCInst *MI, unsigned OpNum, SStream *O) void printAMNoIndex(MCInst *MI, unsigned OpNum, SStream *O) { - add_cs_detail(MI, AArch64_OP_GROUP_AMNoIndex, OpNum); + AArch64_add_cs_detail_0(MI, AArch64_OP_GROUP_AMNoIndex, OpNum); SStream_concat0(O, "["); printRegName(O, MCOperand_getReg(MCInst_getOperand(MI, (OpNum)))); @@ -1344,8 +1345,9 @@ void printAMNoIndex(MCInst *MI, unsigned OpNum, SStream *O) void CONCAT(printImmScale, Scale)(MCInst * MI, unsigned OpNum, \ SStream *O) \ { \ - add_cs_detail(MI, CONCAT(AArch64_OP_GROUP_ImmScale, Scale), \ - OpNum, Scale); \ + AArch64_add_cs_detail_1( \ + MI, CONCAT(AArch64_OP_GROUP_ImmScale, Scale), OpNum, \ + Scale); \ SStream_concat(O, "%s", markup(" 1) @@ -2315,8 +2321,9 @@ void printSVEVecLenSpecifier(MCInst *MI, unsigned OpNum, SStream *O) void CONCAT(printSVERegOp, suffix)(MCInst * MI, unsigned OpNum, \ SStream *O) \ { \ - add_cs_detail(MI, CONCAT(AArch64_OP_GROUP_SVERegOp, suffix), \ - OpNum, CHAR(suffix)); \ + AArch64_add_cs_detail_1( \ + MI, CONCAT(AArch64_OP_GROUP_SVERegOp, suffix), OpNum, \ + CHAR(suffix)); \ switch (CHAR(suffix)) { \ case '0': \ case 'b': \ @@ -2394,8 +2401,8 @@ DEFINE_isSignedType(uint64_t); void CONCAT(printImm8OptLsl, T)(MCInst * MI, unsigned OpNum, \ SStream *O) \ { \ - add_cs_detail(MI, CONCAT(AArch64_OP_GROUP_Imm8OptLsl, T), \ - OpNum, sizeof(T)); \ + AArch64_add_cs_detail_1( \ + MI, CONCAT(AArch64_OP_GROUP_Imm8OptLsl, T), OpNum, sizeof(T)); \ unsigned UnscaledVal = \ MCOperand_getImm(MCInst_getOperand(MI, (OpNum))); \ unsigned Shift = \ @@ -2434,8 +2441,9 @@ DEFINE_printImm8OptLsl(uint32_t); void CONCAT(printSVELogicalImm, T)(MCInst * MI, unsigned OpNum, \ SStream *O) \ { \ - add_cs_detail(MI, CONCAT(AArch64_OP_GROUP_SVELogicalImm, T), \ - OpNum, sizeof(T)); \ + AArch64_add_cs_detail_1( \ + MI, CONCAT(AArch64_OP_GROUP_SVELogicalImm, T), OpNum, \ + sizeof(T)); \ typedef T SignedT; \ typedef CONCATS(u, T) UnsignedT; \ \ @@ -2462,8 +2470,9 @@ DEFINE_printSVELogicalImm(int64_t); void CONCAT(printZPRasFPR, Width)(MCInst * MI, unsigned OpNum, \ SStream *O) \ { \ - add_cs_detail(MI, CONCAT(AArch64_OP_GROUP_ZPRasFPR, Width), \ - OpNum, Width); \ + AArch64_add_cs_detail_1( \ + MI, CONCAT(AArch64_OP_GROUP_ZPRasFPR, Width), OpNum, \ + Width); \ unsigned Base; \ switch (Width) { \ case 8: \ @@ -2498,7 +2507,7 @@ DEFINE_printZPRasFPR(128); void CONCAT(printExactFPImm, CONCAT(ImmIs0, ImmIs1))( \ MCInst * MI, unsigned OpNum, SStream *O) \ { \ - add_cs_detail( \ + AArch64_add_cs_detail_2( \ MI, \ CONCAT(CONCAT(AArch64_OP_GROUP_ExactFPImm, ImmIs0), \ ImmIs1), \ @@ -2519,14 +2528,14 @@ DEFINE_printExactFPImm(AArch64ExactFPImm_half, AArch64ExactFPImm_two); void printGPR64as32(MCInst *MI, unsigned OpNum, SStream *O) { - add_cs_detail(MI, AArch64_OP_GROUP_GPR64as32, OpNum); + AArch64_add_cs_detail_0(MI, AArch64_OP_GROUP_GPR64as32, OpNum); unsigned Reg = MCOperand_getReg(MCInst_getOperand(MI, (OpNum))); printRegName(O, getWRegFromXReg(Reg)); } void printGPR64x8(MCInst *MI, unsigned OpNum, SStream *O) { - add_cs_detail(MI, AArch64_OP_GROUP_GPR64x8, OpNum); + AArch64_add_cs_detail_0(MI, AArch64_OP_GROUP_GPR64x8, OpNum); unsigned Reg = MCOperand_getReg(MCInst_getOperand(MI, (OpNum))); printRegName(O, MCRegisterInfo_getSubReg(MI->MRI, Reg, AArch64_x8sub_0)); @@ -2534,7 +2543,7 @@ void printGPR64x8(MCInst *MI, unsigned OpNum, SStream *O) void printSyspXzrPair(MCInst *MI, unsigned OpNum, SStream *O) { - add_cs_detail(MI, AArch64_OP_GROUP_SyspXzrPair, OpNum); + AArch64_add_cs_detail_0(MI, AArch64_OP_GROUP_SyspXzrPair, OpNum); unsigned Reg = MCOperand_getReg(MCInst_getOperand(MI, (OpNum))); SStream_concat(O, "%s%s", getRegisterName(Reg, AArch64_NoRegAltName), diff --git a/arch/AArch64/AArch64InstPrinter.h b/arch/AArch64/AArch64InstPrinter.h index 44c86953f..536598ed1 100644 --- a/arch/AArch64/AArch64InstPrinter.h +++ b/arch/AArch64/AArch64InstPrinter.h @@ -76,7 +76,7 @@ void printPostIncOperand(MCInst *MI, unsigned OpNo, unsigned Imm, SStream *O); static inline void CONCAT(printPostIncOperand, Amount)( \ MCInst * MI, unsigned OpNo, SStream *O) \ { \ - add_cs_detail(MI, \ + AArch64_add_cs_detail_1(MI, \ CONCAT(AArch64_OP_GROUP_PostIncOperand, Amount), \ OpNo, Amount); \ printPostIncOperand(MI, OpNo, Amount, O); \ @@ -118,7 +118,7 @@ void printMemExtend(MCInst *MI, unsigned OpNum, SStream *O, char SrcRegKind, static inline void CONCAT(printMemExtend, CONCAT(SrcRegKind, Width))( \ MCInst * MI, unsigned OpNum, SStream *O) \ { \ - add_cs_detail( \ + AArch64_add_cs_detail_2( \ MI, \ CONCAT(CONCAT(AArch64_OP_GROUP_MemExtend, SrcRegKind), \ Width), \ @@ -182,7 +182,7 @@ void printAMIndexedWB(MCInst *MI, unsigned OpNum, unsigned Scale, SStream *O); static inline void CONCAT(printUImm12Offset, Scale)( \ MCInst * MI, unsigned OpNum, SStream *O) \ { \ - add_cs_detail(MI, \ + AArch64_add_cs_detail_1(MI, \ CONCAT(AArch64_OP_GROUP_UImm12Offset, Scale), \ OpNum, Scale); \ printUImm12Offset(MI, OpNum, Scale, O); \ diff --git a/arch/AArch64/AArch64Mapping.c b/arch/AArch64/AArch64Mapping.c index 71ac20347..cfe103e54 100644 --- a/arch/AArch64/AArch64Mapping.c +++ b/arch/AArch64/AArch64Mapping.c @@ -1281,13 +1281,29 @@ void AArch64_set_mem_access(MCInst *MI, bool status) } } +/// Common prefix for all AArch64_add_cs_detail_* functions +static bool add_cs_detail_begin(MCInst *MI, unsigned op_num) +{ + if (!detail_is_set(MI) || !map_fill_detail_ops(MI)) + return false; + + if (AArch64_get_detail(MI)->is_doing_sme) { + // Unset the flag if there is no bound operand anymore. + if (!(map_get_op_type(MI, op_num) & CS_OP_BOUND)) { + AArch64_get_detail(MI)->is_doing_sme = false; + AArch64_inc_op_count(MI); + } + } + return true; +} + /// Fills cs_detail with the data of the operand. /// This function handles operands which's original printer function has no /// specialities. -static void add_cs_detail_general(MCInst *MI, aarch64_op_group op_group, +void AArch64_add_cs_detail_0(MCInst *MI, aarch64_op_group op_group, unsigned OpNum) { - if (!detail_is_set(MI)) + if (!add_cs_detail_begin(MI, OpNum)) return; // Fill cs_detail @@ -1501,7 +1517,7 @@ static void add_cs_detail_general(MCInst *MI, aarch64_op_group op_group, break; case AArch64_OP_GROUP_ImplicitlyTypedVectorList: // The TypedVectorList implements the logic of implicitly typed operand. - add_cs_detail(MI, AArch64_OP_GROUP_TypedVectorList_0_b, OpNum, + AArch64_add_cs_detail_2(MI, AArch64_OP_GROUP_TypedVectorList_0_b, OpNum, 0, 0); break; case AArch64_OP_GROUP_InverseCondCode: { @@ -1724,10 +1740,10 @@ static void add_cs_detail_general(MCInst *MI, aarch64_op_group op_group, /// Fills cs_detail with the data of the operand. /// This function handles operands which original printer function is a template /// with one argument. -static void add_cs_detail_template_1(MCInst *MI, aarch64_op_group op_group, +void AArch64_add_cs_detail_1(MCInst *MI, aarch64_op_group op_group, unsigned OpNum, uint64_t temp_arg_0) { - if (!detail_is_set(MI)) + if (!add_cs_detail_begin(MI, OpNum)) return; switch (op_group) { default: @@ -2052,11 +2068,11 @@ static void add_cs_detail_template_1(MCInst *MI, aarch64_op_group op_group, /// Fills cs_detail with the data of the operand. /// This function handles operands which original printer function is a template /// with two arguments. -static void add_cs_detail_template_2(MCInst *MI, aarch64_op_group op_group, +void AArch64_add_cs_detail_2(MCInst *MI, aarch64_op_group op_group, unsigned OpNum, uint64_t temp_arg_0, uint64_t temp_arg_1) { - if (!detail_is_set(MI)) + if (!add_cs_detail_begin(MI, OpNum)) return; switch (op_group) { default: @@ -2235,12 +2251,12 @@ static void add_cs_detail_template_2(MCInst *MI, aarch64_op_group op_group, /// Fills cs_detail with the data of the operand. /// This function handles operands which original printer function is a template /// with four arguments. -static void add_cs_detail_template_4(MCInst *MI, aarch64_op_group op_group, +void AArch64_add_cs_detail_4(MCInst *MI, aarch64_op_group op_group, unsigned OpNum, uint64_t temp_arg_0, uint64_t temp_arg_1, uint64_t temp_arg_2, uint64_t temp_arg_3) { - if (!detail_is_set(MI)) + if (!add_cs_detail_begin(MI, OpNum)) return; switch (op_group) { default: @@ -2302,216 +2318,6 @@ static void add_cs_detail_template_4(MCInst *MI, aarch64_op_group op_group, } } -void AArch64_add_cs_detail(MCInst *MI, int /* aarch64_op_group */ op_group, - va_list args) -{ - if (!detail_is_set(MI) || !map_fill_detail_ops(MI)) - return; - - unsigned op_num = va_arg(args, unsigned); - if (AArch64_get_detail(MI)->is_doing_sme) { - // Unset the flag if there is no bound operand anymore. - if (!(map_get_op_type(MI, op_num) & CS_OP_BOUND)) { - AArch64_get_detail(MI)->is_doing_sme = false; - AArch64_inc_op_count(MI); - } - } - - switch (op_group) { - default: - printf("Operand group %d not handled\n", op_group); - break; - case AArch64_OP_GROUP_AddSubImm: - case AArch64_OP_GROUP_AdrLabel: - case AArch64_OP_GROUP_AdrpLabel: - case AArch64_OP_GROUP_AdrAdrpLabel: - case AArch64_OP_GROUP_AlignedLabel: - case AArch64_OP_GROUP_AMNoIndex: - case AArch64_OP_GROUP_ArithExtend: - case AArch64_OP_GROUP_BarriernXSOption: - case AArch64_OP_GROUP_BarrierOption: - case AArch64_OP_GROUP_BTIHintOp: - case AArch64_OP_GROUP_CondCode: - case AArch64_OP_GROUP_ExtendedRegister: - case AArch64_OP_GROUP_FPImmOperand: - case AArch64_OP_GROUP_GPR64as32: - case AArch64_OP_GROUP_GPR64x8: - case AArch64_OP_GROUP_Imm: - case AArch64_OP_GROUP_ImmHex: - case AArch64_OP_GROUP_ImplicitlyTypedVectorList: - case AArch64_OP_GROUP_InverseCondCode: - case AArch64_OP_GROUP_MatrixTile: - case AArch64_OP_GROUP_MatrixTileList: - case AArch64_OP_GROUP_MRSSystemRegister: - case AArch64_OP_GROUP_MSRSystemRegister: - case AArch64_OP_GROUP_Operand: - case AArch64_OP_GROUP_PSBHintOp: - case AArch64_OP_GROUP_RPRFMOperand: - case AArch64_OP_GROUP_ShiftedRegister: - case AArch64_OP_GROUP_Shifter: - case AArch64_OP_GROUP_SIMDType10Operand: - case AArch64_OP_GROUP_SVCROp: - case AArch64_OP_GROUP_SVEPattern: - case AArch64_OP_GROUP_SVEVecLenSpecifier: - case AArch64_OP_GROUP_SysCROperand: - case AArch64_OP_GROUP_SyspXzrPair: - case AArch64_OP_GROUP_SystemPStateField: - case AArch64_OP_GROUP_VRegOperand: { - add_cs_detail_general(MI, op_group, op_num); - break; - } - case AArch64_OP_GROUP_GPRSeqPairsClassOperand_32: - case AArch64_OP_GROUP_GPRSeqPairsClassOperand_64: - case AArch64_OP_GROUP_Imm8OptLsl_int16_t: - case AArch64_OP_GROUP_Imm8OptLsl_int32_t: - case AArch64_OP_GROUP_Imm8OptLsl_int64_t: - case AArch64_OP_GROUP_Imm8OptLsl_int8_t: - case AArch64_OP_GROUP_Imm8OptLsl_uint16_t: - case AArch64_OP_GROUP_Imm8OptLsl_uint32_t: - case AArch64_OP_GROUP_Imm8OptLsl_uint64_t: - case AArch64_OP_GROUP_Imm8OptLsl_uint8_t: - case AArch64_OP_GROUP_ImmScale_16: - case AArch64_OP_GROUP_ImmScale_2: - case AArch64_OP_GROUP_ImmScale_3: - case AArch64_OP_GROUP_ImmScale_32: - case AArch64_OP_GROUP_ImmScale_4: - case AArch64_OP_GROUP_ImmScale_8: - case AArch64_OP_GROUP_LogicalImm_int16_t: - case AArch64_OP_GROUP_LogicalImm_int32_t: - case AArch64_OP_GROUP_LogicalImm_int64_t: - case AArch64_OP_GROUP_LogicalImm_int8_t: - case AArch64_OP_GROUP_Matrix_0: - case AArch64_OP_GROUP_Matrix_16: - case AArch64_OP_GROUP_Matrix_32: - case AArch64_OP_GROUP_Matrix_64: - case AArch64_OP_GROUP_MatrixIndex_0: - case AArch64_OP_GROUP_MatrixIndex_1: - case AArch64_OP_GROUP_MatrixIndex_8: - case AArch64_OP_GROUP_MatrixTileVector_0: - case AArch64_OP_GROUP_MatrixTileVector_1: - case AArch64_OP_GROUP_PostIncOperand_1: - case AArch64_OP_GROUP_PostIncOperand_12: - case AArch64_OP_GROUP_PostIncOperand_16: - case AArch64_OP_GROUP_PostIncOperand_2: - case AArch64_OP_GROUP_PostIncOperand_24: - case AArch64_OP_GROUP_PostIncOperand_3: - case AArch64_OP_GROUP_PostIncOperand_32: - case AArch64_OP_GROUP_PostIncOperand_4: - case AArch64_OP_GROUP_PostIncOperand_48: - case AArch64_OP_GROUP_PostIncOperand_6: - case AArch64_OP_GROUP_PostIncOperand_64: - case AArch64_OP_GROUP_PostIncOperand_8: - case AArch64_OP_GROUP_PredicateAsCounter_0: - case AArch64_OP_GROUP_PredicateAsCounter_16: - case AArch64_OP_GROUP_PredicateAsCounter_32: - case AArch64_OP_GROUP_PredicateAsCounter_64: - case AArch64_OP_GROUP_PredicateAsCounter_8: - case AArch64_OP_GROUP_PrefetchOp_0: - case AArch64_OP_GROUP_PrefetchOp_1: - case AArch64_OP_GROUP_SImm_16: - case AArch64_OP_GROUP_SImm_8: - case AArch64_OP_GROUP_SVELogicalImm_int16_t: - case AArch64_OP_GROUP_SVELogicalImm_int32_t: - case AArch64_OP_GROUP_SVELogicalImm_int64_t: - case AArch64_OP_GROUP_SVERegOp_0: - case AArch64_OP_GROUP_SVERegOp_b: - case AArch64_OP_GROUP_SVERegOp_d: - case AArch64_OP_GROUP_SVERegOp_h: - case AArch64_OP_GROUP_SVERegOp_q: - case AArch64_OP_GROUP_SVERegOp_s: - case AArch64_OP_GROUP_UImm12Offset_1: - case AArch64_OP_GROUP_UImm12Offset_16: - case AArch64_OP_GROUP_UImm12Offset_2: - case AArch64_OP_GROUP_UImm12Offset_4: - case AArch64_OP_GROUP_UImm12Offset_8: - case AArch64_OP_GROUP_VectorIndex_1: - case AArch64_OP_GROUP_VectorIndex_8: - case AArch64_OP_GROUP_ZPRasFPR_128: - case AArch64_OP_GROUP_ZPRasFPR_16: - case AArch64_OP_GROUP_ZPRasFPR_32: - case AArch64_OP_GROUP_ZPRasFPR_64: - case AArch64_OP_GROUP_ZPRasFPR_8: { - uint64_t temp_arg_0 = va_arg(args, uint64_t); - add_cs_detail_template_1(MI, op_group, op_num, temp_arg_0); - break; - } - case AArch64_OP_GROUP_ComplexRotationOp_180_90: - case AArch64_OP_GROUP_ComplexRotationOp_90_0: - case AArch64_OP_GROUP_ExactFPImm_AArch64ExactFPImm_half_AArch64ExactFPImm_one: - case AArch64_OP_GROUP_ExactFPImm_AArch64ExactFPImm_half_AArch64ExactFPImm_two: - case AArch64_OP_GROUP_ExactFPImm_AArch64ExactFPImm_zero_AArch64ExactFPImm_one: - case AArch64_OP_GROUP_ImmRangeScale_2_1: - case AArch64_OP_GROUP_ImmRangeScale_4_3: - case AArch64_OP_GROUP_MemExtend_w_128: - case AArch64_OP_GROUP_MemExtend_w_16: - case AArch64_OP_GROUP_MemExtend_w_32: - case AArch64_OP_GROUP_MemExtend_w_64: - case AArch64_OP_GROUP_MemExtend_w_8: - case AArch64_OP_GROUP_MemExtend_x_128: - case AArch64_OP_GROUP_MemExtend_x_16: - case AArch64_OP_GROUP_MemExtend_x_32: - case AArch64_OP_GROUP_MemExtend_x_64: - case AArch64_OP_GROUP_MemExtend_x_8: - case AArch64_OP_GROUP_TypedVectorList_0_b: - case AArch64_OP_GROUP_TypedVectorList_0_d: - case AArch64_OP_GROUP_TypedVectorList_0_h: - case AArch64_OP_GROUP_TypedVectorList_0_q: - case AArch64_OP_GROUP_TypedVectorList_0_s: - case AArch64_OP_GROUP_TypedVectorList_0_0: - case AArch64_OP_GROUP_TypedVectorList_16_b: - case AArch64_OP_GROUP_TypedVectorList_1_d: - case AArch64_OP_GROUP_TypedVectorList_2_d: - case AArch64_OP_GROUP_TypedVectorList_2_s: - case AArch64_OP_GROUP_TypedVectorList_4_h: - case AArch64_OP_GROUP_TypedVectorList_4_s: - case AArch64_OP_GROUP_TypedVectorList_8_b: - case AArch64_OP_GROUP_TypedVectorList_8_h: { - uint64_t temp_arg_0 = va_arg(args, uint64_t); - uint64_t temp_arg_1 = va_arg(args, uint64_t); - add_cs_detail_template_2(MI, op_group, op_num, temp_arg_0, - temp_arg_1); - break; - } - case AArch64_OP_GROUP_RegWithShiftExtend_0_128_x_0: - case AArch64_OP_GROUP_RegWithShiftExtend_0_16_w_d: - case AArch64_OP_GROUP_RegWithShiftExtend_0_16_w_s: - case AArch64_OP_GROUP_RegWithShiftExtend_0_16_x_0: - case AArch64_OP_GROUP_RegWithShiftExtend_0_16_x_d: - case AArch64_OP_GROUP_RegWithShiftExtend_0_16_x_s: - case AArch64_OP_GROUP_RegWithShiftExtend_0_32_w_d: - case AArch64_OP_GROUP_RegWithShiftExtend_0_32_w_s: - case AArch64_OP_GROUP_RegWithShiftExtend_0_32_x_0: - case AArch64_OP_GROUP_RegWithShiftExtend_0_32_x_d: - case AArch64_OP_GROUP_RegWithShiftExtend_0_32_x_s: - case AArch64_OP_GROUP_RegWithShiftExtend_0_64_w_d: - case AArch64_OP_GROUP_RegWithShiftExtend_0_64_w_s: - case AArch64_OP_GROUP_RegWithShiftExtend_0_64_x_0: - case AArch64_OP_GROUP_RegWithShiftExtend_0_64_x_d: - case AArch64_OP_GROUP_RegWithShiftExtend_0_64_x_s: - case AArch64_OP_GROUP_RegWithShiftExtend_0_8_w_d: - case AArch64_OP_GROUP_RegWithShiftExtend_0_8_w_s: - case AArch64_OP_GROUP_RegWithShiftExtend_0_8_x_0: - case AArch64_OP_GROUP_RegWithShiftExtend_0_8_x_d: - case AArch64_OP_GROUP_RegWithShiftExtend_0_8_x_s: - case AArch64_OP_GROUP_RegWithShiftExtend_1_16_w_d: - case AArch64_OP_GROUP_RegWithShiftExtend_1_16_w_s: - case AArch64_OP_GROUP_RegWithShiftExtend_1_32_w_d: - case AArch64_OP_GROUP_RegWithShiftExtend_1_32_w_s: - case AArch64_OP_GROUP_RegWithShiftExtend_1_64_w_d: - case AArch64_OP_GROUP_RegWithShiftExtend_1_64_w_s: - case AArch64_OP_GROUP_RegWithShiftExtend_1_8_w_d: - case AArch64_OP_GROUP_RegWithShiftExtend_1_8_w_s: { - uint64_t temp_arg_0 = va_arg(args, uint64_t); - uint64_t temp_arg_1 = va_arg(args, uint64_t); - uint64_t temp_arg_2 = va_arg(args, uint64_t); - uint64_t temp_arg_3 = va_arg(args, uint64_t); - add_cs_detail_template_4(MI, op_group, op_num, temp_arg_0, - temp_arg_1, temp_arg_2, temp_arg_3); - break; - } - } -} - /// Adds a register AArch64 operand at position OpNum and increases the op_count by /// one. void AArch64_set_detail_op_reg(MCInst *MI, unsigned OpNum, aarch64_reg Reg) diff --git a/arch/AArch64/AArch64Mapping.h b/arch/AArch64/AArch64Mapping.h index 481346be1..9c232bd26 100644 --- a/arch/AArch64/AArch64Mapping.h +++ b/arch/AArch64/AArch64Mapping.h @@ -36,19 +36,17 @@ void AArch64_reg_access(const cs_insn *insn, cs_regs regs_read, uint8_t *regs_read_count, cs_regs regs_write, uint8_t *regs_write_count); -void AArch64_add_cs_detail(MCInst *MI, int /* aarch64_op_group */ op_group, - va_list args); - -static inline void add_cs_detail(MCInst *MI, - int /* aarch64_op_group */ op_group, ...) -{ - if (!MI->flat_insn->detail) - return; - va_list args; - va_start(args, op_group); - AArch64_add_cs_detail(MI, op_group, args); - va_end(args); -} +void AArch64_add_cs_detail_0(MCInst *MI, aarch64_op_group op_group, + unsigned OpNum); +void AArch64_add_cs_detail_1(MCInst *MI, aarch64_op_group op_group, + unsigned OpNum, uint64_t temp_arg_0); +void AArch64_add_cs_detail_2(MCInst *MI, aarch64_op_group op_group, + unsigned OpNum, uint64_t temp_arg_0, + uint64_t temp_arg_1); +void AArch64_add_cs_detail_4(MCInst *MI, aarch64_op_group op_group, + unsigned OpNum, uint64_t temp_arg_0, + uint64_t temp_arg_1, uint64_t temp_arg_2, + uint64_t temp_arg_3); void AArch64_init_mri(MCRegisterInfo *MRI); diff --git a/suite/auto-sync/src/autosync/cpptranslator/Tests/test_patches.py b/suite/auto-sync/src/autosync/cpptranslator/Tests/test_patches.py index ab198bf16..79361c5cb 100644 --- a/suite/auto-sync/src/autosync/cpptranslator/Tests/test_patches.py +++ b/suite/auto-sync/src/autosync/cpptranslator/Tests/test_patches.py @@ -117,7 +117,7 @@ class TestPatches(unittest.TestCase): patch, syntax, b"static inline void printThumbLdrLabelOperand(MCInst *MI, unsigned OpNo, SStream *O){ " - b"add_cs_detail(MI, ARCH_OP_GROUP_ThumbLdrLabelOperand, OpNo); " + b"ARCH_add_cs_detail_0(MI, ARCH_OP_GROUP_ThumbLdrLabelOperand, OpNo); " b"int i = OpNo; " b"}", ) diff --git a/suite/auto-sync/src/autosync/cpptranslator/patches/AddCSDetail.py b/suite/auto-sync/src/autosync/cpptranslator/patches/AddCSDetail.py index bf79cdfdc..2a0a74de1 100644 --- a/suite/auto-sync/src/autosync/cpptranslator/patches/AddCSDetail.py +++ b/suite/auto-sync/src/autosync/cpptranslator/patches/AddCSDetail.py @@ -16,17 +16,17 @@ from autosync.cpptranslator.patches.Patch import Patch class AddCSDetail(Patch): """ - Adds calls to `add_cs_detail()` for printOperand functions in InstPrinter.c + Adds calls to `_add_cs_detail_()` for printOperand functions in InstPrinter.c Patch void printThumbLdrLabelOperand(MCInst *MI, unsigned OpNo, SStream *O) {...} to void printThumbLdrLabelOperand(MCInst *MI, unsigned OpNo, SStream *O) { - add_cs_detail(MI, ARM_OP_GROUP_ThumbLdrLabelOperand, ...); + _add_cs_detail_(MI, ARM_OP_GROUP_ThumbLdrLabelOperand, ...); ... } """ # TODO Simply checking for the passed types would be so much nicer. - # Parameter lists of printOperand() functions we need to add `add_cs_detail()` to. + # Parameter lists of printOperand() functions we need to add `_add_cs_detail_()` to. # Spaces are removed, so we only need to check the letters. valid_param_lists = [ b"(MCInst*MI,unsignedOpNum,SStream*O)", # Default printOperand parameters. @@ -59,7 +59,9 @@ class AddCSDetail(Patch): def get_main_capture_name(self) -> str: return "print_op" - def get_patch(self, captures: [(Node, str)], src: bytes, **kwargs) -> bytes: + def get_patch( + self, captures: list[tuple[Node, str]], src: bytes, **kwargs + ) -> bytes: fcn_def: Node = captures[0][0] params = captures[2][0] params = get_text(src, params.start_byte, params.end_byte) @@ -105,7 +107,7 @@ class AddCSDetail(Patch): # Standard printOperand() parameters mcinst_var = get_MCInst_var_name(src, fcn_def) return ( - b"add_cs_detail(" + f"{self.arch}_add_cs_detail_0(".encode() + mcinst_var + b", " + op_group_enum @@ -114,7 +116,11 @@ class AddCSDetail(Patch): + b");" ) elif op_group_enum == b"ARM_OP_GROUP_RegImmShift": - return b"add_cs_detail(MI, " + op_group_enum + b", ShOpc, ShImm);" + return ( + f"{self.arch}_add_cs_detail_1(MI, ".encode() + + op_group_enum + + b", ShOpc, ShImm);" + ) elif is_template and op_num_var_name in params: mcinst_var = get_MCInst_var_name(src, fcn_def) templ_p = template_param_list_to_dict(fcn_def.prev_sibling) @@ -125,7 +131,7 @@ class AddCSDetail(Patch): ) cs_args += b", " + tp["identifier"] return ( - b"add_cs_detail(" + f"{self.arch}_add_cs_detail_{len(templ_p)}(".encode() + mcinst_var + b", " + op_group_enum diff --git a/tests/issues/issues.yaml b/tests/issues/issues.yaml index c99f2ce2b..6adbd7da9 100644 --- a/tests/issues/issues.yaml +++ b/tests/issues/issues.yaml @@ -5506,3 +5506,28 @@ test_cases: asm_text: "call #0x7fffffcc" - asm_text: "call #0x7fffffe0" + - + input: + name: "issue 2458 -- UB on PPC because vargs are not casted (AArch64)" + bytes: [ 0x20, 0x48, 0x62, 0xf8 ] + arch: "CS_ARCH_AARCH64" + options: [ CS_OPT_DETAIL ] + address: 0x0 + expected: + insns: + - + asm_text: "ldr x0, [x1, w2, uxtw]" + details: + aarch64: + operands: + - + type: AARCH64_OP_REG + reg: x0 + access: CS_AC_WRITE + - + type: AARCH64_OP_MEM + mem_base: x1 + mem_index: w2 + access: CS_AC_READ + regs_read: [ x1, w2 ] + regs_write: [ x0 ]