Skip to content

Commit

Permalink
i#5498 AArch64: fix memory instrs using 32 bit index register (#5511)
Browse files Browse the repository at this point in the history
- Handle stolen register in drutil_insert_get_mem_addr_arm() if it's
  used as a 32 bit index register.
- Add client test case using W28 as index register in memory
  instructions.

Issue: #5498
  • Loading branch information
AssadHashmi authored Jun 7, 2022
1 parent bdb0ab3 commit 4091617
Show file tree
Hide file tree
Showing 5 changed files with 263 additions and 1 deletion.
20 changes: 20 additions & 0 deletions ext/drutil/drutil.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* **********************************************************
* Copyright (c) 2011-2021 Google, Inc. All rights reserved.
* Copyright (c) 2008-2010 VMware, Inc. All rights reserved.
* Copyright (c) 2022 Arm Limited All rights reserved.
* **********************************************************/

/* drutil: DynamoRIO Instrumentation Utilities
Expand Down Expand Up @@ -422,6 +423,18 @@ drutil_insert_get_mem_addr_arm(void *drcontext, instrlist_t *bb, instr_t *where,
disp = -disp;
negated = !negated;
}
# ifdef AARCH64
/* In cases where only the lower 32 bits of the index register are
* used, we need to widen to 64 bits in order to handle stolen
* register's replacement. See replace_stolen_reg() below, where index
* is narrowed after replacement.
*/
bool is_index_32bit_stolen = false;
if (index == reg_64_to_32(stolen)) {
index = stolen;
is_index_32bit_stolen = true;
}
# endif
if (dst == stolen || scratch == stolen)
return false;
if (base == stolen) {
Expand All @@ -430,6 +443,13 @@ drutil_insert_get_mem_addr_arm(void *drcontext, instrlist_t *bb, instr_t *where,
} else if (index == stolen) {
index = replace_stolen_reg(drcontext, bb, where, memref, dst, scratch,
scratch_used);
# ifdef AARCH64
/* Narrow replaced index register if it was 32 bit stolen register
* before replace_stolen_reg() call.
*/
if (is_index_32bit_stolen)
index = reg_64_to_32(stolen);
# endif
}
if (index == REG_NULL && opnd_get_disp(memref) != 0) {
/* first try "add dst, base, #disp" */
Expand Down
7 changes: 6 additions & 1 deletion suite/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# **********************************************************
# Copyright (c) 2010-2022 Google, Inc. All rights reserved.
# Copyright (c) 2009-2010 VMware, Inc. All rights reserved.
# Copyright (c) 2016 ARM Limited. All rights reserved.
# Copyright (c) 2016-2022 ARM Limited. All rights reserved.
# **********************************************************

# Redistribution and use in source and binary forms, with or without
Expand Down Expand Up @@ -2557,6 +2557,11 @@ set(client.option_parse_client_ops_islist ON)

if (AARCH64)
tobuild_ci(client.features client-interface/features.c "" "" "")

tobuild_ci(client.stolen-reg-index client-interface/stolen-reg-index.c "" "" "")
use_DynamoRIO_extension(client.stolen-reg-index.dll drmgr)
use_DynamoRIO_extension(client.stolen-reg-index.dll drreg)
use_DynamoRIO_extension(client.stolen-reg-index.dll drutil)
endif ()

tobuild_ci(client.reg_size_test client-interface/reg_size_test.c "" "" "")
Expand Down
88 changes: 88 additions & 0 deletions suite/tests/client-interface/stolen-reg-index.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/* **********************************************************
* Copyright (c) 2022 Arm Limited All rights reserved.
* **********************************************************/

/*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
*
* * Redistributions of source code must retain the above copyright notice,
* this list of conditions and the following disclaimer.
*
* * Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
*
* * Neither the name of Google, Inc. nor the names of its contributors may be
* used to endorse or promote products derived from this software without
* specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL GOOGLE, INC. OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
* SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
* CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
* DAMAGE.
*/

/* Executes two indexed memory instructions, one using X28 for index register,
* the other W28. Intention is for DR to call drutil_insert_get_mem_addr() in
* order to test the 'if (index == stolen)' clause in
* drutil_insert_get_mem_addr_arm() in the case of W28.
*/

#ifndef ASM_CODE_ONLY /* C code */

# include "tools.h"

/* In asm code. */
void
indexed_mem_test(int *val);

int
main(int argc, char *argv[])
{
int value = 41;
indexed_mem_test(&value);
if (value != 42)
print("indexed_mem_test() failed with %d, expected 42.\n", value);
else
print("indexed_mem_test() passed.\n");

print("Tested the use of stolen register as memory index register.\n");
return 0;
}

#else /* asm code *************************************************************/
# include "asm_defines.asm"
/* clang-format off */
START_FILE
#define FUNCNAME indexed_mem_test
DECLARE_EXPORTED_FUNC(FUNCNAME)
GLOBAL_LABEL(FUNCNAME:)

stp x0, x1, [sp, #-16]!

/* Load passed in value using index register X28, then increment. */
mov x28, #0
ldr x1, [x0, x28, lsl #0]
add x1, x1, #1

/* Store incremented value using index register W28. */
mov w28, #0
str x1, [x0, w28, uxtw #0]

ldp x0, x1, [sp], #16
ret

END_FUNC(FUNCNAME)
# undef FUNCNAME

END_FILE
/* clang-format on */
#endif /* ASM_CODE_ONLY */
145 changes: 145 additions & 0 deletions suite/tests/client-interface/stolen-reg-index.dll.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
/* **********************************************************
* Copyright (c) 2022 Arm Limited All rights reserved.
* **********************************************************/

/*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
*
* * Redistributions of source code must retain the above copyright notice,
* this list of conditions and the following disclaimer.
*
* * Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
*
* * Neither the name of VMware, Inc. nor the names of its contributors may be
* used to endorse or promote products derived from this software without
* specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL VMWARE, INC. OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
* SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
* CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
* DAMAGE.
*/

/* Tests a memory operand's index register is handled correctly if it happens
* to be the stolen register W28, rather than X28.
*/

#include "dr_api.h"
#include "drmgr.h"
#include "drreg.h"
#include "drutil.h"
#include "client_tools.h"
#include "drreg-test-shared.h"

static void
event_exit(void)
{
drreg_exit();
drmgr_exit();
drutil_exit();
}

/* Simply calls drutil_insert_get_mem_addr(). */
static bool
insert_get_addr(void *drcontext, instrlist_t *ilist, instr_t *instr, opnd_t mref)
{
reg_id_t reg_ptr, reg_tmp, reg_addr;

if (drreg_reserve_register(drcontext, ilist, instr, NULL, &reg_tmp) !=
DRREG_SUCCESS) {
DR_ASSERT(false);
}

if (drreg_reserve_register(drcontext, ilist, instr, NULL, &reg_ptr) !=
DRREG_SUCCESS) {
DR_ASSERT(false);
}

/* XXX i#5498: We should check the address in reg_ptr for correctness. A
* constant reference address in this client to check for correctness based
* on the test binary stolen-reg-index is not a viable approach because
* different versions of compiler, linker/loader and OS will produce
* different addresses at runtime.
*/
if (!drutil_insert_get_mem_addr(drcontext, ilist, instr, mref, reg_ptr, reg_tmp)) {
dr_printf("drutil_insert_get_mem_addr() failed!\n");
return false;
}

if (drreg_unreserve_register(drcontext, ilist, instr, reg_tmp) != DRREG_SUCCESS)
DR_ASSERT(false);

if (drreg_unreserve_register(drcontext, ilist, instr, reg_ptr) != DRREG_SUCCESS)
DR_ASSERT(false);

return true;
}

static dr_emit_flags_t
event_app_instruction(void *drcontext, void *tag, instrlist_t *bb, instr_t *inst,
bool for_trace, bool translating, void *user_data)
{
opnd_t opnd;
reg_id_t reg;
reg_id_t stolen = dr_get_stolen_reg();
DR_ASSERT(stolen == DR_REG_X28);

/* Selects store insruction, str x1, [x0, w28, uxtw #0] from test subject,
* stolen-reg-index.c.
*/
if (instr_writes_memory(inst)) {
opnd = instr_get_dst(inst, 0);
if (opnd_is_memory_reference(opnd) && opnd_is_base_disp(opnd)) {
if (opnd_get_index(opnd) == stolen && opnd_get_base(opnd) == DR_REG_X0)
dr_printf("store memref with index reg X28\n");
if (opnd_get_index(opnd) == DR_REG_W28 && opnd_get_base(opnd) == DR_REG_X0)
dr_printf("store memref with index reg W28\n");
if (insert_get_addr(drcontext, bb, inst, instr_get_dst(inst, 0)))
return DR_EMIT_DEFAULT;
else
DR_ASSERT(false);
}
}

/* Selects load instruction, ldr x1, [x0, x28, lsl #0] from test subject,
* stolen-reg-index.c.
*/
if (instr_reads_memory(inst)) {
opnd = instr_get_src(inst, 0);
if (opnd_is_memory_reference(opnd) && opnd_is_base_disp(opnd)) {
if (opnd_get_index(opnd) == stolen && opnd_get_base(opnd) == DR_REG_X0)
dr_printf("load memref with index reg X28\n");
if (opnd_get_index(opnd) == DR_REG_W28 && opnd_get_base(opnd) == DR_REG_X0)
dr_printf("load memref with index reg W28\n");
if (insert_get_addr(drcontext, bb, inst, instr_get_src(inst, 0)))
return DR_EMIT_DEFAULT;
else
DR_ASSERT(false);
}
}

return DR_EMIT_DEFAULT;
}

DR_EXPORT void
dr_client_main(client_id_t id, int argc, const char *argv[])
{
drreg_options_t ops = { sizeof(ops), 4 /*max slots needed*/, false };
if (!drmgr_init() || !drutil_init() || drreg_init(&ops) != DRREG_SUCCESS)
DR_ASSERT(false);
CHECK(dr_get_stolen_reg() == TEST_REG_STOLEN, "stolen reg doesn't match");

dr_register_exit_event(event_exit);
if (!drmgr_register_bb_instrumentation_event(NULL, event_app_instruction, NULL))
DR_ASSERT(false);
}
4 changes: 4 additions & 0 deletions suite/tests/client-interface/stolen-reg-index.expect
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
load memref with index reg X28
store memref with index reg W28
indexed_mem_test() passed.
Tested the use of stolen register as memory index register.

1 comment on commit 4091617

@derekbruening
Copy link
Contributor

Choose a reason for hiding this comment

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

This commit actually failed to fix the bug: #5498 (comment)

Please sign in to comment.