From 4570a68da9b64c4fbedfad7aeede3ab7e5eac140 Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Thu, 10 Feb 2022 01:10:29 +0000 Subject: [PATCH] i#4139: Add method to clear droption values for static re-attach (#5335) 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 --- api/docs/release.dox | 2 ++ clients/drcachesim/tests/burst_malloc.cpp | 3 ++- .../tests/offline-burst_malloc.templatex | 17 +++++++++++- .../tests/offline-burst_reattach.templatex | 2 -- clients/drcachesim/tracer/tracer.cpp | 4 ++- ext/droption/droption.dox | 9 +++++++ ext/droption/droption.h | 26 +++++++++++++++++++ 7 files changed, 58 insertions(+), 5 deletions(-) diff --git a/api/docs/release.dox b/api/docs/release.dox index 68e7b134ce1..8a3b2187ef3 100644 --- a/api/docs/release.dox +++ b/api/docs/release.dox @@ -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: diff --git a/clients/drcachesim/tests/burst_malloc.cpp b/clients/drcachesim/tests/burst_malloc.cpp index c7a0563d4c4..b341a726b95 100644 --- a/clients/drcachesim/tests/burst_malloc.cpp +++ b/clients/drcachesim/tests/burst_malloc.cpp @@ -1,5 +1,5 @@ /* ********************************************************** - * Copyright (c) 2016-2020 Google, Inc. All rights reserved. + * Copyright (c) 2016-2022 Google, Inc. All rights reserved. * **********************************************************/ /* @@ -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"; diff --git a/clients/drcachesim/tests/offline-burst_malloc.templatex b/clients/drcachesim/tests/offline-burst_malloc.templatex index b68676da612..762f171d4f2 100644 --- a/clients/drcachesim/tests/offline-burst_malloc.templatex +++ b/clients/drcachesim/tests/offline-burst_malloc.templatex @@ -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: diff --git a/clients/drcachesim/tests/offline-burst_reattach.templatex b/clients/drcachesim/tests/offline-burst_reattach.templatex index bcf92ea73ae..e962ce38ec0 100644 --- a/clients/drcachesim/tests/offline-burst_reattach.templatex +++ b/clients/drcachesim/tests/offline-burst_reattach.templatex @@ -1,6 +1,4 @@ pre-DR init -Warning: duplicated function name .* -.* pre-DR start pre-DR detach .* diff --git a/clients/drcachesim/tracer/tracer.cpp b/clients/drcachesim/tracer/tracer.cpp index c97ed930b3d..23906ac7c89 100644 --- a/clients/drcachesim/tracer/tracer.cpp +++ b/clients/drcachesim/tracer/tracer.cpp @@ -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. * ******************************************************************************/ @@ -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 diff --git a/ext/droption/droption.dox b/ext/droption/droption.dox index 94e579a6394..b001cc293d6 100644 --- a/ext/droption/droption.dox +++ b/ext/droption/droption.dox @@ -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 @@ -190,4 +191,12 @@ droption_t 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. + */ diff --git a/ext/droption/droption.h b/ext/droption/droption.h index 17905e14d5d..b47e87e474e 100644 --- a/ext/droption/droption.h +++ b/ext/droption/droption.h @@ -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::iterator opi = allops().begin(); + opi != allops().end(); ++opi) { + droption_parser_t *op = *opi; + op->clear_value(); + } + } + protected: virtual bool option_takes_arg() const = 0; @@ -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 & @@ -512,6 +530,14 @@ template 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). */