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

static detach/reattach: options with append semantics are appended to on subsequent reattaches #2661

Closed
Carrotman42 opened this issue Oct 6, 2017 · 5 comments
Closed

static detach/reattach: options with append semantics are appended to on subsequent reattaches #2661

Carrotman42 opened this issue Oct 6, 2017 · 5 comments
Assignees
@derekbruening
Labels
Bug-ToolFail

Comments

@Carrotman42
Copy link
Contributor

For example, when -client_lib (a liststring_t) is specified in two consecutive attach/detach runs, the second attach will simply append its value for -client_lib to the value in the first run instead of overwriting it.

I'm thinking that we should just set the dynamo_options back to default values before each time the options are parsed.
@derekbruening
Copy link
Contributor

Just to give context: this is only for static library attach/detach. As a shared library there would be no trouble here as we'd unload and reload the library, naturally resetting its state.

@derekbruening
Copy link
Contributor

Perhaps this should just be part of #2157 -- or at least linked to it

@Carrotman42 Carrotman42 mentioned this issue Jun 24, 2018
Open
@derekbruening derekbruening changed the title detach/reattach: options with append semantics are appended to on subsequent reattaches static detach/reattach: options with append semantics are appended to on subsequent reattaches Jun 25, 2018
Carrotman42 added a commit that referenced this issue Aug 7, 2018
@Carrotman42
i#2661 option append: clear options and config on attach.
837e7f2 
The change in config.c clears out the storage for configuration
variables in config_init to deal with its lazy loading in the context on
static reattach.

The change in options.c resets the dynamo_options to their default
values during options_init to ensure that values from any previous
attaches do not get carried over to the current attach.

Comes with a unit test which fails without these changes.

Fixes #2661
@derekbruening derekbruening mentioned this issue Dec 11, 2019
Merged
@derekbruening
Copy link
Contributor

Note that there is a workaround for this: the special prefix # will overwrite instead of append.

@derekbruening
Copy link
Contributor

I take it back: the # prefix is not enough by itself: the config_exit memset of myvals is also needed.

derekbruening added a commit that referenced this issue Dec 12, 2019
@derekbruening
Adds a new test which runs assembly code sequences covering corner
ad963b8 
cases twice, postprocesses each, and compares the final trace entries
using the external analyzer iterator interface and a custom analysis
tool.  This found another bug (fixed here) and gives much higher
confidence that these optimizations are safe.

Adds a -disable_optimizations flag to drmemtrace for the test to use.

Improves the pre-existing displacement elision optimization by sharing
code between the tracer and raw2trace via
offline_instru_t::opnd_disp_is_elidable() and by adding test cases to
the new test.  Also implements displacement elision for ARM and
AArch64, which is required for proper address elision without also
recording displacements.

For the final commit description, the earlier commit:
Also fixes a bug in raw2trace_t::get_instr_summary where the INOUT
parameter 'pc' was not always correctly updated.

The new test includes AArch64 assembly code.  I tested this on an
AArch64 machine by temporarily enabling these static-DR tests.
Unfortunately i#2007 prevents us from enabling them on Travis for now.
I also fixed a TLS segment setup bug in static DR mode to get the test
to run.

The new test includes ARM assembly code.  I did test building it, and
I attempted to run it, including implementing dr_app_start() and
dr_running_under_dynamorio() for ARM here: but there is more to be
done for start/stop support on ARM so I bailed at that point.

For the new test on Windows, I relaxed two asserts on thread pool
threads as initial efforts to fix #3983 (and disabled the thread pool
in the test on Windows as a workaround for the rest of #983).

Includes a part of PR #3120 plus a '#' option prefix to work around

Issue: #2007, #3983, #2661
derekbruening added a commit that referenced this issue Dec 15, 2019
@derekbruening
i#2001 trace perf: elide rip-rel and same-base addrs (#3977)
575b6d4 
Reduces drcachesim offline tracing overhead by eliding rip-relative
and same-unmodified-base addresses from the recorded trace,
reconstructing their values during post-processing.  In measurements
this is pretty significant, removing 12%-17% of entries that need to
be written out during tracing.

Adds identification of elidable memory operands to instru_offline_t,
exported both to the runtime tracer and the raw2trace post-processor.

Changes raw2trace's instruction cache key to a pair <tag,pc> to handle
tail-duplicated blocks.  Adds elision identification through a
constructed instrlist when first encountering a block tag.  Adds a new
struct memref_summary_t to store elision information with each cached
memory operand.

Increases the offline file version and adds versioning support for
backward compatibility with no-elision traces, as well as to make it
easier to keep compatibility when more elision cases are added in the
future.

Adds a file type to the offline file header to identify filtered
traces as a sanity check and to avoid extra work when there are
no elided addresses at all.  Another file type flag identifies
whether any optimizations (this and the existing displacement
elision) are present, making it possible to disable them for
testing purposes.  Adds a -disable_optimizations flag for this.

Adds a new test burst_traceopts which runs assembly code
sequences covering corner cases twice, once with and once without
optimizations.  It then postprocesses each, and compares the
final trace entries using the external analyzer iterator
interface.  This found bugs during development and provides
confidence that these optimizations are safe.

Improves the pre-existing displacement elision optimization by sharing
code between the tracer and raw2trace via
offline_instru_t::opnd_disp_is_elidable() and by adding test cases to
the new test.  Also implements displacement elision for ARM and
AArch64, which is required for proper address elision without also
recording displacements.

The new test includes AArch64 and ARM assembly code.  The AArch64
was tested by temporarily enabling these static-DR
tests (unfortunately i#2007 prevents us from enabling them on
Travis for now).  The ARM assembly builds but is not testable due
to missing start/stop features on ARM>

Adds a statistics interface to retrieve raw2trace metrics.  The
initial metric is the number of elided addresses.

Includes a part of PR #3120 (the memset in d_r_config_exit())
plus a '#' option prefix to work around #2661.

Fixes a bug revealed by the tighter post-processing constraints
with elision: Do not count an artificial jump from a trampoline
return as an instruction in the recorded block tag entry.
Counting it resulted in a duplicated instruction during
post-processing.

Issue: #2001, #2661
@derekbruening
Copy link
Contributor

#4139 is the same issue with droption options

@derekbruening derekbruening mentioned this issue Feb 26, 2020
Closed
derekbruening added a commit that referenced this issue Mar 25, 2020
@derekbruening
i#3048 func-trace: Improve function tracing re-attach
69f4b45 
Clears the list of traced functions on exit to avoid accumulation on re-attach.

Sets the handoff separately for each attach in the burst_malloc test.

There are still problems with re-setting options: droption #4139 and
core #2661 (with the '#' prefix workaround).  Those are left to their
respective issues.

Issue: #3048, #2661, #4139
@derekbruening derekbruening mentioned this issue Mar 25, 2020
Merged
derekbruening added a commit that referenced this issue Mar 26, 2020
@derekbruening
i#3048 func-trace: Improve function tracing re-attach (#4222)
d7942b3 
Clears the list of traced functions on exit to avoid accumulation on re-attach.

Sets the handoff separately for each attach in the burst_malloc test.

There are still problems with re-setting options: droption #4139 and
core #2661 (with the '#' prefix workaround).  Those are left to their
respective issues.

Issue: #3048, #2661, #4139
@derekbruening derekbruening mentioned this issue Jul 1, 2020
Open
@derekbruening derekbruening mentioned this issue Mar 7, 2021
Closed
@derekbruening derekbruening mentioned this issue Feb 9, 2022
Merged
@derekbruening derekbruening self-assigned this Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@derekbruening derekbruening

Labels
Bug-ToolFail
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Carrotman42 @derekbruening