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#3544 RV64: Rebase the dcontext pointer. #7235

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
+95 −30
Open

i#3544 RV64: Rebase the dcontext pointer. #7235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
0f5fbaa
i#3544 Rebase the dcontext pointer for the RISC-V architecture
mariospaok4 Jan 30, 2025
44261bc
Fixed style
mariospaok4 Jan 30, 2025
990ddda
Formating fixes
mariospaok4 Jan 30, 2025
59d4761
More formating fixes
mariospaok4 Jan 30, 2025
d504800
Fixed a wrong register
mariospaok4 Jan 30, 2025
1e7f230
Merge branch 'master' into master
mariospaok4 Feb 3, 2025
f70ec3c
Fixxed incorrect stack increment
mariospaok4 Feb 4, 2025
2c232bc
Merge branch 'master' into master
mariospaok4 Feb 4, 2025
5df374a
Used the offset, somewhere that it was missed
mariospaok4 Feb 5, 2025
878887a
Fixed style
mariospaok4 Feb 6, 2025
f629122
Merge branch 'DynamoRIO:master' into master
mariospaok4 Feb 6, 2025
34f4520
Fixed another offset issue
mariospaok4 Feb 6, 2025
149d543
Fixed format and added comment about insert_get_mcontext_base()
mariospaok4 Feb 6, 2025
2766030
Added copyright at mangle_shared.c
mariospaok4 Feb 6, 2025
604b177
Merge branch 'DynamoRIO:master' into master
mariospaok4 Feb 7, 2025
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
12 changes: 12 additions & 0 deletions core/arch/arch_exports.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* **********************************************************
* Copyright (c) 2011-2022 Google, Inc. All rights reserved.
* Copyright (c) 2000-2010 VMware, Inc. All rights reserved.
* Copyright (c) 2025 Foundation of Research and Technology, Hellas.
* **********************************************************/

/*
Expand Down Expand Up @@ -1819,4 +1820,15 @@ typedef struct _rseq_entry_state_t {
} rseq_entry_state_t;
#endif

/*
* In riscv we cannot address the entire dcontext by using base + immediate.
* for the reason we add 0x800 to the saved pointer and access it by
* substracting 0x800 from the offset in the struct.
*/
#ifdef RISCV64
# define CONTEXT_REBASE_OFFT 0x800
Copy link
Member

Choose a reason for hiding this comment

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

Suggest a better name such as CONTEXT_HEAD_OFFSET.

I wonder whether a negative offset will be more descriptive than using subtraction everywhere, which gives a hint about the memory layout (REG_DCTX points to the middle of dcontext_t, yielding a negative offset).

Copy link
Contributor

Choose a reason for hiding this comment

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

(I prefer to keep the offset positive, as it is a positive offset.)

Maybe define some macros to avoid raw arithmetic operations all over the place?

#else
# define CONTEXT_REBASE_OFFT 0
#endif

#endif /* _ARCH_EXPORTS_H_ */
33 changes: 29 additions & 4 deletions core/arch/emit_utils_shared.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* **********************************************************
* Copyright (c) 2010-2022 Google, Inc. All rights reserved.
* Copyright (c) 2000-2010 VMware, Inc. All rights reserved.
* Copyright (c) 2025 Foundation of Research and Technology, Hellas.
* **********************************************************/

/*
Expand Down Expand Up @@ -2180,12 +2181,14 @@ emit_fcache_enter_common(dcontext_t *dcontext, generated_code_t *code, byte *pc,
#elif defined(RISCV64)
APP(&ilist,
INSTR_CREATE_ld(dcontext, opnd_create_reg(DR_REG_A0),
opnd_create_base_disp(REG_DCXT, DR_REG_NULL, 0,
REG_OFFSET(DR_REG_A0), OPSZ_8)));
opnd_create_base_disp(
REG_DCXT, DR_REG_NULL, 0,
(int)REG_OFFSET(DR_REG_A0) - CONTEXT_REBASE_OFFT, OPSZ_8)));
APP(&ilist,
INSTR_CREATE_ld(dcontext, opnd_create_reg(DR_REG_A1),
opnd_create_base_disp(REG_DCXT, DR_REG_NULL, 0,
REG_OFFSET(DR_REG_A1), OPSZ_8)));
opnd_create_base_disp(
REG_DCXT, DR_REG_NULL, 0,
(int)REG_OFFSET(DR_REG_A1) - CONTEXT_REBASE_OFFT, OPSZ_8)));

APP(&ilist,
INSTR_CREATE_sd(
Expand Down Expand Up @@ -2380,9 +2383,31 @@ append_call_dispatch(dcontext_t *dcontext, instrlist_t *ilist, bool absolute)
/* call central d_r_dispatch routine */
/* for x64 linux we could optimize and avoid the "mov rdi, rdi" */
/* for ARM we use _noreturn to avoid storing to %lr */

/*
* REG_DCTXT holds the rebased dcontext d_r_dispatch expect the normal one
* so we should restore it.
*
* Currently only affects RISCV64
*/
#if (CONTEXT_REBASE_OFFT != 0)
if (absolute) {
dr_insert_call_noreturn((void *)dcontext, ilist, NULL /*append*/,
(void *)d_r_dispatch, 1,
OPND_CREATE_INTPTR((ptr_int_t)dcontext));
} else {
APP(ilist,
XINST_CREATE_add_2src(dcontext, opnd_create_reg(DR_REG_A0),
opnd_create_reg(REG_DCTXT),
OPND_CREATE_INT32(-CONTEXT_REBASE_OFFT)));
dr_insert_call_noreturn((void *)dcontext, ilist, NULL /*append*/,
(void *)d_r_dispatch, 1, opnd_create_reg(DR_REG_A0));
}
#else
dr_insert_call_noreturn(
(void *)dcontext, ilist, NULL /*append*/, (void *)d_r_dispatch, 1,
absolute ? OPND_CREATE_INTPTR((ptr_int_t)dcontext) : opnd_create_reg(REG_DCTXT));
#endif

/* d_r_dispatch() shouldn't return! */
insert_reachable_cti(dcontext, ilist, NULL, vmcode_get_start(),
Expand Down
17 changes: 12 additions & 5 deletions core/arch/mangle_shared.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* Copyright (c) 2010-2022 Google, Inc. All rights reserved.
* Copyright (c) 2010 Massachusetts Institute of Technology All rights reserved.
* Copyright (c) 2000-2010 VMware, Inc. All rights reserved.
* Copyright (c) 2025 Foundation of Research and Technology, Hellas.
* ******************************************************************************/

/*
Expand Down Expand Up @@ -81,6 +82,9 @@ get_clean_call_temp_stack_size(void)
/* utility routines for inserting clean calls to an instrumentation routine
* strategy is very similar to fcache_enter/return
* FIXME: try to share code with fcache_enter/return?
* FIXME: Return the correct mcontext base when CONTEXT_REBASE_OFFT is used
* This will need calls like opnd_create_dcontext_field_via_reg_sz to be replaced
* with something else. Currently we work around that by other means.
*
* first swap stacks to DynamoRIO stack:
* SAVE_TO_UPCONTEXT %xsp,xsp_OFFSET
Expand Down Expand Up @@ -757,7 +761,8 @@ insert_meta_call_vargs(dcontext_t *dcontext, instrlist_t *ilist, instr_t *instr,
* We save it to dcontext.mcontext.x0.
*/
PRE(ilist, instr,
XINST_CREATE_store(dcontext, OPND_CREATE_MEMPTR(link_reg, 0),
XINST_CREATE_store(dcontext,
OPND_CREATE_MEMPTR(link_reg, -CONTEXT_REBASE_OFFT),
opnd_create_reg(SCRATCH_REG0)));
instrlist_insert_mov_immed_ptrsz(dcontext, (ptr_int_t)DR_WHERE_CLEAN_CALLEE,
opnd_create_reg(SCRATCH_REG0), ilist, instr,
Expand All @@ -770,7 +775,7 @@ insert_meta_call_vargs(dcontext_t *dcontext, instrlist_t *ilist, instr_t *instr,
/* Restore scratch_reg from dcontext.mcontext.x0. */
PRE(ilist, instr,
XINST_CREATE_load(dcontext, opnd_create_reg(SCRATCH_REG0),
OPND_CREATE_MEMPTR(link_reg, 0)));
OPND_CREATE_MEMPTR(link_reg, -CONTEXT_REBASE_OFFT)));
#else
/* SCRATCH_REG0 is dead here, because clean calls only support "cdecl",
* which specifies that the caller must save xax (and xcx and xdx).
Expand Down Expand Up @@ -823,7 +828,8 @@ insert_meta_call_vargs(dcontext_t *dcontext, instrlist_t *ilist, instr_t *instr,
* We save it to dcontext.mcontext.x0.
*/
PRE(ilist, instr,
XINST_CREATE_store(dcontext, OPND_CREATE_MEMPTR(SCRATCH_REG0, 0),
XINST_CREATE_store(dcontext,
OPND_CREATE_MEMPTR(SCRATCH_REG0, -CONTEXT_REBASE_OFFT),
opnd_create_reg(SCRATCH_REG1)));
instrlist_insert_mov_immed_ptrsz(dcontext, (ptr_int_t)whereami,
opnd_create_reg(SCRATCH_REG1), ilist, instr,
Expand All @@ -835,8 +841,9 @@ insert_meta_call_vargs(dcontext_t *dcontext, instrlist_t *ilist, instr_t *instr,
WHEREAMI_OFFSET));
/* Restore scratch_reg from dcontext.mcontext.x0. */
PRE(ilist, instr,
XINST_CREATE_load(dcontext, opnd_create_reg(SCRATCH_REG1),
OPND_CREATE_MEMPTR(SCRATCH_REG0, 0)));
XINST_CREATE_load(
dcontext, opnd_create_reg(SCRATCH_REG1),
OPND_CREATE_MEMPTR(SCRATCH_REG0, -CONTEXT_REBASE_OFFT)));
#else
PRE(ilist, instr,
instr_create_save_immed_to_dc_via_reg(dcontext, SCRATCH_REG0,
Expand Down
24 changes: 13 additions & 11 deletions core/arch/riscv64/emit_utils.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* **********************************************************
* Copyright (c) 2022 Rivos, Inc. All rights reserved.
* Copyright (c) 2024 Foundation of Research and Technology, Hellas.
* Copyright (c) 2024-2025 Foundation of Research and Technology, Hellas.
* **********************************************************/

/*
Expand Down Expand Up @@ -698,12 +698,13 @@ append_restore_simd_reg(dcontext_t *dcontext, instrlist_t *ilist, bool absolute)
*
* ma ta sew=8 lmul=8 */
vtypei = (0b1 << 7) | (0b1 << 6) | (0b000 << 3) | 0b011;
memopnd = opnd_create_dcontext_field_via_reg_sz(
dcontext, DR_REG_A1, 0, reg_get_size_lmul(DR_REG_VR0, RV64_LMUL_8));
memopnd = opnd_create_base_disp(DR_REG_A1, REG_NULL, 0, 0,
reg_get_size_lmul(DR_REG_VR0, RV64_LMUL_8));
APP(ilist,
INSTR_CREATE_addi(dcontext, opnd_create_reg(DR_REG_A1),
opnd_create_reg(REG_DCXT),
opnd_create_immed_int(VREG_OFFSET(DR_REG_VR0), OPSZ_12b)));
INSTR_CREATE_addi(
dcontext, opnd_create_reg(DR_REG_A1), opnd_create_reg(REG_DCXT),
opnd_create_immed_int(VREG_OFFSET(DR_REG_VR0) - CONTEXT_REBASE_OFFT,
OPSZ_12b)));
/* For the following vector instructions, set the element width to 8b, and use 8
* registers as a group (lmul=8).
*/
Expand Down Expand Up @@ -835,12 +836,13 @@ append_save_simd_reg(dcontext_t *dcontext, instrlist_t *ilist, bool absolute)
*
* ma ta sew=8 lmul=8 */
vtypei = (0b1 << 7) | (0b1 << 6) | (0b000 << 3) | 0b011;
memopnd = opnd_create_dcontext_field_via_reg_sz(
dcontext, DR_REG_A1, 0, reg_get_size_lmul(DR_REG_VR0, RV64_LMUL_8));
memopnd = opnd_create_base_disp(DR_REG_A1, REG_NULL, 0, 0,
reg_get_size_lmul(DR_REG_VR0, RV64_LMUL_8));
APP(ilist,
INSTR_CREATE_addi(dcontext, opnd_create_reg(DR_REG_A1),
opnd_create_reg(REG_DCXT),
opnd_create_immed_int(VREG_OFFSET(DR_REG_VR0), OPSZ_12b)));
INSTR_CREATE_addi(
dcontext, opnd_create_reg(DR_REG_A1), opnd_create_reg(REG_DCXT),
opnd_create_immed_int(VREG_OFFSET(DR_REG_VR0) - CONTEXT_REBASE_OFFT,
OPSZ_12b)));
/* For the following vector instructions, set the element width to 8b, and use 8
* registers as a group (lmul=8).
*/
Expand Down
12 changes: 6 additions & 6 deletions core/arch/riscv64/mangle.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* **********************************************************
* Copyright (c) 2022 Rivos, Inc. All rights reserved.
* Copyright (c) 2024 Foundation of Research and Technology, Hellas.
* Copyright (c) 2024-2025 Foundation of Research and Technology, Hellas.
* **********************************************************/

/*
Expand Down Expand Up @@ -219,7 +219,7 @@ insert_push_all_registers(dcontext_t *dcontext, clean_call_info_t *cci,
opnd_create_reg(DR_REG_A0)));
}

dstack_offs += 2 * XSP_SZ;
dstack_offs += XSP_SZ;

/* Push vector registers. */
if (proc_has_feature(FEATURE_VECTOR)) {
Expand All @@ -230,8 +230,8 @@ insert_push_all_registers(dcontext_t *dcontext, clean_call_info_t *cci,
*
* ma ta sew=8 lmul=8 */
vtypei = (0b1 << 7) | (0b1 << 6) | (0b000 << 3) | 0b011;
memopnd = opnd_create_dcontext_field_via_reg_sz(
dcontext, DR_REG_A0, 0, reg_get_size_lmul(DR_REG_VR0, RV64_LMUL_8));
memopnd = opnd_create_base_disp(DR_REG_A0, REG_NULL, 0, 0,
reg_get_size_lmul(DR_REG_VR0, RV64_LMUL_8));
PRE(ilist, instr,
INSTR_CREATE_addi(dcontext, opnd_create_reg(DR_REG_A0),
opnd_create_reg(DR_REG_SP),
Expand Down Expand Up @@ -301,8 +301,8 @@ insert_pop_all_registers(dcontext_t *dcontext, clean_call_info_t *cci, instrlist
*
* ma ta sew=8 lmul=8 */
vtypei = (0b1 << 7) | (0b1 << 6) | (0b000 << 3) | 0b011;
memopnd = opnd_create_dcontext_field_via_reg_sz(
dcontext, DR_REG_A0, 0, reg_get_size_lmul(DR_REG_VR0, RV64_LMUL_8));
memopnd = opnd_create_base_disp(DR_REG_A0, REG_NULL, 0, 0,
reg_get_size_lmul(DR_REG_VR0, RV64_LMUL_8));
PRE(ilist, instr,
INSTR_CREATE_addi(dcontext, opnd_create_reg(DR_REG_A0),
opnd_create_reg(DR_REG_SP),
Expand Down
11 changes: 11 additions & 0 deletions core/dispatch.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* **********************************************************
* Copyright (c) 2011-2023 Google, Inc. All rights reserved.
* Copyright (c) 2000-2010 VMware, Inc. All rights reserved.
* Copyright (c) 2025 Foundation of Research and Technology, Hellas.
* **********************************************************/

/*
Expand Down Expand Up @@ -569,7 +570,17 @@ enter_fcache(dcontext_t *dcontext, fcache_enter_func_t entry, cache_pc pc)
* paths were missed?
*/
PTHREAD_JIT_READ();

/*
* XXX arithmetic with void pointers does not work on windows.
* Since RISCV is the only architecture that has CONTEXT_REBASE_OFFT != 0
* and it does not run windows, we can work around that with conditional compilation.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Well, we could convert it to intptr_t to do the math and then convert it back.

#if (CONTEXT_REBASE_OFFT != 0)
(*entry)(((void *)dcontext) + CONTEXT_REBASE_OFFT);
Copy link
Member

Choose a reason for hiding this comment

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

Conversion between REG_DCTX and the real head of dcontext_t occurs several times. Suggest wrap these into macros. (CONTEXT_PTR_TO_HEAD() and CONTEXT_HEAD_TO_PTR).

#else
(*entry)(dcontext);
#endif
IF_WINDOWS(ASSERT_NOT_REACHED()); /* returns for signals on unix */
}

Expand Down
6 changes: 4 additions & 2 deletions core/ir/opnd_shared.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* **********************************************************
* Copyright (c) 2011-2022 Google, Inc. All rights reserved.
* Copyright (c) 2000-2010 VMware, Inc. All rights reserved.
* Copyright (c) 2025 Foundation of Research and Technology, Hellas.
* **********************************************************/

/*
Expand Down Expand Up @@ -2905,7 +2906,8 @@ dcontext_opnd_common(dcontext_t *dcontext, bool absolute, reg_id_t basereg, int
offs -= sizeof(unprotected_context_t);
return opnd_create_base_disp(
absolute ? REG_NULL : (basereg == REG_NULL ? REG_DCXT : basereg), REG_NULL, 0,
((int)(ptr_int_t)(absolute ? dcontext : 0)) + offs, size);
((int)(ptr_int_t)(absolute ? dcontext : 0)) + offs - CONTEXT_REBASE_OFFT,
size);
}
}

Expand Down Expand Up @@ -2950,7 +2952,7 @@ update_dcontext_address(opnd_t op, dcontext_t *old_dcontext, dcontext_t *new_dco
opnd_get_index(op) == REG_NULL,
"update_dcontext_address: invalid opnd");
IF_X64(ASSERT_NOT_IMPLEMENTED(false));
offs = opnd_get_disp(op) - (uint)(ptr_uint_t)old_dcontext;
offs = opnd_get_disp(op) - (uint)(ptr_uint_t)old_dcontext + CONTEXT_REBASE_OFFT;
if (offs >= 0 && offs < sizeof(dcontext_t)) {
/* don't pass raw offset, add in upcontext size */
offs += sizeof(unprotected_context_t);
Expand Down
10 changes: 8 additions & 2 deletions core/unix/os.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* Copyright (c) 2010-2025 Google, Inc. All rights reserved.
* Copyright (c) 2011 Massachusetts Institute of Technology All rights reserved.
* Copyright (c) 2000-2010 VMware, Inc. All rights reserved.
* Copyright (c) 2025 Foundation of Research and Technology, Hellas.
* *******************************************************************************/

/*
Expand Down Expand Up @@ -2378,6 +2379,9 @@ os_tls_init(void)
if (last_thread_tls_exited) /* re-attach */
last_thread_tls_exited = false;
}
#if (CONTEXT_REBASE_OFFT != 0)
set_thread_private_dcontext(NULL);
#endif
Comment on lines +2382 to +2384
Copy link
Member

Choose a reason for hiding this comment

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

Existing code compares thread private dcontext with NULL for validation check, thus this is required or they will get NULL - CONTEXT_REBASE_OFFSET and wrongly assumes thread private dcontext is ready.

Do I understand it correcly? If so, some comments to explain this call seem helpful.

ASSERT(is_thread_tls_initialized());
}

Expand Down Expand Up @@ -3134,7 +3138,7 @@ get_thread_private_dcontext(void)
pid_cached != get_process_id());
});
READ_TLS_SLOT_IMM(TLS_DCONTEXT_OFFSET, dcontext);
return dcontext;
return (dcontext_t *)(((void *)dcontext) - CONTEXT_REBASE_OFFT);
#else
/* Assumption: no lock needed on a read => no race conditions between
* reading and writing same tid! Since both get and set are only for
Expand All @@ -3146,7 +3150,8 @@ get_thread_private_dcontext(void)
if (tls_table != NULL) {
for (i = 0; i < MAX_THREADS; i++) {
if (tls_table[i].tid == tid) {
return tls_table[i].dcontext;
return (dcontext_t *)(((void *)tls_table[i].dcontext -
CONTEXT_REBASE_OFFT);
}
}
}
Expand All @@ -3158,6 +3163,7 @@ get_thread_private_dcontext(void)
void
set_thread_private_dcontext(dcontext_t *dcontext)
{
dcontext = (dcontext_t *)(((void *)dcontext) + CONTEXT_REBASE_OFFT);
#ifdef HAVE_TLS
ASSERT(is_thread_tls_allocated());
WRITE_TLS_SLOT_IMM(TLS_DCONTEXT_OFFSET, dcontext);
Expand Down
Loading