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

add trace support for AArch64 #2974

Open
fhahn opened this issue Apr 29, 2018 · 7 comments
Open

add trace support for AArch64 #2974

fhahn opened this issue Apr 29, 2018 · 7 comments

Comments

@fhahn
Copy link
Contributor

fhahn commented Apr 29, 2018

Similar to #1668 for AArch64. There is a bunch of x86-specific trace building code that needs to be ported.

@fhahn
Copy link
Contributor Author

fhahn commented Apr 29, 2018

@egrimley shared a draft patch at #2442

@Vincent-lau
Copy link
Collaborator

Hi I am working on a project to add trace support for AArch64, I am wondering if you have any suggestions on what would be a good starting point?

@AssadHashmi
Copy link
Contributor

Hi I am working on a project to add trace support for AArch64, I am wondering if you have any suggestions on what would be a good starting point?

Have a look at the draft patch #2442 and see if it can be updated to the latest master.

@Vincent-lau
Copy link
Collaborator

Hi, I am planning to submit a patch to add trace support for AArch64, but I am not yet a member of DynamoRIO, shall I submit a few small patches by forking the repo before asking to be added as the member?

@AssadHashmi
Copy link
Contributor

Hi, I am planning to submit a patch to add trace support for AArch64, but I am not yet a member of DynamoRIO, shall I submit a few small patches by forking the repo before asking to be added as the member?

There's no need to fork, I've added you as a contributor. You should receive an invite message from GitHub.
Please follow the advice on contributing here https://dynamorio.org/page_code_reviews.html
Thanks for contributing, we look forward to your pull request.

Vincent-lau added a commit that referenced this issue Aug 11, 2021
…part of support

This patch incorporated changes from patch #2442, and fixed some corner cases
where the assumption was incorrect and caused the program to crash.

Trace support is not yet enabled by default, but can be enabled with "-enable_traces".

A large part of this work was based on work done by Kevin Zhou. Please see branch
i1569-trace and patch #2442 for the original version.

Issues: #1569, #2974
Vincent-lau added a commit that referenced this issue Aug 11, 2021
… with arm

Removed the use of movz which is only available on AArch64.

Issues: #1569, #2974
Vincent-lau added a commit that referenced this issue Aug 11, 2021
…64 (#5042)

This patch added implementation for instr_invert_cbr() on AArch64 as well as
changing the implementation of instr_set_predicate by clearing the predicate
bits before setting them.

Issues: #2974, #1569
Vincent-lau added a commit that referenced this issue Aug 11, 2021
…part of support

This patch incorporated changes from patch #2442, and fixed some corner cases
where the assumption was incorrect and caused the program to crash.

Trace support is not yet enabled by default, but can be enabled with "-enable_traces".

A large part of this work was based on work done by Kevin Zhou. Please see branch
i1569-trace and patch #2442 for the original version.

Issues: #1569, #2974
Vincent-lau added a commit that referenced this issue Aug 12, 2021
Removed all the commented code and an unused parameter.

Issues: #1569, #2974
@fhahn fhahn removed their assignment Aug 13, 2021
Vincent-lau added a commit that referenced this issue Aug 17, 2021
Some clean up according to the review. Also changing the implementation
of check_patched_ibl to merge two checks into one function.

Issues: #1569, #2974
Vincent-lau added a commit that referenced this issue Aug 20, 2021
Replaced some of the if-conditions with assertions and improved code
comments.

Issues: #1569, #2974
Vincent-lau added a commit that referenced this issue Aug 24, 2021
Changed the condition of checking for mangled stolen register to include
the ldr instruction. Added comment on clarifying the need to iterate over the
entire trace to find all indirect exits.

Issues: #1569, #2974
Vincent-lau added a commit that referenced this issue Aug 27, 2021
Changed implementation of checking for str/mov instructions to use the
decoder.

Issues: #1569, #2974
Vincent-lau added a commit that referenced this issue Aug 30, 2021
fixup_indirect_trace_exit

Added translation information when fixing up the indirect trace exits
and added call to fixup_indirect_trace_exit when recreating the fragment ilist

Issues: #1569, #2974
Vincent-lau added a commit that referenced this issue Aug 31, 2021
simpler approach

Added a comment on the possibility to switch to a simpler approach of
not decoding from code cache. And changed setting translation of instructions
using INSTR_XL8 macro.

Issues: #1569, #2974
Vincent-lau added a commit that referenced this issue Aug 31, 2021
bound

Fixed a problem where the branch upper bound is calculated correctly.

Issues: #1569, #2974
Vincent-lau added a commit that referenced this issue Aug 31, 2021
Vincent-lau added a commit that referenced this issue Sep 1, 2021
Vincent-lau added a commit that referenced this issue Sep 1, 2021
Vincent-lau added a commit that referenced this issue Sep 1, 2021
@derekbruening
Copy link
Contributor

Documenting a proposal that eliminates the internal control flow of fixup sequences, which break the linear assumptions of translation:

Pasting from https://github.com/DynamoRIO/dynamorio/pull/5045/files/b9ada873ecc0b86128169aae0bd7ad648c0f921b..fc70e80bad8a66b7f417c27673fa5659e96ff1ef#r697566139

One proposal is to change the i.b. comparison to use x2 instead of x0 as the temp reg holding the expected target immediate. Then the only thing missing is copying from the actual target reg into x2, and maybe that could be done locally. It would use a taken forward branch on the match which maybe is not ideal but certainly simple:

     str x2, TLS_REG2_SLOT
     mov x2, #trace_next_target
     eor x2, x2, jump_target
     cbz x2, match
     mov x2, jump_target
     b ibl_routine_regular_exit_stub
match:
     ldr x2, TLS_REG2_SLOT

Then we eliminate this special-case trace_exit_label.

(If jump_target == x2 then a different scheme needs to be used -- might require 2 spills b/c we want to put the app value in the x2 slot and have a local scratch.)

Vincent-lau added a commit that referenced this issue Sep 1, 2021
linear control flow

Currently fixup_indirect_trace_exit adds a special trace_exit_label that
breaks the linear control flow which is assumed by many places in the code. We need to
consider an alternative scheme to avoid this problem or add special support for the
trace_exit_label.

Also reverted changes on incorrect upper branch bound calculation.

Issues: #1569, #2974
Vincent-lau added a commit that referenced this issue Sep 1, 2021
Vincent-lau added a commit that referenced this issue Sep 1, 2021
…5045)

This patch incorporated changes from PR #2442 that implemented the initial version
of trace support for AArch64.

This patch also fixed some corner cases not considered in PR #2442
where the assumption was incorrect and caused the program to crash.

Trace support is not yet enabled by default, but can be enabled with "-enable_traces".

This commit introduces internal control flow by adding a trace_exit_label in
fixup_indirect_trace_exit, which might break code that assumes linear control
flow (such as translate.c). 
Either special support is needed for this trace_exit_label or alternative
schemes should be used that has a linear control.

Some complexities in this commit can be removed once we have #5062 
implemented and decode_fragment eliminated.

Co-authored-by: Kevin Zhou <[email protected]>

Issues: #1569, #2974
@derekbruening
Copy link
Contributor

Documenting a fundamental issue with tracing building on AArch64: exclusive monitors.

DR assumes it can unlink a block for the purpose of recording where it goes next, in order to collect the set of consecutive blocks to use to build up a trace.

However, on AArch64, unlinking can perturb the app, since the exit stub itself along with all the DR code contains memory traffic which is likely to lose an exclusive monitor. So without something like -ldstex2cas (see https://dynamorio.org/page_ldstex.html), the trace recording path would likely never capture a successful exclusive monitor acquisition. Yet another problem to add to that page: -ldstex2cas (or a similar solution) is needed for core DR operation and not just instrumentation tools.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants