drbbdup fails to interoperate with drmgr emulation API #5390
Assignees
Comments
This is appealing. The downside is that all drbbdup users must use the drmgr_orig_app_instr_for_fetch(), etc. operations whenever they examine the app instruction being instrumented or else they'll get the wrong instruction quite frequently. Previously, only tool code that explicitly used rep-string or scatter-gather expansions or that explicitly wanted to be emulation-aware needed to do this. It's yet another layer on top of the base interfaces...but it still feels like the cleanest approach. @johnfxgalea any opinions here? |
|
So I am actually interested in integrating drbbdup in drmgr as I think we would be able to reduce its api complexity. If this had to happen, how would it be a solution for this issue? |
drmgr_orig_app_instr_for_fetch and its sibling use the stored |
|
So from the perspective of clients using drbbdup, instrumentation routines would no longer have |
Yes, but one option would be to keep the separate Long-term maybe everything should take |
|
And at the moment, what is stopping you from using |
That is the wrong instruction for rep-string or scatter-gather expansions. It results in significant inaccuracies in the trace as those expansions can account for a large number of dynamic instructions, so getting it wrong matters. |
So right now this is a blocker to making the switch to use drbbdup in drmemtrace. Since we want to use this ASAP...I might put in a short-term workaround that can be undone if a longer-term change like integration into drmgr happens. |
derekbruening
added a commit
that referenced
this issue
Mar 4, 2022
Adds emulation markers for each non-cloned final drbbdup instruction to ensure that drmgr_orig_app_instr_for_fetch() and drmgr_orig_app_instr_for_operands() work properly with drbbdup. Adds a sanity test that those emulation queries never see drbbdup's jump to a label. The test fails without the fix here. Fixes #5390
derekbruening
added a commit
that referenced
this issue
Mar 4, 2022
Adds emulation markers for each non-cloned final drbbdup instruction to ensure that drmgr_orig_app_instr_for_fetch() and drmgr_orig_app_instr_for_operands() work properly with drbbdup. Adds a sanity test that those emulation queries never see drbbdup's jump to a label. The test fails without the fix here. Fixes #5390
|
Another issue: will DR_EMULATE_REST_OF_BLOCK in 1st case then apply to every full block dup after that? |
derekbruening
added a commit
that referenced
this issue
Mar 5, 2022
Adds emulation markers for each non-cloned final drbbdup instruction to ensure that drmgr_orig_app_instr_for_fetch() and drmgr_orig_app_instr_for_operands() work properly with drbbdup. These marker labels are hidden from drbbdup callbacks. Adds a new drbbdup-emul test which employs drutil_expand_rep_str as well as its own emulation markers. It ensures that emulation queries never see drbbdup's jump to a label. The test fails without the fix here. Fixes #5390
|
Re-opening as this was not fully fixed: my code only marked the non-final drbbdup cases with emulation markers. The final case doesn't have a jump to the exit: it just ends in a label. drmemtrace ends up not seeing any block-final branch as a result. I found this via the invariant_checker on WIndows where there happened to be a block with nothing but a jump, which was omitted and thus there was a gap: I can repro in an asm app on linux. Using "instr" instead of drmgr_orig_app_instr_for_*: bug goes away. Looks like drbbdup calls the insert cb while at the DRBBDUP_LABEL_EXIT But, putting emul start+end labels around the exit label doesn't work b/c So we either need drbbdup_is_at_end() to return true for the emul start |
derekbruening
added a commit
that referenced
this issue
Mar 8, 2022
Adds new options -trace_for_instrs and -retrace_every_instrs to drcachesim for periodic trace bursts of an unmodified application. Implements them by adapting the existing drbbdup cases for switching between -trace_after_instrs and full tracing. Adds documentation on the new options. Adds instru_t::get_instr_count to count instuctions while tracing, to know when a tracing burst window is finished. Uses a local counter only added to the global every 10K instructions to avoid synchronization costs. Adds a new marker with the ordinal of the trace window. This marker is added to each buffer header. This, combined with a new check for the window having changed to ensure a buffer dump at the end of each block, limits the possible window drift to one block's worth of data. Augments raw2trace to avoid delaying a branch across a window change. Augments the view tool to mark window changes and delay timestamp output to group with the proper window (it is difficult to actually reorder timestamp and window entries). Augments the basic_counts tool to track and display per-window global statistics. Augments the invariant_checker tool to not complain on a control-flow gap across a window. Adds a test of this: but disables it for Windows temporarily due to more emulation interopability issues which #5390 covers. Adds a simple online test and a simple offline test that just confirm multiple windows are hit on simple_app. Adds an assembly test with precise values for the windows. Issue: #3995, #5390
derekbruening
added a commit
that referenced
this issue
Mar 8, 2022
…st missing marker after timestamp; disable invar test for this PR b/c even on linux it sometimes hits the #5390 missing jump bug
derekbruening
added a commit
that referenced
this issue
Mar 9, 2022
Adds new options -trace_for_instrs and -retrace_every_instrs to drcachesim for periodic trace bursts of an unmodified application. Implements them by adapting the existing drbbdup cases for switching between -trace_after_instrs and full tracing. Adds documentation on the new options. Adds instru_t::get_instr_count to count instuctions while tracing, to know when a tracing burst window is finished. Uses a local counter only added to the global every 10K instructions to avoid synchronization costs. Adds a new marker with the ordinal of the trace window. This marker is added to each buffer header. This, combined with a new check for the window having changed to ensure a buffer dump at the end of each block, limits the possible window drift to one block's worth of data. Augments raw2trace to avoid delaying a branch across a window change. Augments the view tool to mark window changes and delay timestamp output to group with the proper window (it is difficult to actually reorder timestamp and window entries). Augments the basic_counts tool to track and display per-window global statistics. Augments the invariant_checker tool to not complain on a control-flow gap across a window. Adds a test of this: but disables it for Windows temporarily due to more emulation interopability issues which #5390 covers. Adds a simple online test and a simple offline test that just confirm multiple windows are hit on simple_app. Adds an assembly test with precise values for the windows. Issue: #3995, #5390
derekbruening
added a commit
that referenced
this issue
Mar 9, 2022
…st missing marker after timestamp; disable invar test for this PR b/c even on linux it sometimes hits the #5390 missing jump bug
derekbruening
added a commit
that referenced
this issue
Mar 9, 2022
Fixes several shortcomings in the initial attempt to use emulation markers to support the drmgr emulation API in drbbdup, and fixes related issues in drmemtrace when it uses drbbdup. Adds missing emulation markers for a special instr for the last bbdup case (previously only the earlier cases were marked). Removes emulation markers from the analysis copy in drbbdup_extract_bb_copy() (otherwise drmemtrace sees them and incorrectly disables elision). Fixes the drmemtrace check for elision labels to use "where" except when "app" is actually an exclusive store, to properly find the labels and elide. Enables the tools.drcacheoff.windows-invar test which now passes on all platforms. Updates the drbbdup-emul-test to cover the drbbdup changes. Fixes #5390
derekbruening
added a commit
that referenced
this issue
Mar 9, 2022
Adds new options -trace_for_instrs and -retrace_every_instrs to drcachesim for periodic trace bursts of an unmodified application. Implements them by adapting the existing drbbdup cases for switching between -trace_after_instrs and full tracing. Adds documentation on the new options. Adds instru_t::get_instr_count to count instuctions while tracing, to know when a tracing burst window is finished. Uses a local counter only added to the global every 10K instructions to avoid synchronization costs. Adds a new marker with the ordinal of the trace window. This marker is added to each buffer header. This, combined with a new check for the window having changed to ensure a buffer dump at the end of each block, limits the possible window drift to one block's worth of data. Augments raw2trace to avoid delaying a branch across a window change. Augments the view tool to mark window changes and delay timestamp output to group with the proper window (it is difficult to actually reorder timestamp and window entries). Augments the basic_counts tool to track and display per-window global statistics. Augments the invariant_checker tool to not complain on a control-flow gap across a window. Adds a test of this: but disables it for Windows temporarily due to more emulation interopability issues which #5390 covers. Adds a simple online test and a simple offline test that just confirm multiple windows are hit on simple_app. Adds an assembly test with precise values for the windows. Issue: #3995, #5390
derekbruening
added a commit
that referenced
this issue
Mar 9, 2022
…st missing marker after timestamp; disable invar test for this PR b/c even on linux it sometimes hits the #5390 missing jump bug
derekbruening
added a commit
that referenced
this issue
Mar 9, 2022
Fixes several shortcomings in the initial attempt to use emulation markers to support the drmgr emulation API in drbbdup, and fixes related issues in drmemtrace when it uses drbbdup. Adds missing emulation markers for a special instr for the last bbdup case (previously only the earlier cases were marked). Removes emulation markers from the analysis copy in drbbdup_extract_bb_copy() (otherwise drmemtrace sees them and incorrectly disables elision). Fixes the drmemtrace check for elision labels to use "where" except when "app" is actually an exclusive store, to properly find the labels and elide. Enables the tools.drcacheoff.windows-invar test which now passes on all platforms. Updates the drbbdup-emul-test to cover the drbbdup changes. Fixes #5390
|
I keep hitting more cases where drbbdup does not interoperate with drmemtrace, and they are not easy to debug. Adding the emulation markers for the final case caused a number of problems with drmemtrace elision. Finally tracked down all the problems. This use of drbbdup by drmemtrace has been a lot more work than anticipated. |
derekbruening
added a commit
that referenced
this issue
Mar 9, 2022
Fixes several shortcomings in the initial attempt to use emulation markers to support the drmgr emulation API in drbbdup, and fixes related issues in drmemtrace when it uses drbbdup. Adds missing emulation markers for a special instr for the last bbdup case (previously only the earlier cases were marked). Removes emulation markers from the analysis copy in drbbdup_extract_bb_copy() (otherwise drmemtrace sees them and incorrectly disables elision). Fixes the drmemtrace check for elision labels to use "where" except when "app" is actually an exclusive store, to properly find the labels and elide. Enables the tools.drcacheoff.windows-invar test which now passes on all platforms. Updates the drbbdup-emul-test to cover the drbbdup changes. Fixes #5390
derekbruening
added a commit
that referenced
this issue
Mar 9, 2022
Adds new options -trace_for_instrs and -retrace_every_instrs to drcachesim for periodic trace bursts of an unmodified application. Implements them by adapting the existing drbbdup cases for switching between -trace_after_instrs and full tracing. Adds documentation on the new options. Adds instru_t::get_instr_count to count instuctions while tracing, to know when a tracing burst window is finished. Uses a local counter only added to the global every 10K instructions to avoid synchronization costs. Adds a new marker with the ordinal of the trace window. This marker is added to each buffer header. This, combined with a new check for the window having changed to ensure a buffer dump at the end of each block, limits the possible window drift to one block's worth of data. Augments raw2trace to avoid delaying a branch across a window change. Augments the view tool to mark window changes and delay timestamp output to group with the proper window (it is difficult to actually reorder timestamp and window entries). Augments the basic_counts tool to track and display per-window global statistics. Augments the invariant_checker tool to not complain on a control-flow gap across a window. Adds a test of this: but disables it temporarily due to more emulation interopability issues which #5390 covers. Adds a simple online test and a simple offline test that just confirm multiple windows are hit on simple_app. Adds an assembly test with precise values for the windows. Issue: #3995, #5390
derekbruening
added a commit
that referenced
this issue
Mar 10, 2022
Fixes several shortcomings in the initial attempt to use emulation markers to support the drmgr emulation API in drbbdup, and fixes related issues in drmemtrace when it uses drbbdup. Adds missing emulation markers for a special instr for the last bbdup case (previously only the earlier cases were marked). Removes emulation markers from the analysis copy in drbbdup_extract_bb_copy() (otherwise drmemtrace sees them and incorrectly disables elision). Fixes the drmemtrace check for elision labels to use "where" except when "app" is actually an exclusive store, to properly find the labels and elide. Enables the tools.drcacheoff.windows-invar test which now passes on all platforms. Updates the drbbdup-emul-test to cover the drbbdup changes. Fixes #5390
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
For #3995 I'm integrating drbbdup with drmemtrace, the tracer for drcachesim.
But drmemtrace uses the drmgr emulation support:
Those routines are not supported by drbbdup, which splits the
whereto insert from the app instr being instrumented, to handle the inability to clone a block-final branch or syscall.This causes drmemtrace to instrument the wrong instruction.
Xref past discussions on possibly integrating drbbdup with drmgr.
A possible simpler solution is to add a drmgr API to set the current app instruction.
Or, could we re-implement the drbbdup
wherevsinstrsplit to instead use the emulation API itself?That is one of the intended uses of the emulation API, for app instr rewrites that ease instrumentation (such as rep string or scatter-gather expansion): it is not only for pure emulation.
The text was updated successfully, but these errors were encountered: