From 0efa38122d389ef95f273cd96fa29ee59263da30 Mon Sep 17 00:00:00 2001 From: Edmund Grimley Evans Date: Thu, 2 Mar 2017 11:16:48 +0000 Subject: [PATCH 1/3] i#2210 clean call: Implement parameter preparation on ARM and AArch64. The implementation should efficiently handle any (reasonable) number of arguments that are immediate integers or full-size registers. Test cases are added in cleancallparams.dll.c. Fixes #2210 --- core/arch/aarchxx/mangle.c | 228 +++++++++++++++--- core/arch/mangle_shared.c | 11 +- .../client-interface/cleancallparams.dll.c | 123 +++++++++- 3 files changed, 318 insertions(+), 44 deletions(-) diff --git a/core/arch/aarchxx/mangle.c b/core/arch/aarchxx/mangle.c index b4f46e24a60..552e99bcb01 100644 --- a/core/arch/aarchxx/mangle.c +++ b/core/arch/aarchxx/mangle.c @@ -577,50 +577,212 @@ shrink_reg_for_param(reg_id_t regular, opnd_t arg) } #endif /* !AARCH64 */ +/* Return true if opnd is a register, but not XSP, or immediate zero on AArch64. */ +static bool +opnd_is_reglike(opnd_t opnd) +{ + return ((opnd_is_reg(opnd) && opnd_get_reg(opnd) != DR_REG_XSP) || + IF_X64_ELSE((opnd_is_immed_int(opnd) && opnd_get_immed_int(opnd) == 0), + false)); +} + uint insert_parameter_preparation(dcontext_t *dcontext, instrlist_t *ilist, instr_t *instr, bool clean_call, uint num_args, opnd_t *args) { - uint i; - instr_t *mark = INSTR_CREATE_label(dcontext); - PRE(ilist, instr, mark); + uint num_regs = num_args < NUM_REGPARM ? num_args : NUM_REGPARM; + signed char regs[NUM_REGPARM]; + int usecount[NUM_REGPARM]; + ptr_int_t stack_inc = 0; + uint i, j; - ASSERT(num_args == 0 || args != NULL); - /* FIXME i#1551, i#1569: we only support limited number of args for now. */ - ASSERT_NOT_IMPLEMENTED(num_args <= NUM_REGPARM); + /* We expect every arg to be an integer or a full-size register. */ for (i = 0; i < num_args; i++) { - if (opnd_is_immed_int(args[i])) { - insert_mov_immed_ptrsz(dcontext, opnd_get_immed_int(args[i]), - opnd_create_reg(regparms[i]), - ilist, instr_get_next(mark), NULL, NULL); - } else if (opnd_is_reg(args[i])) { - opnd_t arg = opnd_create_reg(reg_to_pointer_sized(opnd_get_reg(args[i]))); - if (opnd_get_reg(arg) == DR_REG_XSP) { - instr_t *loc = instr_get_next(mark); - PRE(ilist, loc, instr_create_save_to_tls - (dcontext, regparms[i], TLS_REG0_SLOT)); - insert_get_mcontext_base(dcontext, ilist, loc, regparms[i]); - PRE(ilist, loc, instr_create_restore_from_dc_via_reg + ASSERT(opnd_is_immed_int((args[i])) || + (opnd_is_reg(args[i]) && reg_get_size(opnd_get_reg(args[i])) == OPSZ_PTR)); + } + + /* Set up stack arguments that are registers (not SP) or zero (on AArch64). */ + if (num_args > NUM_REGPARM) { + uint n = num_args - NUM_REGPARM; + stack_inc = ALIGN_FORWARD(n, 2) * XSP_SZ; +#ifdef AARCH64 + for (i = 0; i < n; i += 2) { + opnd_t *arg0 = &args[NUM_REGPARM + i]; + opnd_t *arg1 = &args[NUM_REGPARM + i + 1]; + if (i == 0) { + if (i + 1 < n && opnd_is_reglike(*arg1)) { + /* stp x(...), x(...), [sp, #-(stack_inc)]! */ + PRE(ilist, instr, instr_create_2dst_4src + (dcontext, OP_stp, + opnd_create_base_disp(DR_REG_XSP, DR_REG_NULL, 0, + -stack_inc, OPSZ_16), + opnd_create_reg(DR_REG_XSP), + opnd_is_reg(*arg0) ? *arg0 : opnd_create_reg(DR_REG_XZR), + opnd_is_reg(*arg1) ? *arg1 : opnd_create_reg(DR_REG_XZR), + opnd_create_reg(DR_REG_XSP), + opnd_create_immed_int(-stack_inc, OPSZ_PTR))); + } else if (opnd_is_reglike(*arg0)) { + /* str x(...), [sp, #-(stack_inc)]! */ + PRE(ilist, instr, instr_create_2dst_3src + (dcontext, OP_str, + opnd_create_base_disp(DR_REG_XSP, DR_REG_NULL, 0, + -stack_inc, OPSZ_PTR), + opnd_create_reg(DR_REG_XSP), + opnd_is_reg(*arg0) ? *arg0 : opnd_create_reg(DR_REG_XZR), + opnd_create_reg(DR_REG_XSP), + opnd_create_immed_int(-stack_inc, OPSZ_PTR))); + } else { + /* sub sp, sp, #(stack_inc) */ + PRE(ilist, instr, INSTR_CREATE_sub + (dcontext, opnd_create_reg(DR_REG_XSP), + opnd_create_reg(DR_REG_XSP), OPND_CREATE_INT32(stack_inc))); + } + } else if (opnd_is_reglike(*arg0)) { + if (i + 1 < n && opnd_is_reglike(*arg1)) { + /* stp x(...), x(...), [sp, #(i * XSP_SZ)] */ + PRE(ilist, instr, instr_create_1dst_2src + (dcontext, OP_stp, + opnd_create_base_disp(DR_REG_XSP, DR_REG_NULL, 0, + i * XSP_SZ, OPSZ_16), + opnd_is_reg(*arg0) ? *arg0 : opnd_create_reg(DR_REG_XZR), + opnd_is_reg(*arg1) ? *arg1 : opnd_create_reg(DR_REG_XZR))); + } else { + /* str x(...), [sp, #(i * XSP_SZ)] */ + PRE(ilist, instr, instr_create_1dst_1src + (dcontext, OP_str, + opnd_create_base_disp(DR_REG_XSP, DR_REG_NULL, 0, + i * XSP_SZ, OPSZ_PTR), + opnd_is_reg(*arg0) ? *arg0 : opnd_create_reg(DR_REG_XZR))); + } + } else if (i + 1 < n && opnd_is_reglike(*arg1)) { + /* str x(...), [sp, #((i + 1) * XSP_SZ)] */ + PRE(ilist, instr, instr_create_1dst_1src + (dcontext, OP_str, + opnd_create_base_disp(DR_REG_XSP, DR_REG_NULL, 0, + (i + 1) * XSP_SZ, OPSZ_PTR), + opnd_is_reg(*arg1) ? *arg1 : opnd_create_reg(DR_REG_XZR))); + } + } +#else /* ARM */ + /* XXX: We could use OP_stm here, but with lots of awkward corner cases. */ + PRE(ilist, instr, INSTR_CREATE_sub(dcontext, opnd_create_reg(DR_REG_XSP), + opnd_create_reg(DR_REG_XSP), + OPND_CREATE_INT32(stack_inc))); + for (i = 0; i < n; i++) { + opnd_t arg = args[NUM_REGPARM + i]; + if (opnd_is_reglike(arg)) { + /* str r(...), [sp, #(i * 4)] */ + PRE(ilist, instr, XINST_CREATE_store + (dcontext, opnd_create_base_disp(DR_REG_XSP, DR_REG_NULL, 0, + i * 4, OPSZ_PTR), arg)); + } + } +#endif + } + + /* Initialise regs[], which encodes the contents of parameter registers. */ + for (i = 0; i < num_regs; i++) { + if (opnd_is_immed_int(args[i])) + regs[i] = -1; + else { + reg_id_t reg = opnd_get_reg(args[i]); + regs[i] = -2; + for (j = 0; j < NUM_REGPARM; j++) { + if (reg == regparms[j]) { + regs[i] = j; + break; + } + } + } + } + + /* Initialise usecount[]: how many other registers use the value in a reg. */ + for (i = 0; i < num_regs; i++) + usecount[i] = 0; + for (i = 0; i < num_regs; i++) { + if (regs[i] >= 0 && regs[i] != i) + ++usecount[regs[i]]; + } + + /* Set up register arguments that are not part of a cycle. */ + do { + for (i = 0; i < num_regs; i++) { + if (regs[i] == i || usecount[i] != 0) + continue; + if (regs[i] == -1) { + insert_mov_immed_ptrsz(dcontext, opnd_get_immed_int(args[i]), + opnd_create_reg(regparms[i]), + ilist, instr, NULL, NULL); + } else if (regs[i] == -2 && opnd_get_reg(args[i]) == DR_REG_XSP) { + /* XXX: We could record which register has been set to the SP to + * avoid repeating this load if several arguments are set to SP. + */ + insert_get_mcontext_base(dcontext, ilist, instr, regparms[i]); + PRE(ilist, instr, instr_create_restore_from_dc_via_reg (dcontext, regparms[i], regparms[i], XSP_OFFSET)); - } else if (opnd_get_reg(arg) != regparms[i]) { - POST(ilist, mark, XINST_CREATE_move(dcontext, + } else { + PRE(ilist, instr, XINST_CREATE_move(dcontext, opnd_create_reg(regparms[i]), - arg)); + args[i])); + if (regs[i] != -2) + --usecount[regs[i]]; } - } else { - /* FIXME i#1551, i#1569: we only implement naive parameter preparation, - * where args are all regs or immeds and do not conflict with param regs. - */ - ASSERT_NOT_IMPLEMENTED(false); + regs[i] = i; + break; + } + } while (i < num_regs); + + /* From now on it is safe to use LR as a temporary. */ + + /* Set up register arguments that are in cycles. */ + for (;;) { + int first, tmp; + for (i = 0; i < num_regs; i++) { + if (regs[i] != i) + break; + } + if (i >= num_regs) + break; + first = i; + PRE(ilist, instr, XINST_CREATE_move(dcontext, opnd_create_reg(DR_REG_LR), + opnd_create_reg(regparms[i]))); + do { + tmp = regs[i]; + ASSERT(0 <= tmp && tmp < num_regs); + PRE(ilist, instr, + XINST_CREATE_move(dcontext, opnd_create_reg(regparms[i]), + tmp == first ? opnd_create_reg(DR_REG_LR) : + opnd_create_reg(regparms[tmp]))); + regs[i] = i; + i = tmp; + } while (tmp != first); + } + + /* Set up stack arguments that are (non-zero) constants or SP. */ + for (i = NUM_REGPARM; i < num_args; i++) { + uint off = (i - NUM_REGPARM) * XSP_SZ; + opnd_t arg = args[i]; + if (!opnd_is_reglike(arg)) { + if (opnd_is_reg(arg)) { + ASSERT(opnd_get_reg(arg) == DR_REG_XSP); + insert_get_mcontext_base(dcontext, ilist, instr, DR_REG_LR); + PRE(ilist, instr, instr_create_restore_from_dc_via_reg + (dcontext, DR_REG_LR, DR_REG_LR, XSP_OFFSET)); + } else { + ASSERT(opnd_is_immed_int(arg)); + insert_mov_immed_ptrsz(dcontext, opnd_get_immed_int(arg), + opnd_create_reg(DR_REG_LR), + ilist, instr, NULL, NULL); + } + PRE(ilist, instr, XINST_CREATE_store + (dcontext, + opnd_create_base_disp(DR_REG_XSP, DR_REG_NULL, 0, off, OPSZ_PTR), + opnd_create_reg(DR_REG_LR))); } - DODEBUG({ - uint j; - /* check no reg used by arg conflicts with regparms */ - for (j = 0; j < i; j++) - ASSERT_NOT_IMPLEMENTED(!opnd_uses_reg(args[j], regparms[i])); - }); } - return 0; + + return (uint)stack_inc; } bool diff --git a/core/arch/mangle_shared.c b/core/arch/mangle_shared.c index 4d13b7c1dbe..21e9d307bef 100644 --- a/core/arch/mangle_shared.c +++ b/core/arch/mangle_shared.c @@ -464,14 +464,17 @@ insert_meta_call_vargs(dcontext_t *dcontext, instrlist_t *ilist, instr_t *instr, /* XXX PR 245936: let user decide whether to clean up? * i.e., support calling a stdcall routine? */ -#ifdef X86 +#if defined(X86) PRE(ilist, instr, INSTR_CREATE_lea(dcontext, opnd_create_reg(REG_XSP), opnd_create_base_disp(REG_XSP, REG_NULL, 0, stack_for_params, OPSZ_lea))); -#elif defined(ARM) - /* FIXME i#1551: NYI on ARM */ - ASSERT_NOT_IMPLEMENTED(false); +#elif defined(AARCHXX) + PRE(ilist, instr, INSTR_CREATE_add(dcontext, opnd_create_reg(REG_XSP), + opnd_create_reg(REG_XSP), + OPND_CREATE_INT32(stack_for_params))); +#else +# error NYI #endif } diff --git a/suite/tests/client-interface/cleancallparams.dll.c b/suite/tests/client-interface/cleancallparams.dll.c index fa2cc09fd0f..a7b337c4105 100644 --- a/suite/tests/client-interface/cleancallparams.dll.c +++ b/suite/tests/client-interface/cleancallparams.dll.c @@ -48,7 +48,7 @@ #include "dr_api.h" #include "client_tools.h" -#define MAX_NUM_ARGS 2 +#define MAX_NUM_ARGS 12 /* Some registers that we will use for testing. The registers used for * parameter passing should come first, in the right order, as some @@ -101,10 +101,36 @@ static const struct { { 1, { 2 } }, { 2, { -1, -2 } }, { 2, { 1, 2 } }, -#ifdef X86 - /* FIXME i#2210: Make this case work on ARM/AArch64, and add more cases. */ - { 2, { 2, 1 } }, /* Swap first two parameter registers. */ -#endif + { 2, { 2, 1 } }, /* Swap two registers. */ + { 3, { 2, 3, 1 } }, /* Rotate three registers. */ + { 4, { 2, 3, 4, 1 } }, /* Rotate four registers. */ + { 8, { 2, 1, 4, 3, 6, 5, 8, 7 } }, /* Rotate eight registers. */ + { 4, { 1, 1, 2, 3 } }, + { 4, { 2, 3, 4, 5 } }, + { 4, { -1, 1, 2, 3 } }, + { 11, { -1, -2, -1, -1, -2, -1, -1, -2, -1, -1, -2 } }, + { 12, { 2, 8, 8, 3, 1, 5, 3, 5, 5, 3, 7, 2 } }, + { 12, { 6, 9, 3, 1, 3, 1, 1, 4, 3, 1, 6, 6 } }, + { 12, { 9, 9, 10, 1, 9, 8, 6, 6, 5, 6, 9, 2 } }, + { 12, { 2, 5, 6, 7, 6, 6, 6, 3, 9, 5, 7, 8 } }, + { 12, { 9, 3, 2, 6, 9, 2, 9, 8, 10, 4, 2, 6 } }, + /* AArch64 stack args. */ + { 9, { 1, 2, 3, 4, 5, 6, 7, 8, -1 } }, + { 9, { 1, 2, 3, 4, 5, 6, 7, 8, 0 } }, + { 9, { 1, 2, 3, 4, 5, 6, 7, 8, 2 } }, + { 10, { 1, 2, 3, 4, 5, 6, 7, 8, -1, -2 } }, + { 10, { 1, 2, 3, 4, 5, 6, 7, 8, -1, 0 } }, + { 10, { 1, 2, 3, 4, 5, 6, 7, 8, -1, 2 } }, + { 10, { 1, 2, 3, 4, 5, 6, 7, 8, 0, -1 } }, + { 10, { 1, 2, 3, 4, 5, 6, 7, 8, 0, 0 } }, + { 10, { 1, 2, 3, 4, 5, 6, 7, 8, 0, 2 } }, + { 10, { 1, 2, 3, 4, 5, 6, 7, 8, 2, -1 } }, + { 10, { 1, 2, 3, 4, 5, 6, 7, 8, 2, 0 } }, + { 10, { 1, 2, 3, 4, 5, 6, 7, 8, 2, 4 } }, + { 11, { 1, 2, 3, 4, 5, 6, 7, 8, 2, 4, -1 } }, + { 11, { 1, 2, 3, 4, 5, 6, 7, 8, 2, 4, 6 } }, + { 12, { 1, 2, 3, 4, 5, 6, 7, 8, 2, 4, -1, -2 } }, + { 12, { 1, 2, 3, 4, 5, 6, 7, 8, 2, 4, 6, 8 } }, }; #define NUM_TESTS (sizeof(tests) / sizeof(*tests)) @@ -214,6 +240,78 @@ callee_2(ptr_uint_t a0, ptr_uint_t a1) callee(2, a0, a1); } +static void +callee_3(ptr_uint_t a0, ptr_uint_t a1, ptr_uint_t a2) +{ + callee(3, a0, a1, a2); +} + +static void +callee_4(ptr_uint_t a0, ptr_uint_t a1, ptr_uint_t a2, ptr_uint_t a3) +{ + callee(4, a0, a1, a2, a3); +} + +static void +callee_5(ptr_uint_t a0, ptr_uint_t a1, ptr_uint_t a2, ptr_uint_t a3, + ptr_uint_t a4) +{ + callee(5, a0, a1, a2, a3, a4); +} + +static void +callee_6(ptr_uint_t a0, ptr_uint_t a1, ptr_uint_t a2, ptr_uint_t a3, + ptr_uint_t a4, ptr_uint_t a5) +{ + callee(6, a0, a1, a2, a3, a4, a5); +} + +static void +callee_7(ptr_uint_t a0, ptr_uint_t a1, ptr_uint_t a2, ptr_uint_t a3, + ptr_uint_t a4, ptr_uint_t a5, ptr_uint_t a6) +{ + callee(7, a0, a1, a2, a3, a4, a5, a6); +} + +static void +callee_8(ptr_uint_t a0, ptr_uint_t a1, ptr_uint_t a2, ptr_uint_t a3, + ptr_uint_t a4, ptr_uint_t a5, ptr_uint_t a6, ptr_uint_t a7) +{ + callee(8, a0, a1, a2, a3, a4, a5, a6, a7); +} + +static void +callee_9(ptr_uint_t a0, ptr_uint_t a1, ptr_uint_t a2, ptr_uint_t a3, + ptr_uint_t a4, ptr_uint_t a5, ptr_uint_t a6, ptr_uint_t a7, + ptr_uint_t a8) +{ + callee(9, a0, a1, a2, a3, a4, a5, a6, a7, a8); +} + +static void +callee_10(ptr_uint_t a0, ptr_uint_t a1, ptr_uint_t a2, ptr_uint_t a3, + ptr_uint_t a4, ptr_uint_t a5, ptr_uint_t a6, ptr_uint_t a7, + ptr_uint_t a8, ptr_uint_t a9) +{ + callee(10, a0, a1, a2, a3, a4, a5, a6, a7, a8, a9); +} + +static void +callee_11(ptr_uint_t a0, ptr_uint_t a1, ptr_uint_t a2, ptr_uint_t a3, + ptr_uint_t a4, ptr_uint_t a5, ptr_uint_t a6, ptr_uint_t a7, + ptr_uint_t a8, ptr_uint_t a9, ptr_uint_t a10) +{ + callee(11, a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10); +} + +static void +callee_12(ptr_uint_t a0, ptr_uint_t a1, ptr_uint_t a2, ptr_uint_t a3, + ptr_uint_t a4, ptr_uint_t a5, ptr_uint_t a6, ptr_uint_t a7, + ptr_uint_t a8, ptr_uint_t a9, ptr_uint_t a10, ptr_uint_t a11) +{ + callee(12, a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11); +} + static void * callee_n(int n) { @@ -222,6 +320,16 @@ callee_n(int n) case 0: return (void *)callee_0; case 1: return (void *)callee_1; case 2: return (void *)callee_2; + case 3: return (void *)callee_3; + case 4: return (void *)callee_4; + case 5: return (void *)callee_5; + case 6: return (void *)callee_6; + case 7: return (void *)callee_7; + case 8: return (void *)callee_8; + case 9: return (void *)callee_9; + case 10: return (void *)callee_10; + case 11: return (void *)callee_11; + case 12: return (void *)callee_12; } return NULL; } @@ -252,9 +360,10 @@ event_basic_block(void *drcontext, void *tag, instrlist_t *bb, for (j = 0; j < num_args; j++) args[j] = convert_arg_to_opnd(tests[i].args[j]); /* Update the following two statements if the value of MAX_NUM_ARGS is changed. */ - ASSERT(MAX_NUM_ARGS == 2); + ASSERT(MAX_NUM_ARGS == 12); dr_insert_clean_call_ex(drcontext, bb, where, callee_n(num_args), false, num_args, - args[0], args[1]); + args[0], args[1], args[2], args[3], args[4], args[5], + args[6], args[7], args[8], args[9], args[10], args[11]); } /* Exit now. We do not run the app. */ From 210d5ffd23aacd3024ac3c362b2b9c419f1ece2a Mon Sep 17 00:00:00 2001 From: Edmund Grimley Evans Date: Fri, 3 Mar 2017 10:12:04 +0000 Subject: [PATCH 2/3] i#2210 clean call: Implement parameter preparation on ARM and AArch64. The implementation should efficiently handle any (reasonable) number of arguments that are immediate integers or full-size registers. Test cases are added in cleancallparams.dll.c. Memory arguments are not yet implemented. --- core/arch/aarchxx/mangle.c | 97 +++++++++++++++++++++++--------------- core/arch/mangle_shared.c | 12 +---- 2 files changed, 61 insertions(+), 48 deletions(-) diff --git a/core/arch/aarchxx/mangle.c b/core/arch/aarchxx/mangle.c index 552e99bcb01..24edcb85e48 100644 --- a/core/arch/aarchxx/mangle.c +++ b/core/arch/aarchxx/mangle.c @@ -581,9 +581,8 @@ shrink_reg_for_param(reg_id_t regular, opnd_t arg) static bool opnd_is_reglike(opnd_t opnd) { - return ((opnd_is_reg(opnd) && opnd_get_reg(opnd) != DR_REG_XSP) || - IF_X64_ELSE((opnd_is_immed_int(opnd) && opnd_get_immed_int(opnd) == 0), - false)); + return ((opnd_is_reg(opnd) && opnd_get_reg(opnd) != DR_REG_XSP) + IF_X64(|| (opnd_is_immed_int(opnd) && opnd_get_immed_int(opnd) == 0))); } uint @@ -596,20 +595,34 @@ insert_parameter_preparation(dcontext_t *dcontext, instrlist_t *ilist, instr_t * ptr_int_t stack_inc = 0; uint i, j; - /* We expect every arg to be an integer or a full-size register. */ + /* We expect every arg to be an immediate integer, a full-size register, + * or a simple memory reference (NYI). + */ for (i = 0; i < num_args; i++) { - ASSERT(opnd_is_immed_int((args[i])) || - (opnd_is_reg(args[i]) && reg_get_size(opnd_get_reg(args[i])) == OPSZ_PTR)); - } + CLIENT_ASSERT(opnd_is_immed_int((args[i])) || + (opnd_is_reg(args[i]) && + reg_get_size(opnd_get_reg(args[i])) == OPSZ_PTR) || + opnd_is_base_disp(args[i]), + "insert_parameter_preparation: bad argument type"); + ASSERT_NOT_IMPLEMENTED(!opnd_is_base_disp(args[i])); + } + + /* The strategy here is to first set up the arguments that can be set up + * without using a temporary register: stack arguments that are registers and + * register arguments that are not involved in a cycle. When this has been done, + * the value in the link register (LR) will be dead, so we can use LR as a + * temporary for setting up the remaining arguments. + */ /* Set up stack arguments that are registers (not SP) or zero (on AArch64). */ if (num_args > NUM_REGPARM) { uint n = num_args - NUM_REGPARM; + /* On both ARM and AArch64 the stack pointer is kept (2 * XSP_SZ)-aligned. */ stack_inc = ALIGN_FORWARD(n, 2) * XSP_SZ; #ifdef AARCH64 for (i = 0; i < n; i += 2) { opnd_t *arg0 = &args[NUM_REGPARM + i]; - opnd_t *arg1 = &args[NUM_REGPARM + i + 1]; + opnd_t *arg1 = i + 1 < n ? &args[NUM_REGPARM + i + 1] : NULL; if (i == 0) { if (i + 1 < n && opnd_is_reglike(*arg1)) { /* stp x(...), x(...), [sp, #-(stack_inc)]! */ @@ -672,16 +685,20 @@ insert_parameter_preparation(dcontext_t *dcontext, instrlist_t *ilist, instr_t * for (i = 0; i < n; i++) { opnd_t arg = args[NUM_REGPARM + i]; if (opnd_is_reglike(arg)) { - /* str r(...), [sp, #(i * 4)] */ + /* str r(...), [sp, #(i * XSP_SZ)] */ PRE(ilist, instr, XINST_CREATE_store (dcontext, opnd_create_base_disp(DR_REG_XSP, DR_REG_NULL, 0, - i * 4, OPSZ_PTR), arg)); + i * XSP_SZ, OPSZ_PTR), arg)); } } #endif } - /* Initialise regs[], which encodes the contents of parameter registers. */ + /* Initialise regs[], which encodes the contents of parameter registers. + * A non-negative value x means regparms[x]; + * -1 means an immediate integer; + * -2 means a non-parameter register. + */ for (i = 0; i < num_regs; i++) { if (opnd_is_immed_int(args[i])) regs[i] = -1; @@ -706,36 +723,42 @@ insert_parameter_preparation(dcontext_t *dcontext, instrlist_t *ilist, instr_t * } /* Set up register arguments that are not part of a cycle. */ - do { - for (i = 0; i < num_regs; i++) { - if (regs[i] == i || usecount[i] != 0) - continue; - if (regs[i] == -1) { - insert_mov_immed_ptrsz(dcontext, opnd_get_immed_int(args[i]), - opnd_create_reg(regparms[i]), - ilist, instr, NULL, NULL); - } else if (regs[i] == -2 && opnd_get_reg(args[i]) == DR_REG_XSP) { - /* XXX: We could record which register has been set to the SP to - * avoid repeating this load if several arguments are set to SP. - */ - insert_get_mcontext_base(dcontext, ilist, instr, regparms[i]); - PRE(ilist, instr, instr_create_restore_from_dc_via_reg - (dcontext, regparms[i], regparms[i], XSP_OFFSET)); - } else { - PRE(ilist, instr, XINST_CREATE_move(dcontext, - opnd_create_reg(regparms[i]), - args[i])); - if (regs[i] != -2) - --usecount[regs[i]]; + { + bool changed; + do { + changed = false; + for (i = 0; i < num_regs; i++) { + if (regs[i] == i || usecount[i] != 0) + continue; + if (regs[i] == -1) { + insert_mov_immed_ptrsz(dcontext, opnd_get_immed_int(args[i]), + opnd_create_reg(regparms[i]), + ilist, instr, NULL, NULL); + } else if (regs[i] == -2 && opnd_get_reg(args[i]) == DR_REG_XSP) { + /* XXX: We could record which register has been set to the SP to + * avoid repeating this load if several arguments are set to SP. + */ + insert_get_mcontext_base(dcontext, ilist, instr, regparms[i]); + PRE(ilist, instr, instr_create_restore_from_dc_via_reg + (dcontext, regparms[i], regparms[i], XSP_OFFSET)); + } else { + PRE(ilist, instr, XINST_CREATE_move(dcontext, + opnd_create_reg(regparms[i]), + args[i])); + if (regs[i] != -2) + --usecount[regs[i]]; + } + regs[i] = i; + changed = true; } - regs[i] = i; - break; - } - } while (i < num_regs); + } while (changed); + } /* From now on it is safe to use LR as a temporary. */ - /* Set up register arguments that are in cycles. */ + /* Set up register arguments that are in cycles. A rotation of n values is + * realised with (n + 1) moves. + */ for (;;) { int first, tmp; for (i = 0; i < num_regs; i++) { diff --git a/core/arch/mangle_shared.c b/core/arch/mangle_shared.c index 21e9d307bef..781109fbc70 100644 --- a/core/arch/mangle_shared.c +++ b/core/arch/mangle_shared.c @@ -464,18 +464,8 @@ insert_meta_call_vargs(dcontext_t *dcontext, instrlist_t *ilist, instr_t *instr, /* XXX PR 245936: let user decide whether to clean up? * i.e., support calling a stdcall routine? */ -#if defined(X86) - PRE(ilist, instr, - INSTR_CREATE_lea(dcontext, opnd_create_reg(REG_XSP), - opnd_create_base_disp(REG_XSP, REG_NULL, 0, - stack_for_params, OPSZ_lea))); -#elif defined(AARCHXX) - PRE(ilist, instr, INSTR_CREATE_add(dcontext, opnd_create_reg(REG_XSP), - opnd_create_reg(REG_XSP), + PRE(ilist, instr, XINST_CREATE_add(dcontext, opnd_create_reg(REG_XSP), OPND_CREATE_INT32(stack_for_params))); -#else -# error NYI -#endif } #ifdef CLIENT_INTERFACE From 9f4193fa7fac0306c71ee75ddd9d02e7f5b2a734 Mon Sep 17 00:00:00 2001 From: Edmund Grimley Evans Date: Fri, 3 Mar 2017 15:13:55 +0000 Subject: [PATCH 3/3] Add FIXME. --- core/arch/aarchxx/mangle.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/arch/aarchxx/mangle.c b/core/arch/aarchxx/mangle.c index 24edcb85e48..79f6e9224fc 100644 --- a/core/arch/aarchxx/mangle.c +++ b/core/arch/aarchxx/mangle.c @@ -604,7 +604,7 @@ insert_parameter_preparation(dcontext_t *dcontext, instrlist_t *ilist, instr_t * reg_get_size(opnd_get_reg(args[i])) == OPSZ_PTR) || opnd_is_base_disp(args[i]), "insert_parameter_preparation: bad argument type"); - ASSERT_NOT_IMPLEMENTED(!opnd_is_base_disp(args[i])); + ASSERT_NOT_IMPLEMENTED(!opnd_is_base_disp(args[i])); /* FIXME i#2210 */ } /* The strategy here is to first set up the arguments that can be set up