From a08aec1b41161c6dae85144c563148a619542c73 Mon Sep 17 00:00:00 2001 From: Jiajie Chen Date: Thu, 26 Sep 2024 01:02:12 +0800 Subject: [PATCH 1/7] i#2297: AARCH64: Implement cbr instrumentation Implement cbr instrumentation for AARCH64, supporting cbz/cbnz/tbz/tbnz/bcond opcodes. Issue: #2297 --- core/ir/aarch64/instr_create_api.h | 12 ++- core/lib/instrument.c | 140 +++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 3 deletions(-) diff --git a/core/ir/aarch64/instr_create_api.h b/core/ir/aarch64/instr_create_api.h index e6fb46b4a95..e428aaa535a 100644 --- a/core/ir/aarch64/instr_create_api.h +++ b/core/ir/aarch64/instr_create_api.h @@ -658,13 +658,19 @@ instr_create_0dst_3src((dc), OP_tbnz, (pc), (reg), (imm)) #define INSTR_CREATE_cmp(dc, rn, rm_or_imm) \ INSTR_CREATE_subs(dc, OPND_CREATE_ZR(rn), rn, rm_or_imm) -#define INSTR_CREATE_eor(dc, d, s) \ - INSTR_CREATE_eor_shift(dc, d, d, s, OPND_CREATE_INT8(DR_SHIFT_LSL), \ - OPND_CREATE_INT8(0)) +#define INSTR_CREATE_eor(dc, d, s_or_imm) \ + opnd_is_immed(s_or_imm) \ + ? instr_create_1dst_2src(dc, OP_eor, d, d, s_or_imm) \ + : INSTR_CREATE_eor_shift(dc, d, d, s_or_imm, OPND_CREATE_INT8(DR_SHIFT_LSL), \ + OPND_CREATE_INT8(0)) #define INSTR_CREATE_eor_shift(dc, rd, rn, rm, sht, sha) \ instr_create_1dst_4src(dc, OP_eor, rd, rn, \ opnd_create_reg_ex(opnd_get_reg(rm), 0, DR_OPND_SHIFTED), \ opnd_add_flags(sht, DR_OPND_IS_SHIFT), sha) +#define INSTR_CREATE_csinc(dc, rd, rn, rm, cond) \ + instr_create_1dst_3src(dc, OP_csinc, rd, rn, rm, cond) +#define INSTR_CREATE_ubfm(dc, rd, rn, lsb, width) \ + instr_create_1dst_3src(dc, OP_ubfm, rd, rn, lsb, width) #define INSTR_CREATE_ldp(dc, rt1, rt2, mem) \ instr_create_2dst_1src(dc, OP_ldp, rt1, rt2, mem) diff --git a/core/lib/instrument.c b/core/lib/instrument.c index e7c2f9b1446..f386feac63d 100644 --- a/core/lib/instrument.c +++ b/core/lib/instrument.c @@ -6328,6 +6328,146 @@ dr_insert_cbr_instrumentation_help(void *drcontext, instrlist_t *ilist, instr_t #elif defined(RISCV64) /* FIXME i#3544: Not implemented */ ASSERT_NOT_IMPLEMENTED(false); +#elif defined(AARCH64) + dcontext_t *dcontext = (dcontext_t *)drcontext; + ptr_uint_t address, target; + reg_id_t dir = DR_REG_NULL; + reg_id_t flags = DR_REG_NULL; + int opc; + ; + CLIENT_ASSERT(drcontext != NULL, + "dr_insert_cbr_instrumentation: drcontext cannot be NULL"); + address = (ptr_uint_t)instr_get_translation(instr); + CLIENT_ASSERT(address != 0, + "dr_insert_cbr_instrumentation: can't determine app address"); + CLIENT_ASSERT(instr_is_cbr(instr), + "dr_insert_cbr_instrumentation must be applied to a cbr"); + target = (ptr_uint_t)opnd_get_pc(instr_get_target(instr)); + + /* Compute branch direction */ + opc = instr_get_opcode(instr); + if (opc == OP_cbnz || opc == OP_cbz) { + opnd_t reg_op = instr_get_src(instr, 1); + reg_id_t reg = opnd_get_reg(reg_op); + /* use dir register to compute direction */ + dir = (reg == DR_REG_X0 || reg == DR_REG_W0) ? DR_REG_X1 : DR_REG_X0; + /* save old value of dir register to SPILL_SLOT_1 */ + dr_save_reg(dcontext, ilist, instr, dir, SPILL_SLOT_1); + /* use flags register to save nzcv */ + flags = (reg == DR_REG_X2 || reg == DR_REG_W2) ? DR_REG_X3 : DR_REG_X2; + /* save old value of flags register to SPILL_SLOT_2 */ + dr_save_reg(dcontext, ilist, instr, flags, SPILL_SLOT_2); + /* save flags to flags register */ + dr_save_arith_flags_to_reg(dcontext, ilist, instr, flags); + + /* compare reg against zero */ + instr_t *cmp = INSTR_CREATE_cmp(dcontext, reg_op, OPND_CREATE_INT(0)); + MINSERT(ilist, instr, cmp); + /* compute branch direction */ + opnd_t dir_op = opnd_create_reg(dir); + instr_t *cset = INSTR_CREATE_csinc( + dcontext, dir_op, OPND_CREATE_ZR(dir_op), OPND_CREATE_ZR(dir_op), + opnd_create_cond(opc == OP_cbnz ? DR_PRED_EQ : DR_PRED_NE)); + MINSERT(ilist, instr, cset); + } else if (opc == OP_tbnz || opc == OP_tbz) { + opnd_t reg_op = instr_get_src(instr, 1); + reg_id_t reg = opnd_get_reg(reg_op); + reg_id_t dir_same_width = DR_REG_NULL; + + /* use dir register to compute direction */ + if (DR_REG_START_64 <= reg && reg <= DR_REG_STOP_64) { + /* 64-bit register */ + dir = (reg == DR_REG_X0) ? DR_REG_X1 : DR_REG_X0; + dir_same_width = (reg == DR_REG_X0) ? DR_REG_X1 : DR_REG_X0; + } else { + /* 32-bit register */ + dir = (reg == DR_REG_W0) ? DR_REG_X1 : DR_REG_X0; + dir_same_width = (reg == DR_REG_W0) ? DR_REG_W1 : DR_REG_W0; + } + /* save old value of dir register to SPILL_SLOT_1 */ + dr_save_reg(dcontext, ilist, instr, dir, SPILL_SLOT_1); + + /* extract tst_bit from reg */ + int tst_bit = opnd_get_immed_int(instr_get_src(instr, 2)); + opnd_t dir_same_width_op = opnd_create_reg(dir_same_width); + instr_t *ubfm = + INSTR_CREATE_ubfm(dcontext, dir_same_width_op, reg_op, + OPND_CREATE_INT(tst_bit), OPND_CREATE_INT(tst_bit)); + MINSERT(ilist, instr, ubfm); + + /* invert result if tbz */ + if (opc == OP_tbz) { + instr_t *eor = + INSTR_CREATE_eor(dcontext, dir_same_width_op, OPND_CREATE_INT(1)); + MINSERT(ilist, instr, eor); + } + } else if (opc == OP_bcond) { + /* use dir register to compute direction */ + dir = SCRATCH_REG0; + /* save old value of dir register to SPILL_SLOT_1 */ + dr_save_reg(dcontext, ilist, instr, dir, SPILL_SLOT_1); + /* compute branch direction */ + dr_pred_type_t pred = instr_get_predicate(instr); + opnd_t dir_op = opnd_create_reg(dir); + instr_t *cset = INSTR_CREATE_csinc( + dcontext, dir_op, OPND_CREATE_ZR(dir_op), OPND_CREATE_ZR(dir_op), + opnd_create_cond(instr_invert_predicate(pred))); + MINSERT(ilist, instr, cset); + } else { + CLIENT_ASSERT(false, "unknown conditional branch type"); + return; + } + + if (has_fallthrough) { + ptr_uint_t fallthrough = address + instr_length(drcontext, instr); + CLIENT_ASSERT(fallthrough > address, "wrong fallthrough address"); + dr_insert_clean_call_ex( + drcontext, ilist, instr, callee, + /* Many users will ask for mcontexts; some will set; it doesn't seem worth + * asking the user to pass in a flag: if they're using this they are not + * super concerned about overhead. + */ + DR_CLEANCALL_READS_APP_CONTEXT | DR_CLEANCALL_WRITES_APP_CONTEXT, 5, + /* address of cbr is 1st parameter */ + OPND_CREATE_INTPTR(address), + /* target is 2nd parameter */ + OPND_CREATE_INTPTR(target), + /* fall-through is 3rd parameter */ + OPND_CREATE_INTPTR(fallthrough), + /* branch direction is 4th parameter */ + opnd_create_reg(dir), + /* user defined data is 5th parameter */ + opnd_is_null(user_data) ? OPND_CREATE_INT32(0) : user_data); + } else { + dr_insert_clean_call_ex( + drcontext, ilist, instr, callee, + /* Many users will ask for mcontexts; some will set; it doesn't seem worth + * asking the user to pass in a flag: if they're using this they are not + * super concerned about overhead. + */ + DR_CLEANCALL_READS_APP_CONTEXT | DR_CLEANCALL_WRITES_APP_CONTEXT, 3, + /* address of cbr is 1st parameter */ + OPND_CREATE_INTPTR(address), + /* target is 2nd parameter */ + OPND_CREATE_INTPTR(target), + /* branch direction is 3rd parameter */ + opnd_create_reg(dir)); + } + + /* Restore state */ + if (opc == OP_cbnz || opc == OP_cbz) { + /* restore arith flags */ + dr_restore_arith_flags_from_reg(dcontext, ilist, instr, flags); + /* restore old value of flags register */ + dr_restore_reg(dcontext, ilist, instr, flags, SPILL_SLOT_2); + /* restore old value of dir register */ + dr_restore_reg(dcontext, ilist, instr, dir, SPILL_SLOT_1); + } else if (opc == OP_bcond || opc == OP_tbnz || opc == OP_tbz) { + /* restore old value of dir register */ + dr_restore_reg(dcontext, ilist, instr, dir, SPILL_SLOT_1); + } else { + CLIENT_ASSERT(false, "unknown conditional branch type"); + } #endif /* X86/ARM/RISCV64 */ } From 01872bac5798d6ca5646714af5bc97a2b5345410 Mon Sep 17 00:00:00 2001 From: Jiajie Chen Date: Thu, 26 Sep 2024 11:32:05 +0800 Subject: [PATCH 2/7] i#2297: Document and refactor new instr_create_api for AARCH64 Document newly added instruction creation macros for EOR, CSINC and UBFM. Refactor XINST_CREATE_slr_s to use the new macro to create ubfm instruction. Fixes #2297 --- core/ir/aarch64/instr_create_api.h | 45 ++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/core/ir/aarch64/instr_create_api.h b/core/ir/aarch64/instr_create_api.h index e428aaa535a..91e404ba343 100644 --- a/core/ir/aarch64/instr_create_api.h +++ b/core/ir/aarch64/instr_create_api.h @@ -463,12 +463,12 @@ * they just need to know whether they need to preserve the app's flags, so maybe * we can just document that this may not write them. */ -#define XINST_CREATE_slr_s(dc, d, rm_or_imm) \ - (opnd_is_reg(rm_or_imm) \ - ? instr_create_1dst_2src(dc, OP_lsrv, d, d, rm_or_imm) \ - : instr_create_1dst_3src(dc, OP_ubfm, d, d, rm_or_imm, \ - reg_is_32bit(opnd_get_reg(d)) ? OPND_CREATE_INT(31) \ - : OPND_CREATE_INT(63))) +#define XINST_CREATE_slr_s(dc, d, rm_or_imm) \ + (opnd_is_reg(rm_or_imm) \ + ? instr_create_1dst_2src(dc, OP_lsrv, d, d, rm_or_imm) \ + : INSTR_CREATE_ubfm(dc, d, d, rm_or_imm, \ + reg_is_32bit(opnd_get_reg(d)) ? OPND_CREATE_INT(31) \ + : OPND_CREATE_INT(63))) /** * This platform-independent macro creates an instr_t for a nop instruction. @@ -658,6 +658,15 @@ instr_create_0dst_3src((dc), OP_tbnz, (pc), (reg), (imm)) #define INSTR_CREATE_cmp(dc, rn, rm_or_imm) \ INSTR_CREATE_subs(dc, OPND_CREATE_ZR(rn), rn, rm_or_imm) + +/** + * Creates an EOR instruction with one output and two inputs. For simplicity, the first + * input reuses the output register. + * + * \param dc The void * dcontext used to allocate memory for the instr_t. + * \param d The output register and the first input register. + * \param s_or_imm The second input register or immediate. + */ #define INSTR_CREATE_eor(dc, d, s_or_imm) \ opnd_is_immed(s_or_imm) \ ? instr_create_1dst_2src(dc, OP_eor, d, d, s_or_imm) \ @@ -667,10 +676,30 @@ instr_create_1dst_4src(dc, OP_eor, rd, rn, \ opnd_create_reg_ex(opnd_get_reg(rm), 0, DR_OPND_SHIFTED), \ opnd_add_flags(sht, DR_OPND_IS_SHIFT), sha) + +/** + * Creates a CSINC instruction with one output and three inputs. + * + * \param dc The void * dcontext used to allocate memory for the instr_t. + * \param rd The output register. + * \param rn The first input register. + * \param rm The second input register. + * \param cond The third input condition code. + */ #define INSTR_CREATE_csinc(dc, rd, rn, rm, cond) \ instr_create_1dst_3src(dc, OP_csinc, rd, rn, rm, cond) -#define INSTR_CREATE_ubfm(dc, rd, rn, lsb, width) \ - instr_create_1dst_3src(dc, OP_ubfm, rd, rn, lsb, width) + +/** + * Creates an UBFM instruction with one output and three inputs. + * + * \param dc The void * dcontext used to allocate memory for the instr_t. + * \param rd The output register. + * \param rn The first input register. + * \param immr The second input immediate. + * \param imms The third input immediate. + */ +#define INSTR_CREATE_ubfm(dc, rd, rn, immr, imms) \ + instr_create_1dst_3src(dc, OP_ubfm, rd, rn, immr, imms) #define INSTR_CREATE_ldp(dc, rt1, rt2, mem) \ instr_create_2dst_1src(dc, OP_ldp, rt1, rt2, mem) From 79e33755b66f76c672f81c216904dca6d70d78ef Mon Sep 17 00:00:00 2001 From: Jiajie Chen Date: Fri, 27 Sep 2024 10:07:32 +0800 Subject: [PATCH 3/7] i#2297: Enable cbr tests on AARCH64 --- api/samples/CMakeLists.txt | 6 ++++-- suite/tests/CMakeLists.txt | 8 +++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/api/samples/CMakeLists.txt b/api/samples/CMakeLists.txt index 39a95f45537..c05ba0f880e 100644 --- a/api/samples/CMakeLists.txt +++ b/api/samples/CMakeLists.txt @@ -265,10 +265,12 @@ if (X86) # FIXME i#1551, i#1569, i#3544: port to ARM/AArch64/RISCV64 add_sample_client(modxfer "modxfer.c;utils.c" "drmgr;drreg;drx") add_sample_client(modxfer_app2lib "modxfer_app2lib.c" "drmgr") add_sample_client(instrcalls "instrcalls.c;utils.c" "drmgr;drsyms;drx") - # dr_insert_cbr_instrument_ex is NYI - add_sample_client(cbrtrace "cbrtrace.c;utils.c" "drmgr;drx") add_sample_client(hot_bbcount "hot_bbcount.c" "drmgr;drreg;drbbdup;drx") endif (X86) +if (X86 OR AARCH64) + # dr_insert_cbr_instrument_ex is NYI + add_sample_client(cbrtrace "cbrtrace.c;utils.c" "drmgr;drx") +endif (X86 OR AARCH64) add_sample_client(wrap "wrap.c" "drmgr;drwrap") add_sample_client(signal "signal.c" "drmgr") add_sample_client(syscall "syscall.c" "drmgr") diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index 577fe842833..72be275d20d 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -2419,13 +2419,15 @@ if (X86) # FIXME i#1551, i#1569: port to ARM and AArch64 client-interface/cleancall.c "" "-thread_private -prof_pcs -opt_cleancall 0" "") endif (NOT X64) endif (UNIX) + tobuild_ci(client.syscall client-interface/syscall.c "" "-no_follow_children" "") + tobuild_ci(client.count-bbs client-interface/count-bbs.c "" "" "") +endif (X86) +if (X86 OR AARCH64) tobuild_ci(client.count-ctis client-interface/count-ctis.c "" "" "") # check dr_insert_cbr_instrumentation with out-of-line clean call torunonly_ci(client.count-ctis-noopt client.count-ctis client.count-ctis.dll client-interface/count-ctis.c "" "-opt_cleancall 0" "") - tobuild_ci(client.syscall client-interface/syscall.c "" "-no_follow_children" "") - tobuild_ci(client.count-bbs client-interface/count-bbs.c "" "" "") -endif (X86) +endif (X86 OR AARCH64) if (NOT RISCV64) # TODO i#3544: Port tests to RISC-V 64 tobuild_ci(client.cleancallparams client-interface/cleancallparams.c "" "" "") tobuild_ci(client.app_inscount client-interface/app_inscount.c "" "" "") From b5155c3b943a026377531c55d3d03eba8997dd12 Mon Sep 17 00:00:00 2001 From: Jiajie Chen Date: Fri, 27 Sep 2024 10:11:51 +0800 Subject: [PATCH 4/7] i#2297: Refactor code and conform to code style --- core/ir/aarch64/instr_create_api.h | 2 +- core/lib/instrument.c | 72 +++++++++++++----------------- 2 files changed, 33 insertions(+), 41 deletions(-) diff --git a/core/ir/aarch64/instr_create_api.h b/core/ir/aarch64/instr_create_api.h index 91e404ba343..e4fd2029d65 100644 --- a/core/ir/aarch64/instr_create_api.h +++ b/core/ir/aarch64/instr_create_api.h @@ -690,7 +690,7 @@ instr_create_1dst_3src(dc, OP_csinc, rd, rn, rm, cond) /** - * Creates an UBFM instruction with one output and three inputs. + * Creates a UBFM instruction with one output and three inputs. * * \param dc The void * dcontext used to allocate memory for the instr_t. * \param rd The output register. diff --git a/core/lib/instrument.c b/core/lib/instrument.c index f386feac63d..c83b94e294b 100644 --- a/core/lib/instrument.c +++ b/core/lib/instrument.c @@ -6334,7 +6334,6 @@ dr_insert_cbr_instrumentation_help(void *drcontext, instrlist_t *ilist, instr_t reg_id_t dir = DR_REG_NULL; reg_id_t flags = DR_REG_NULL; int opc; - ; CLIENT_ASSERT(drcontext != NULL, "dr_insert_cbr_instrumentation: drcontext cannot be NULL"); address = (ptr_uint_t)instr_get_translation(instr); @@ -6344,26 +6343,26 @@ dr_insert_cbr_instrumentation_help(void *drcontext, instrlist_t *ilist, instr_t "dr_insert_cbr_instrumentation must be applied to a cbr"); target = (ptr_uint_t)opnd_get_pc(instr_get_target(instr)); - /* Compute branch direction */ + /* Compute branch direction. */ opc = instr_get_opcode(instr); if (opc == OP_cbnz || opc == OP_cbz) { opnd_t reg_op = instr_get_src(instr, 1); reg_id_t reg = opnd_get_reg(reg_op); - /* use dir register to compute direction */ - dir = (reg == DR_REG_X0 || reg == DR_REG_W0) ? DR_REG_X1 : DR_REG_X0; - /* save old value of dir register to SPILL_SLOT_1 */ + /* Use dir register to compute direction. */ + dir = (reg_to_pointer_sized(reg) == DR_REG_X0) ? DR_REG_X1 : DR_REG_X0; + /* Save old value of dir register to SPILL_SLOT_1. */ dr_save_reg(dcontext, ilist, instr, dir, SPILL_SLOT_1); - /* use flags register to save nzcv */ - flags = (reg == DR_REG_X2 || reg == DR_REG_W2) ? DR_REG_X3 : DR_REG_X2; - /* save old value of flags register to SPILL_SLOT_2 */ + /* Use flags register to save nzcv. */ + flags = (reg_to_pointer_sized(reg) == DR_REG_X2) ? DR_REG_X3 : DR_REG_X2; + /* Save old value of flags register to SPILL_SLOT_2. */ dr_save_reg(dcontext, ilist, instr, flags, SPILL_SLOT_2); - /* save flags to flags register */ + /* Save flags to flags register. */ dr_save_arith_flags_to_reg(dcontext, ilist, instr, flags); - /* compare reg against zero */ + /* Compare reg against zero. */ instr_t *cmp = INSTR_CREATE_cmp(dcontext, reg_op, OPND_CREATE_INT(0)); MINSERT(ilist, instr, cmp); - /* compute branch direction */ + /* Compute branch direction. */ opnd_t dir_op = opnd_create_reg(dir); instr_t *cset = INSTR_CREATE_csinc( dcontext, dir_op, OPND_CREATE_ZR(dir_op), OPND_CREATE_ZR(dir_op), @@ -6374,20 +6373,13 @@ dr_insert_cbr_instrumentation_help(void *drcontext, instrlist_t *ilist, instr_t reg_id_t reg = opnd_get_reg(reg_op); reg_id_t dir_same_width = DR_REG_NULL; - /* use dir register to compute direction */ - if (DR_REG_START_64 <= reg && reg <= DR_REG_STOP_64) { - /* 64-bit register */ - dir = (reg == DR_REG_X0) ? DR_REG_X1 : DR_REG_X0; - dir_same_width = (reg == DR_REG_X0) ? DR_REG_X1 : DR_REG_X0; - } else { - /* 32-bit register */ - dir = (reg == DR_REG_W0) ? DR_REG_X1 : DR_REG_X0; - dir_same_width = (reg == DR_REG_W0) ? DR_REG_W1 : DR_REG_W0; - } - /* save old value of dir register to SPILL_SLOT_1 */ + /* Use dir register to compute direction. */ + dir = (reg_to_pointer_sized(reg) == DR_REG_X0) ? DR_REG_X1 : DR_REG_X0; + dir_same_width = reg_resize_to_opsz(dir, reg_get_size(reg)); + /* Save old value of dir register to SPILL_SLOT_1. */ dr_save_reg(dcontext, ilist, instr, dir, SPILL_SLOT_1); - /* extract tst_bit from reg */ + /* Extract tst_bit from reg. */ int tst_bit = opnd_get_immed_int(instr_get_src(instr, 2)); opnd_t dir_same_width_op = opnd_create_reg(dir_same_width); instr_t *ubfm = @@ -6395,18 +6387,18 @@ dr_insert_cbr_instrumentation_help(void *drcontext, instrlist_t *ilist, instr_t OPND_CREATE_INT(tst_bit), OPND_CREATE_INT(tst_bit)); MINSERT(ilist, instr, ubfm); - /* invert result if tbz */ + /* Invert result if tbz. */ if (opc == OP_tbz) { instr_t *eor = INSTR_CREATE_eor(dcontext, dir_same_width_op, OPND_CREATE_INT(1)); MINSERT(ilist, instr, eor); } } else if (opc == OP_bcond) { - /* use dir register to compute direction */ + /* Use dir register to compute direction. */ dir = SCRATCH_REG0; - /* save old value of dir register to SPILL_SLOT_1 */ + /* Save old value of dir register to SPILL_SLOT_1. */ dr_save_reg(dcontext, ilist, instr, dir, SPILL_SLOT_1); - /* compute branch direction */ + /* Compute branch direction. */ dr_pred_type_t pred = instr_get_predicate(instr); opnd_t dir_op = opnd_create_reg(dir); instr_t *cset = INSTR_CREATE_csinc( @@ -6428,15 +6420,15 @@ dr_insert_cbr_instrumentation_help(void *drcontext, instrlist_t *ilist, instr_t * super concerned about overhead. */ DR_CLEANCALL_READS_APP_CONTEXT | DR_CLEANCALL_WRITES_APP_CONTEXT, 5, - /* address of cbr is 1st parameter */ + /* Address of cbr is 1st parameter. */ OPND_CREATE_INTPTR(address), - /* target is 2nd parameter */ + /* Target is 2nd parameter. */ OPND_CREATE_INTPTR(target), - /* fall-through is 3rd parameter */ + /* Fall-through is 3rd parameter. */ OPND_CREATE_INTPTR(fallthrough), - /* branch direction is 4th parameter */ + /* Branch direction is 4th parameter. */ opnd_create_reg(dir), - /* user defined data is 5th parameter */ + /* User defined data is 5th parameter. */ opnd_is_null(user_data) ? OPND_CREATE_INT32(0) : user_data); } else { dr_insert_clean_call_ex( @@ -6446,29 +6438,29 @@ dr_insert_cbr_instrumentation_help(void *drcontext, instrlist_t *ilist, instr_t * super concerned about overhead. */ DR_CLEANCALL_READS_APP_CONTEXT | DR_CLEANCALL_WRITES_APP_CONTEXT, 3, - /* address of cbr is 1st parameter */ + /* Address of cbr is 1st parameter. */ OPND_CREATE_INTPTR(address), - /* target is 2nd parameter */ + /* Target is 2nd parameter. */ OPND_CREATE_INTPTR(target), - /* branch direction is 3rd parameter */ + /* Branch direction is 3rd parameter. */ opnd_create_reg(dir)); } /* Restore state */ if (opc == OP_cbnz || opc == OP_cbz) { - /* restore arith flags */ + /* Restore arith flags. */ dr_restore_arith_flags_from_reg(dcontext, ilist, instr, flags); - /* restore old value of flags register */ + /* Restore old value of flags register. */ dr_restore_reg(dcontext, ilist, instr, flags, SPILL_SLOT_2); - /* restore old value of dir register */ + /* Restore old value of dir register. */ dr_restore_reg(dcontext, ilist, instr, dir, SPILL_SLOT_1); } else if (opc == OP_bcond || opc == OP_tbnz || opc == OP_tbz) { - /* restore old value of dir register */ + /* Restore old value of dir register. */ dr_restore_reg(dcontext, ilist, instr, dir, SPILL_SLOT_1); } else { CLIENT_ASSERT(false, "unknown conditional branch type"); } -#endif /* X86/ARM/RISCV64 */ +#endif /* X86/ARM/RISCV64/AARCH64 */ } DR_API void From 78f6037393e98d0bf08e6c0d1c76388b08e8d63d Mon Sep 17 00:00:00 2001 From: Jiajie Chen Date: Fri, 27 Sep 2024 12:43:28 +0800 Subject: [PATCH 5/7] i#2297: Add AARCH64 support to count-ctis test --- suite/tests/client-interface/count-ctis.c | 31 +++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/suite/tests/client-interface/count-ctis.c b/suite/tests/client-interface/count-ctis.c index 26552f83c37..49a766fd866 100644 --- a/suite/tests/client-interface/count-ctis.c +++ b/suite/tests/client-interface/count-ctis.c @@ -51,6 +51,7 @@ main() #else /* asm code *************************************************************/ # include "asm_defines.asm" /* clang-format off */ +#ifdef X86 START_FILE #define FUNCNAME test_jecxz @@ -95,5 +96,35 @@ GLOBAL_LABEL(FUNCNAME:) END_FILE +#elif defined(AARCH64) +START_FILE + +#define FUNCNAME test_jecxz + DECLARE_FUNC(FUNCNAME) +GLOBAL_LABEL(FUNCNAME:) + mov x1, ARG1 + END_PROLOG + + /* test cbnz */ + mov x1, 0 + cbz x1, jecxz_taken + nop + jecxz_taken: + mov x1, 1 + cbz x1, jecxz_taken + nop + jecxz_not_taken: + nop + + /* test x0 being preserved */ + ldr x1, =0xabcd1234 + str x1, [x0] + + ret + END_FUNC(FUNCNAME) + + +END_FILE +#endif /* clang-format on */ #endif From 22e365dcf6391de87914ee28630d05e0e023fcff Mon Sep 17 00:00:00 2001 From: Jiajie Chen Date: Fri, 27 Sep 2024 09:38:40 +0800 Subject: [PATCH 6/7] i#2919: Implement mbr instrumentation on AARCH64 Issue: #2919 --- api/samples/CMakeLists.txt | 3 ++- core/lib/instrument.c | 43 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/api/samples/CMakeLists.txt b/api/samples/CMakeLists.txt index c05ba0f880e..c079a3fe5fb 100644 --- a/api/samples/CMakeLists.txt +++ b/api/samples/CMakeLists.txt @@ -264,10 +264,11 @@ if (X86) # FIXME i#1551, i#1569, i#3544: port to ARM/AArch64/RISCV64 # dr_insert_mbr_instrumentation is NYI add_sample_client(modxfer "modxfer.c;utils.c" "drmgr;drreg;drx") add_sample_client(modxfer_app2lib "modxfer_app2lib.c" "drmgr") - add_sample_client(instrcalls "instrcalls.c;utils.c" "drmgr;drsyms;drx") add_sample_client(hot_bbcount "hot_bbcount.c" "drmgr;drreg;drbbdup;drx") endif (X86) if (X86 OR AARCH64) + # dr_insert_mbr_instrumentation is NYI + add_sample_client(instrcalls "instrcalls.c;utils.c" "drmgr;drsyms;drx") # dr_insert_cbr_instrument_ex is NYI add_sample_client(cbrtrace "cbrtrace.c;utils.c" "drmgr;drx") endif (X86 OR AARCH64) diff --git a/core/lib/instrument.c b/core/lib/instrument.c index c83b94e294b..e5defb2094c 100644 --- a/core/lib/instrument.c +++ b/core/lib/instrument.c @@ -6090,6 +6090,49 @@ dr_insert_mbr_instrumentation(void *drcontext, instrlist_t *ilist, instr_t *inst #elif defined(RISCV64) /* FIXME i#3544: Not implemented */ ASSERT_NOT_IMPLEMENTED(false); +#elif defined(AARCH64) + dcontext_t *dcontext = (dcontext_t *)drcontext; + ptr_uint_t address; + reg_id_t target = DR_REG_NULL; + reg_id_t tmp = DR_REG_NULL; + CLIENT_ASSERT(drcontext != NULL, + "dr_insert_mbr_instrumentation: drcontext cannot be NULL"); + address = (ptr_uint_t)instr_get_translation(instr); + CLIENT_ASSERT(address != 0, + "dr_insert_mbr_instrumentation: can't determine app address"); + CLIENT_ASSERT(instr_is_mbr(instr), + "dr_insert_mbr_instrumentation must be applied to a mbr"); + + /* For all known mbr instructions on aarch64, the first source register saves the + * target. */ + target = opnd_get_reg(instr_get_src(instr, 0)); + /* Use tmp register to save target address. */ + tmp = (reg_to_pointer_sized(target) == DR_REG_X0) ? DR_REG_X1 : DR_REG_X0; + /* Save old value of tmp register to scratch_slot. */ + dr_save_reg(dcontext, ilist, instr, tmp, scratch_slot); + /* Copy target to tmp. */ + instr_t *mov = + XINST_CREATE_move(dcontext, opnd_create_reg(tmp), opnd_create_reg(target)); + /* PR 214962: ensure we'll properly translate the de-ref of app + * memory by marking the spill and de-ref as INSTR_OUR_MANGLING. + */ + instr_set_our_mangling(mov, true); + MINSERT(ilist, instr, mov); + + dr_insert_clean_call_ex( + drcontext, ilist, instr, callee, + /* Many users will ask for mcontexts; some will set; it doesn't seem worth + * asking the user to pass in a flag: if they're using this they are not + * super concerned about overhead. + */ + DR_CLEANCALL_READS_APP_CONTEXT | DR_CLEANCALL_WRITES_APP_CONTEXT, 2, + /* Address of mbr is 1st param. */ + OPND_CREATE_INTPTR(address), + /* Indirect target is 2nd param. */ + opnd_create_reg(tmp)); + + /* Restore old value of tmp register. */ + dr_restore_reg(dcontext, ilist, instr, tmp, scratch_slot); #endif /* X86/ARM/RISCV64 */ } From f02728e2cc48a04663d339fb180c41d8282ff258 Mon Sep 17 00:00:00 2001 From: Jiajie Chen Date: Fri, 27 Sep 2024 14:54:57 +0800 Subject: [PATCH 7/7] i#2297: Add XXX comments to note different impls --- core/lib/instrument.c | 1 + 1 file changed, 1 insertion(+) diff --git a/core/lib/instrument.c b/core/lib/instrument.c index e5defb2094c..ef5d711b9f5 100644 --- a/core/lib/instrument.c +++ b/core/lib/instrument.c @@ -6389,6 +6389,7 @@ dr_insert_cbr_instrumentation_help(void *drcontext, instrlist_t *ilist, instr_t /* Compute branch direction. */ opc = instr_get_opcode(instr); if (opc == OP_cbnz || opc == OP_cbz) { + /* XXX: which is faster, additional conditional branch or cmp + csinc? */ opnd_t reg_op = instr_get_src(instr, 1); reg_id_t reg = opnd_get_reg(reg_op); /* Use dir register to compute direction. */