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.

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#2974 trace support for AArch64, part 2: added trace implementation #5045

Merged
merged 18 commits into from
Sep 1, 2021
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
f29a4da
i#2974 trace support for AArch64, part 2: implementation of the main …
Vincent-lau Aug 11, 2021
8a14b7b
i#2974 trace support for AArch64, part 3: removed comments in code
Vincent-lau Aug 12, 2021
7b5b87d
i#2974 trace support for AArch64, part 4: Code clean up
Vincent-lau Aug 17, 2021
d0a5ff4
i#2974 trace support for AArch64, part 5: Adding assertions
Vincent-lau Aug 20, 2021
fc4fdd4
i#2974 trace support for AArch64, part 6: Code clean up
Vincent-lau Aug 24, 2021
e6d9fd6
i#2974 trace support for AArch64, part 7: Use higher level IR query API
Vincent-lau Aug 27, 2021
b9ada87
i#2974 trace support for AArch64, part 7: Add translation info in
Vincent-lau Aug 30, 2021
e5b89cb
i#2974 trace support for AArch64, part 9: Added comments on switching to
Vincent-lau Aug 31, 2021
5b20ed0
i#2974 trace support for AArch64, part 11: Fixed incorrect branch upper
Vincent-lau Aug 31, 2021
d09d2c9
i#2974 trace support for AArch64, part 12: Fixed clang-format issue
Vincent-lau Aug 31, 2021
88c535d
i#2974 trace support for AArch64, part 13: Fixed a bug of appending the
Vincent-lau Sep 1, 2021
ed826fd
i#2974 trace support for AArch64, part 14: Adding comments on
Vincent-lau Sep 1, 2021
6c7cf64
i#2974 trace support for AArch64, part 15: Implementing
Vincent-lau Sep 1, 2021
a2047ba
i#2974 trace support for AArch64, part 16: Adding comment on unsupported
Vincent-lau Sep 1, 2021
fc70e80
i#2974 trace support for AArch64, part 17: Fixed clang-format issue
Vincent-lau Sep 1, 2021
0d456fa
i#2974 trace support for AArch64, part 18: Adding comment on preferring
Vincent-lau Sep 1, 2021
4dffc58
i#2974 trace support for AArch64, part 19: Improved comments about
Vincent-lau Sep 1, 2021
3012697
Merge branch 'master' into i2974-trace
Vincent-lau Sep 1, 2021
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
22 changes: 14 additions & 8 deletions core/arch/aarch64/emit_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -370,14 +370,14 @@ patch_branch(dr_isa_mode_t isa_mode, cache_pc branch_pc, cache_pc target_pc,
ASSERT(ALIGNED(branch_pc, 4) && ALIGNED(target_pc, 4));
Copy link
Contributor

Choose a reason for hiding this comment

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

i#2974 trace support for AArch64, part 18: Adding comment on preferring
linear control flow

Note that the ", part N: " is a convention for merges to master: i.e., this PR being merged will be part 1; the next PR for trace building merged into master would be part 2 (not part 19 or sthg: the PR-internal ones don't count toward the master merges).

Copy link
Collaborator Author

@Vincent-lau Vincent-lau Sep 1, 2021

Choose a reason for hiding this comment

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

sure, that's why I put part 2 in my final commit message above. Have a look if you want and see if there is anything missing or not quite right.

if ((enc & 0xfc000000) == 0x14000000) { /* B */
ASSERT(off + 0x8000000 < 0x10000000);
*pc_writable = (0x14000000 | (0x03ffffff & off >> 2));
*pc_writable = (0x14000000 | (0x03ffffff & (off >> 2)));
} else if ((enc & 0xff000010) == 0x54000000 ||
(enc & 0x7e000000) == 0x34000000) { /* B.cond, CBNZ, CBZ */
ASSERT(off + 0x40000 < 0x80000);
*pc_writable = (enc & 0xff00001f) | (0x00ffffe0 & off >> 2 << 5);
ASSERT(off + 0x100000 < 0x200000);
Vincent-lau marked this conversation as resolved.
Show resolved Hide resolved
*pc_writable = (enc & 0xff00001f) | (0x00ffffe0 & (off >> 2 << 5));
} else if ((enc & 0x7e000000) == 0x36000000) { /* TBNZ, TBZ */
ASSERT(off + 0x2000 < 0x4000);
*pc_writable = (enc & 0xfff8001f) | (0x0007ffe0 & off >> 2 << 5);
ASSERT(off + 0x8000 < 0x10000);
*pc_writable = (enc & 0xfff8001f) | (0x0007ffe0 & (off >> 2 << 5));
} else
ASSERT(false);
if (hot_patch)
Expand Down Expand Up @@ -446,8 +446,14 @@ indirect_linkstub_stub_pc(dcontext_t *dcontext, fragment_t *f, linkstub_t *l)
cache_pc cti = EXIT_CTI_PC(f, l);
Vincent-lau marked this conversation as resolved.
Show resolved Hide resolved
Vincent-lau marked this conversation as resolved.
Show resolved Hide resolved
if (!EXIT_HAS_STUB(l->flags, f->flags))
return NULL;
ASSERT(decode_raw_is_jmp(dcontext, cti));
return decode_raw_jmp_target(dcontext, cti);
if (decode_raw_is_jmp(dcontext, cti))
return decode_raw_jmp_target(dcontext, cti);
/* In trace, we might have cbz/cbnz to indirect linkstubs. */
if (decode_raw_is_cond_branch_zero(dcontext, cti))
return decode_raw_cond_branch_zero_target(dcontext, cti);
/* There should be no other types of branch to linkstubs. */
ASSERT_NOT_REACHED();
return NULL;
}

cache_pc
Expand Down Expand Up @@ -528,7 +534,7 @@ insert_fragment_prefix(dcontext_t *dcontext, fragment_t *f)
/* ldp x0, x1, [x(stolen), #(off)] */
*(uint *)pc = (0xa9400000 | (DR_REG_X0 - DR_REG_X0) | (DR_REG_X1 - DR_REG_X0) << 10 |
(dr_reg_stolen - DR_REG_X0) << 5 | TLS_REG0_SLOT >> 3 << 10);
pc += 4;
pc += AARCH64_INSTR_SIZE;
f->prefix_size = (byte)(((cache_pc)pc) - write_start);
ASSERT(f->prefix_size == fragment_prefix_size(f->flags));
}
Expand Down
8 changes: 8 additions & 0 deletions core/arch/arch_exports.h
Original file line number Diff line number Diff line change
Expand Up @@ -2196,6 +2196,14 @@ int
append_trace_speculate_last_ibl(dcontext_t *dcontext, instrlist_t *trace,
app_pc speculate_next_tag, bool record_translation);

/* XXX i#5062 In the long term we should have this only called in mangle_trace()
* and it should be removed from here.
*/
#ifdef AARCH64
int
fixup_indirect_trace_exit(dcontext_t *dcontext, instrlist_t *trace);
Copy link
Contributor

Choose a reason for hiding this comment

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

See below: long-term this should only be inside mangle_trace() and so this would be removed when #5062 is implemented. Please add a XXX i#5062 comment about this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of 'here', maybe mention end_and_emit_trace.

#endif

uint
forward_eflags_analysis(dcontext_t *dcontext, instrlist_t *ilist, instr_t *instr);

Expand Down
6 changes: 6 additions & 0 deletions core/arch/emit_utils_shared.c
Original file line number Diff line number Diff line change
Expand Up @@ -1294,6 +1294,11 @@ update_indirect_exit_stub(dcontext_t *dcontext, fragment_t *f, linkstub_t *l)
int
fragment_prefix_size(uint flags)
{
#ifdef AARCH64
/* For AArch64, there is no need to save the flags
* so we always have the same ibt prefix. */
return fragment_ibt_prefix_size(flags);
#else
if (use_ibt_prefix(flags)) {
return fragment_ibt_prefix_size(flags);
} else {
Expand All @@ -1302,6 +1307,7 @@ fragment_prefix_size(uint flags)
else
return 0;
}
#endif
}

#ifdef PROFILE_RDTSC
Expand Down
Loading