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

Jump to bottom

i#6213,i#5490: Add drmemtrace branch targets up front #6219

Merged
merged 9 commits into from
Jul 25, 2023
Merged

i#6213,i#5490: Add drmemtrace branch targets up front #6219

merged 9 commits into from
Jul 25, 2023

Conversation

derekbruening
Copy link
Contributor

@derekbruening derekbruening commented Jul 21, 2023 •
edited
Loading

The drmemtrace record type TRACE_TYPE_INSTR_CONDITIONAL_JUMP is now deprecated in offline traces where it is replaced by TRACE_TYPE_INSTR_TAKEN_JUMP and TRACE_TYPE_INSTR_UNTAKEN_JUMP. The version number is bumped to accomplish this.

Indirect branches in drmemtrace traces now contain a marker holding the actual target (TRACE_MARKER_TYPE_BRANCH_TARGET), which immediately precedes the branch record.

These changes are implemented inside raw2trace and in PT ir2trace. raw2trace tests are updated and new tests added.

Special cases with new logic and tests include:

  • Rseq side exits must specially set whether taken or untaken
  • Rseq abort rollbacks to a branch are tested as this is the case where it was not easy to identify whether a branch was taken in the past (it required particular inferences).
  • Branches prior to signals.
  • Trace-final branches: we just delete these.

Adds view support:

     2212815     1648444:     1249326 ifetch       6 byte(s) @ 0x00007f3406720707 48 3d 01 f0 ff ff    cmp    %rax, $0xfffff001
     2212816     1648445:     1249326 ifetch       2 byte(s) @ 0x00007f340672070d 73 01                jnb    $0x00007f3406720710 (untaken)
     2212817     1648445:     1249326 <marker: indirect branch target 0x7f34066a8b37>
     2212818     1648446:     1249326 ifetch       1 byte(s) @ 0x00007f340672070f c3                   ret
     2212819     1648446:     1249326 read         8 byte(s) @ 0x00007ffd91e24fa8 by PC 0x00007f340672070f
     2212820     1648447:     1249326 ifetch       5 byte(s) @ 0x00007f34066a8b37 4c 8b 54 24 48       mov    0x48(%rsp), %r10

Adds several new invariant checks and augments the existing PC continuity checks. This required a little refactoring to check branches before signals which is part of #5912. Unit tests for each case are added.

Updates the documentation and the changelist.

Issue: #5490, #6213, #5912
Fixes #6213
Fixes #5490

@derekbruening
i#6213,i#5490: Add drmemtrace branch targets up front
5aecd44 
The drmemtrace record type TRACE_TYPE_INSTR_CONDITIONAL_JUMP is now
deprecated in offline traces where it is replaced by
TRACE_TYPE_INSTR_TAKEN_JUMP and TRACE_TYPE_INSTR_UNTAKEN_JUMP.  The
version number is bumped to accomplish this.

Indirect branches in drmemtrace traces now contain a marker holding
the actual target (TRACE_MARKER_TYPE_BRANCH_TARGET), which immediately
precedes the branch record.

These changes are implemented inside raw2trace and in PT ir2trace.
raw2trace tests are updated and new tests added.

Special cases with new logic and tests include:
+ Rseq side exits must specially set whether taken or untaken
+ Rseq abort rollbacks to a branch are tested as this is the case
  where it was not easy to identify whether a branch was taken
  in the past (it required particular inferences).
+ Branches prior to signals.
+ Trace-final branches: we just delete these.

Adds view support:

```
     2212815     1648444:     1249326 ifetch       6 byte(s) @ 0x00007f3406720707 48 3d 01 f0 ff ff    cmp    %rax, $0xfffff001
     2212816     1648445:     1249326 ifetch       2 byte(s) @ 0x00007f340672070d 73 01                jnb    $0x00007f3406720710 (untaken)
     2212817     1648445:     1249326 <marker: indirect branch target 0x7f34066a8b37>
     2212818     1648446:     1249326 ifetch       1 byte(s) @ 0x00007f340672070f c3                   ret
     2212819     1648446:     1249326 read         8 byte(s) @ 0x00007ffd91e24fa8 by PC 0x00007f340672070f
     2212820     1648447:     1249326 ifetch       5 byte(s) @ 0x00007f34066a8b37 4c 8b 54 24 48       mov    0x48(%rsp), %r10
```

Adds several new invariant checks and augments the existing PC
continuity checks.  This required a little refactoring to check
branches before signals which is part of #5912.  Unit tests for each
cases are added.

Updates the documentation and the changelist.

Issue: #5490, #6213, #5912
Fixes #6213
Fixes #5490
@derekbruening
Fix Windows build warnings; exclude decoration invariants for i-filte…
a592fdf 
…red traces
@derekbruening
Handle consecutive indirect branches in the checker
29a4d17
@derekbruening
Update phys_SUDO version printout; make signal tests UNIX-only
f61f184
@derekbruening
Copy link
Contributor Author

x64 failure is #6185 api.rseq synch assert

@derekbruening
Copy link
Contributor Author

Actually the win64 has a failure which is not flaky which I missed before: seems to be failure to remove from the encoding cache; can repro in a new unit test.

abhinav92003
abhinav92003 approved these changes Jul 25, 2023
View reviewed changes
clients/drcachesim/common/trace_entry.h Outdated Show resolved Hide resolved
clients/drcachesim/common/trace_entry.h Outdated Show resolved Hide resolved
clients/drcachesim/docs/drcachesim.dox.in Show resolved Hide resolved
clients/drcachesim/docs/drcachesim.dox.in Outdated Show resolved Hide resolved
clients/drcachesim/tools/invariant_checker.h Outdated Show resolved Hide resolved
clients/drcachesim/tools/invariant_checker.cpp Outdated Show resolved Hide resolved
clients/drcachesim/tools/invariant_checker.cpp Outdated Show resolved Hide resolved
clients/drcachesim/tools/invariant_checker.cpp Show resolved Hide resolved
clients/drcachesim/tools/invariant_checker.cpp Outdated Show resolved Hide resolved
clients/drcachesim/tracer/raw2trace.cpp Show resolved Hide resolved
@derekbruening
Reviewer requests:
334bd68 
+ Remove documented guarantee that branches are delayed
  (but the impl remains; added a comment about waiting
  for users to update before changing that, if we do)
+ Added comments about some issues in signal PC continuity
  checks that #5912 should address
+ Added missing clear of last_indirect_target_
+ Use last_insr_in_cur_context_ instead of prev_instr_
+ Multiple comment wording and variable name and test parameter updates
+ Added an untaken-before-signal raw2trace unit test
+ Added an assert on no non-branch instrs being delayed
@derekbruening
Merge branch 'master' of github.com:DynamoRIO/dynamorio into i6213-cb…
5b97312 
…r-taken
@derekbruening
Fix Windows build which has no last_instr_in_cur_context_
d5ea665
@derekbruening
Go back to passing in shard->prev_instr_ as it has to match shard->pr…
69d3a7b 
…ev_instr_decoded_; #5912 covers changing both sides to the prior in the cur context
@derekbruening derekbruening merged commit 2220a83 into master Jul 25, 2023
@derekbruening derekbruening deleted the i6213-cbr-taken branch July 25, 2023 23:43
ivankyluk pushed a commit to ivankyluk/dynamorio that referenced this pull request Jul 28, 2023
@derekbruening @ivankyluk
i#6213,i#5490: Add drmemtrace branch targets up front (DynamoRIO#6219)
55c46bc 
The drmemtrace record type TRACE_TYPE_INSTR_CONDITIONAL_JUMP is now
deprecated in offline traces where it is replaced by
TRACE_TYPE_INSTR_TAKEN_JUMP and TRACE_TYPE_INSTR_UNTAKEN_JUMP. The
version number is bumped to accomplish this.

Indirect branches in drmemtrace traces now contain a marker holding the
actual target (TRACE_MARKER_TYPE_BRANCH_TARGET), which immediately
precedes the branch record.

These changes are implemented inside raw2trace and in PT ir2trace.
raw2trace tests are updated and new tests added.

Special cases with new logic and tests include:
+ Rseq side exits must specially set whether taken or untaken
+ Rseq abort rollbacks to a branch are tested as this is the case where
it was not easy to identify whether a branch was taken in the past (it
required particular inferences).
+ Branches prior to signals.
+ Trace-final and window-final branches: we just delete these.

Adds view support:

```
     2212815     1648444:     1249326 ifetch       6 byte(s) @ 0x00007f3406720707 48 3d 01 f0 ff ff    cmp    %rax, $0xfffff001
     2212816     1648445:     1249326 ifetch       2 byte(s) @ 0x00007f340672070d 73 01                jnb    $0x00007f3406720710 (untaken)
     2212817     1648445:     1249326 <marker: indirect branch target 0x7f34066a8b37>
     2212818     1648446:     1249326 ifetch       1 byte(s) @ 0x00007f340672070f c3                   ret
     2212819     1648446:     1249326 read         8 byte(s) @ 0x00007ffd91e24fa8 by PC 0x00007f340672070f
     2212820     1648447:     1249326 ifetch       5 byte(s) @ 0x00007f34066a8b37 4c 8b 54 24 48       mov    0x48(%rsp), %r10
```

Adds several new invariant checks and augments the existing PC
continuity checks. This required a little refactoring to check branches
before signals which is part of DynamoRIO#5912. Unit tests for each case are
added.  Adds a couple of comments on issues that DynamoRIO#5912 should
address.

Updates the documentation to remove the documented guarantee
that branches are delayed.

Updates the changelist.

Issue: DynamoRIO#5490, DynamoRIO#6213, DynamoRIO#5912
Fixes DynamoRIO#6213
Fixes DynamoRIO#5490
ivankyluk pushed a commit to ivankyluk/dynamorio that referenced this pull request Jul 29, 2023
@derekbruening @ivankyluk
i#6213,i#5490: Add drmemtrace branch targets up front (DynamoRIO#6219)
d37eee7 
The drmemtrace record type TRACE_TYPE_INSTR_CONDITIONAL_JUMP is now
deprecated in offline traces where it is replaced by
TRACE_TYPE_INSTR_TAKEN_JUMP and TRACE_TYPE_INSTR_UNTAKEN_JUMP. The
version number is bumped to accomplish this.

Indirect branches in drmemtrace traces now contain a marker holding the
actual target (TRACE_MARKER_TYPE_BRANCH_TARGET), which immediately
precedes the branch record.

These changes are implemented inside raw2trace and in PT ir2trace.
raw2trace tests are updated and new tests added.

Special cases with new logic and tests include:
+ Rseq side exits must specially set whether taken or untaken
+ Rseq abort rollbacks to a branch are tested as this is the case where
it was not easy to identify whether a branch was taken in the past (it
required particular inferences).
+ Branches prior to signals.
+ Trace-final and window-final branches: we just delete these.

Adds view support:

```
     2212815     1648444:     1249326 ifetch       6 byte(s) @ 0x00007f3406720707 48 3d 01 f0 ff ff    cmp    %rax, $0xfffff001
     2212816     1648445:     1249326 ifetch       2 byte(s) @ 0x00007f340672070d 73 01                jnb    $0x00007f3406720710 (untaken)
     2212817     1648445:     1249326 <marker: indirect branch target 0x7f34066a8b37>
     2212818     1648446:     1249326 ifetch       1 byte(s) @ 0x00007f340672070f c3                   ret
     2212819     1648446:     1249326 read         8 byte(s) @ 0x00007ffd91e24fa8 by PC 0x00007f340672070f
     2212820     1648447:     1249326 ifetch       5 byte(s) @ 0x00007f34066a8b37 4c 8b 54 24 48       mov    0x48(%rsp), %r10
```

Adds several new invariant checks and augments the existing PC
continuity checks. This required a little refactoring to check branches
before signals which is part of DynamoRIO#5912. Unit tests for each case are
added.  Adds a couple of comments on issues that DynamoRIO#5912 should
address.

Updates the documentation to remove the documented guarantee
that branches are delayed.

Updates the changelist.

Issue: DynamoRIO#5490, DynamoRIO#6213, DynamoRIO#5912
Fixes DynamoRIO#6213
Fixes DynamoRIO#5490
@derekbruening derekbruening mentioned this pull request Aug 18, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abhinav92003 abhinav92003 abhinav92003 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@derekbruening @abhinav92003