Skip to content

Commit

Permalink
i#4139: Add method to clear droption values for static re-attach (#5335)
Browse files Browse the repository at this point in the history
The values of droption accumulated options are not reset on detach,
causing them to accumulate in a statically-linked client.

There is no control point for droption to do this automatically, so we
add droption_parser_t::clear_values() that a client can call.

Adds a call to clear_values() in drmemtrace.

Augments the drmemtrace burst_malloc test to fail when the options are
not reset (i.e., fail without this fix).  This serves as a test of the
new feature, as the existing droption tests do not have static clients
nor simple ways to add re-attaches.

Adds a section to the droption documentation explaining the new
feature.

Fixes #4139
  • Loading branch information
derekbruening authored Feb 10, 2022
1 parent cc8bd67 commit 4570a68
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 5 deletions.
2 changes: 2 additions & 0 deletions api/docs/release.dox
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ Further non-compatibility-affecting changes include:
- Added alias support to droption.
- The drcpusim option -blacklist was renamed to -blocklist but the old name
is still accepted.
- Added droption_parser_t::clear_values() for re-setting accumulating option
values on re-attach for statically linked clients.

The changes between version 9.0.0 and 8.0.0 include the following compatibility
changes:
Expand Down
3 changes: 2 additions & 1 deletion clients/drcachesim/tests/burst_malloc.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* **********************************************************
* Copyright (c) 2016-2020 Google, Inc. All rights reserved.
* Copyright (c) 2016-2022 Google, Inc. All rights reserved.
* **********************************************************/

/*
Expand Down Expand Up @@ -157,6 +157,7 @@ main(int argc, const char *argv[])
// Test aliases with differing arg counts.
" -record_function \"has_aliases|2&alias_1arg|1&alias_3args|3\""
#endif
// Deliberately test a duplication warning on "malloc".
// Test large values that require two entries.
" -record_function \"malloc|1&return_big_value|1\"'"))
std::cerr << "failed to set env var!\n";
Expand Down
17 changes: 16 additions & 1 deletion clients/drcachesim/tests/offline-burst_malloc.templatex
Original file line number Diff line number Diff line change
@@ -1,6 +1,21 @@
pre-DR init
Warning: duplicated function name .*
Warning: duplicated function name malloc in [^W]*
pre-DR start
pre-DR detach
DynamoRIO statistics:
*Peak threads under DynamoRIO control : *1
.*
all done
pre-DR init
Warning: duplicated function name malloc in [^W]*
pre-DR start
pre-DR detach
DynamoRIO statistics:
*Peak threads under DynamoRIO control : *1
.*
all done
pre-DR init
Warning: duplicated function name malloc in [^W]*
pre-DR start
pre-DR detach
DynamoRIO statistics:
Expand Down
2 changes: 0 additions & 2 deletions clients/drcachesim/tests/offline-burst_reattach.templatex
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
pre-DR init
Warning: duplicated function name .*
.*
pre-DR start
pre-DR detach
.*
Expand Down
4 changes: 3 additions & 1 deletion clients/drcachesim/tracer/tracer.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* ******************************************************************************
* Copyright (c) 2011-2021 Google, Inc. All rights reserved.
* Copyright (c) 2011-2022 Google, Inc. All rights reserved.
* Copyright (c) 2010 Massachusetts Institute of Technology All rights reserved.
* ******************************************************************************/

Expand Down Expand Up @@ -2000,6 +2000,8 @@ event_exit(void)
drmgr_exit();
func_trace_exit();
drx_exit();
/* Avoid accumulation of option values on static-link re-attach. */
droption_parser_t::clear_values();
}

static bool
Expand Down
9 changes: 9 additions & 0 deletions ext/droption/droption.dox
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ declaration and parsing for both clients and standalone programs.
- \ref sec_droption_types
- \ref sec_droption_html
- \ref sec_droption_aliases
- \ref sec_droption_reattach

\section sec_droption_setup Setup

Expand Down Expand Up @@ -190,4 +191,12 @@ droption_t<std::string> op_blocklist(
"should not be reported.");
\endcode

\section sec_droption_reattach Clearing Accumulated Values on Re-attach

For options that accumulate values (#DROPTION_FLAG_ACCUMULATE), a
re-attach with a statically linked client will keep the old value from
the prior attach. The `droption_parser_t::clear_values()` function
can be called from the client on exit or initialization to clear all
the values.

*/
26 changes: 26 additions & 0 deletions ext/droption/droption.h
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,22 @@ class droption_parser_t {
return names_[0];
}

/**
* For clients statically linked into the target application, global state is not
* reset between a detach and re-attach. Thus, #DROPTION_FLAG_ACCUMULATE options
* will append to their values from the prior run. This function is provided for
* a client to call on detach or re-attach to reset these values.
*/
static void
clear_values()
{
for (std::vector<droption_parser_t *>::iterator opi = allops().begin();
opi != allops().end(); ++opi) {
droption_parser_t *op = *opi;
op->clear_value();
}
}

protected:
virtual bool
option_takes_arg() const = 0;
Expand All @@ -357,6 +373,8 @@ class droption_parser_t {
clamp_value() = 0;
virtual std::string
default_as_string() const = 0;
virtual void
clear_value() = 0;

// To avoid static initializer ordering problems we use a function:
static std::vector<droption_parser_t *> &
Expand Down Expand Up @@ -512,6 +530,14 @@ template <typename T> class droption_t : public droption_parser_t {
return value_;
}

/** Resets the value of this option to the default value. */
void
clear_value() override
{
value_ = defval_;
is_specified_ = false;
}

/** Returns the separator of the option value
* (see #DROPTION_FLAG_ACCUMULATE).
*/
Expand Down

0 comments on commit 4570a68

Please sign in to comment.