Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i#2210 clean call: Implement parameter preparation on ARM and AArch64. #2260

Merged
merged 6 commits into from
Mar 5, 2017
Merged

i#2210 clean call: Implement parameter preparation on ARM and AArch64. #2260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0efa381
i#2210 clean call: Implement parameter preparation on ARM and AArch64.
egrimley-arm Mar 2, 2017
210d5ff
i#2210 clean call: Implement parameter preparation on ARM and AArch64.
egrimley-arm Mar 3, 2017
9f4193f
Add FIXME.
egrimley-arm Mar 3, 2017
e160b1f
Merge branch 'master' into i2210-aarchxx-clean-call
egrimley-arm Mar 3, 2017
4b4f331
Merge branch 'master' into i2210-aarchxx-clean-call
egrimley-arm Mar 4, 2017
c99c52f
Merge branch 'master' into i2210-aarchxx-clean-call
egrimley-arm Mar 5, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
257 changes: 221 additions & 36 deletions core/arch/aarchxx/mangle.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,50 +577,235 @@ 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(|| (opnd_is_immed_int(opnd) && opnd_get_immed_int(opnd) == 0)));
}

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);

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);
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;

/* 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++) {
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
(dcontext, regparms[i], regparms[i], XSP_OFFSET));
} else if (opnd_get_reg(arg) != regparms[i]) {
POST(ilist, mark, XINST_CREATE_move(dcontext,
opnd_create_reg(regparms[i]),
arg));
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])); /* FIXME i#2210 */
}

/* 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 = 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)]! */
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 {
/* 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);
}
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]));
});
#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 * XSP_SZ)] */
PRE(ilist, instr, XINST_CREATE_store
(dcontext, opnd_create_base_disp(DR_REG_XSP, DR_REG_NULL, 0,
i * XSP_SZ, OPSZ_PTR), arg));
}
}
#endif
}
return 0;

/* 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;
else {
reg_id_t reg = opnd_get_reg(args[i]);
regs[i] = -2;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above too: this -1, -2 too stuff needs comments explaining, or, better, to use an enum type or something

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. */
{
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;
}
} while (changed);
}

/* From now on it is safe to use LR as a temporary. */

/* 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++) {
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];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I think I understand what it's doing, shifting one into LR and everything else one down, then repeat, but comments on the approach would help, and on whether there might be a more efficient method but it would be much more complex and not worthwhile, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A rotation of n registers is implemented with (n + 1) moves in the obvious way. I don't think there's a better way.

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. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above: IMHO these should be up by the other stack args

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No guarantee of a free register earlier.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment should explain this: it was not obvious.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a comment near the start of the function: /* The strategy here is to first set up ... (Yes, I do split infinitives!)

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)));
}
}

return (uint)stack_inc;
}

bool
Expand Down
11 changes: 2 additions & 9 deletions core/arch/mangle_shared.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,15 +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?
*/
#ifdef 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);
#endif
PRE(ilist, instr, XINST_CREATE_add(dcontext, opnd_create_reg(REG_XSP),
OPND_CREATE_INT32(stack_for_params)));
}

#ifdef CLIENT_INTERFACE
Expand Down
Loading