From ac033e879332cb103f09821e063e756567be0c62 Mon Sep 17 00:00:00 2001 From: David Smith Date: Thu, 16 May 2013 11:51:00 -0500 Subject: [PATCH] Fix PR15110 by eliminating duplicate warnings for stapdyn. * runtime/dyninst/transport.c (__stp_d_t_eliminate_duplicate_warnings): New function. (_stp_dyninst_transport_thread_func): Added warning suppression and duplicate warnings elimination. * translate.cxx (c_unparser::emit_global_init_setters): Emit code to call stp_session_attribute_setter(). * stapdyn/mutator.cxx (mutator::init_session_attributes): New function. (mutator::run_module_init): Call init_session_attributes(). * stapdyn/mutator.h (mutator): Add init_session_attributes() prototype. * runtime/dyninst/session_attributes.h: New file. * runtime/dyninst/session_attributes.c: Ditto. * runtime/dyninst/runtime.h: Includes session_attributes.c. * runtime/dyninst/common_session_state.h: Add session attributes structure to 'struct stp_runtime_session'. (stp_session_attributes): New function. (stp_session_init): Add call to stp_session_attributes_init(). * runtime/dyninst/stapdyn.h: Update stp_global_setter comment to note that names starting with '@' are internal names. * stapdyn/dynutil.cxx: Correct spelling of 'stapdyn_suppress_warnings' variable name. * stapdyn/dynutil.h: Ditto. * stapdyn/stapdyn.cxx (main): Ditto. * testsuite/systemtap.base/warnings2.exp: Remove dyninst_kfails proc, since duplicate warnings are now eliminated in dyninst mode. --- runtime/dyninst/common_session_state.h | 13 ++++ runtime/dyninst/runtime.h | 2 + runtime/dyninst/session_attributes.c | 44 +++++++++++++ runtime/dyninst/session_attributes.h | 20 ++++++ runtime/dyninst/stapdyn.h | 5 +- runtime/dyninst/transport.c | 90 ++++++++++++++++++++++++-- stapdyn/dynutil.cxx | 4 +- stapdyn/dynutil.h | 2 +- stapdyn/mutator.cxx | 30 +++++++++ stapdyn/mutator.h | 3 + stapdyn/stapdyn.cxx | 2 +- testsuite/systemtap.base/warnings2.exp | 9 --- translate.cxx | 5 +- 13 files changed, 206 insertions(+), 23 deletions(-) create mode 100644 runtime/dyninst/session_attributes.c create mode 100644 runtime/dyninst/session_attributes.h diff --git a/runtime/dyninst/common_session_state.h b/runtime/dyninst/common_session_state.h index 43a404a02..a365662c7 100644 --- a/runtime/dyninst/common_session_state.h +++ b/runtime/dyninst/common_session_state.h @@ -6,6 +6,7 @@ static void *_stp_shm_base; static void *_stp_shm_alloc(size_t size); +#include "session_attributes.h" #include "transport.h" // Global state shared throughout the module @@ -21,6 +22,8 @@ struct stp_runtime_session { unsigned long _hash_seed; + struct _stp_session_attributes _session_attributes; + #ifdef STP_ALIBI atomic_t _probe_alibi[STP_PROBE_COUNT]; #endif @@ -76,6 +79,14 @@ static inline unsigned long _stap_hash_seed() } +static inline struct _stp_session_attributes *stp_session_attributes(void) +{ + if (likely(_stp_session())) + return &_stp_session()->_session_attributes; + return NULL; +} + + #ifdef STP_ALIBI static inline atomic_t *probe_alibi(size_t index) { @@ -184,6 +195,8 @@ static int stp_session_init(void) _stp_session()->_hash_seed = _stp_random_u ((unsigned long)-1); + stp_session_attributes_init(); + #ifdef STP_ALIBI // Initialize all the alibi counters for (i = 0; i < STP_PROBE_COUNT; ++i) diff --git a/runtime/dyninst/runtime.h b/runtime/dyninst/runtime.h index 2fc2cc79b..ca6434446 100644 --- a/runtime/dyninst/runtime.h +++ b/runtime/dyninst/runtime.h @@ -132,6 +132,7 @@ static int _stp_sched_getcpu(void) /* see common_session_state.h */ static inline struct _stp_transport_session_data *stp_transport_data(void); +static inline struct _stp_session_attributes *stp_session_attributes(void); /* * By definition, we can only debug our own processes with dyninst, so @@ -158,6 +159,7 @@ static inline struct _stp_transport_session_data *stp_transport_data(void); #include "addr-map.c" #include "stat.c" #include "unwind.c" +#include "session_attributes.c" /* Support function for int64_t module parameters. */ static int set_int64_t(const char *val, int64_t *mp) diff --git a/runtime/dyninst/session_attributes.c b/runtime/dyninst/session_attributes.c new file mode 100644 index 000000000..a51ef2a35 --- /dev/null +++ b/runtime/dyninst/session_attributes.c @@ -0,0 +1,44 @@ +// stapdyn session attribute code +// Copyright (C) 2013 Red Hat Inc. +// +// This file is part of systemtap, and is free software. You can +// redistribute it and/or modify it under the terms of the GNU General +// Public License (GPL); either version 2, or (at your option) any +// later version. + +#ifndef SESSION_ATTRIBUTES_C +#define SESSION_ATTRIBUTES_C + +#include "session_attributes.h" + +static struct _stp_session_attributes _stp_init_session_attributes = { + .log_level = 0, + .suppress_warnings = 0, +}; + +static void stp_session_attributes_init(void) +{ + stp_session_attributes()->log_level + = _stp_init_session_attributes.log_level; + stp_session_attributes()->suppress_warnings + = _stp_init_session_attributes.suppress_warnings; +} + +static int stp_session_attribute_setter(const char *name, const char *value) +{ + // Note that We start all internal variables with '@', since + // that can't start a "real" variable. + if (strcmp(name, "@log_level") == 0) { + _stp_init_session_attributes.log_level = strtoul(value, NULL, + 10); + return 0; + } + else if (strcmp(name, "@suppress_warnings") == 0) { + _stp_init_session_attributes.suppress_warnings + = strtoul(value, NULL, 10); + return 0; + } + return -EINVAL; +} + +#endif // SESSION_ATTRIBUTES_C diff --git a/runtime/dyninst/session_attributes.h b/runtime/dyninst/session_attributes.h new file mode 100644 index 000000000..b96e2455c --- /dev/null +++ b/runtime/dyninst/session_attributes.h @@ -0,0 +1,20 @@ +// stapdyn session attribute declarations +// Copyright (C) 2013 Red Hat Inc. +// +// This file is part of systemtap, and is free software. You can +// redistribute it and/or modify it under the terms of the GNU General +// Public License (GPL); either version 2, or (at your option) any +// later version. + +#ifndef SESSION_ATTRIBUTES_H +#define SESSION_ATTRIBUTES_H + +struct _stp_session_attributes { + unsigned long log_level; + unsigned long suppress_warnings; +}; + +static void stp_session_attributes_init(void); +static int stp_session_attribute_setter(const char *name, const char *value); + +#endif // SESSION_ATTRIBUTES_H diff --git a/runtime/dyninst/stapdyn.h b/runtime/dyninst/stapdyn.h index 32970a4f7..a215dd7d6 100644 --- a/runtime/dyninst/stapdyn.h +++ b/runtime/dyninst/stapdyn.h @@ -71,8 +71,9 @@ extern int stp_dyninst_shm_connect(const char* name); /**** STAP 2.2 : ****/ /* The following function is dynamically generated by systemtap, and - used by stapdyn to modify global variables at module startup only - (that is, *before* running stp_dyninst_session_init()): */ + * used by stapdyn to modify global variables at module startup only + * (that is, *before* running stp_dyninst_session_init()). If the + * name starts with '@', the name is assumed to be an internal value. */ extern int stp_global_setter(const char *name, const char *value); diff --git a/runtime/dyninst/transport.c b/runtime/dyninst/transport.c index d4905ff89..f810ef12c 100644 --- a/runtime/dyninst/transport.c +++ b/runtime/dyninst/transport.c @@ -19,6 +19,7 @@ #include #include +#include #include "transport.h" @@ -179,6 +180,61 @@ __stp_dyninst_transport_queue_add(unsigned type, int data_index, pthread_mutex_unlock(&(sess_data->queue_mutex)); } +/* Handle duplicate warning elimination. Returns 0 if we've seen this + * warning (and should be eliminated), 1 otherwise. */ +static int +__stp_d_t_eliminate_duplicate_warnings(char *data, size_t bytes) +{ + static void *seen = 0; + static unsigned seen_count = 0; + char *dupstr = strndup (data, bytes); + char *retval; + int rc = 1; + + if (! dupstr) { + /* OOM, should not happen. */ + return 1; + } + + retval = tfind (dupstr, &seen, + (int (*)(const void*, const void*))strcmp); + if (! retval) { /* new message */ + /* We set a maximum for stored warning messages, to + * prevent a misbehaving script/environment from + * emitting countless _stp_warn()s, and overflow + * staprun's memory. */ +#define MAX_STORED_WARNINGS 1024 + if (seen_count++ == MAX_STORED_WARNINGS) { + fprintf(_stp_err, + "WARNING deduplication table full\n"); + free (dupstr); + } + else if (seen_count > MAX_STORED_WARNINGS) { + /* Be quiet in the future, but stop counting + * to preclude overflow. */ + free (dupstr); + seen_count = MAX_STORED_WARNINGS + 1; + } + else if (seen_count < MAX_STORED_WARNINGS) { + /* NB: don't free dupstr; it's going into the tree. */ + retval = tsearch (dupstr, & seen, + (int (*)(const void*, const void*))strcmp); + if (retval == 0) { + /* OOM, should not happen. Next time + * we should get the 'full' + * message. */ + free (dupstr); + seen_count = MAX_STORED_WARNINGS; + } + } + } + else { /* old message */ + free (dupstr); + rc = 0; + } + return rc; +} + static void * _stp_dyninst_transport_thread_func(void *arg __attribute((unused))) { @@ -226,6 +282,7 @@ _stp_dyninst_transport_thread_func(void *arg __attribute((unused))) // Process the queue twice. First handle the OOB data. for (size_t i = 0; i < q->items; i++) { + int write_data = 1; item = &(q->queue[i]); if (item->type != STP_DYN_OOB_DATA) continue; @@ -239,16 +296,35 @@ _stp_dyninst_transport_thread_func(void *arg __attribute((unused))) c = stp_session_context(item->data_index); data = &c->transport_data; read_ptr = data->log_buf + item->offset; - size = write(err_fd, read_ptr, item->bytes); - if (size != item->bytes) - fprintf(_stp_err, - "write only wrote %ld bytes (%ld)," - " errno %d\n", - (long)size, (long)item->bytes, errno); - data->log_start = _STP_D_T_LOG_INC(data->log_start); + + /* Note that "WARNING:" should not be + * translated, since it is part of the module + * cmd protocol. */ + if (strncmp(read_ptr, "WARNING:", 7) == 0) { + if (stp_session_attributes()->suppress_warnings) { + write_data = 0; + } + /* If we're not verbose, eliminate + * duplicate warning messages. */ + else if (stp_session_attributes()->log_level + == 0) { + write_data = __stp_d_t_eliminate_duplicate_warnings(read_ptr, item->bytes); + } + } + + if (write_data) { + size = write(err_fd, read_ptr, item->bytes); + if (size != item->bytes) + fprintf(_stp_err, + "write only wrote %ld bytes" + " (%ld), errno %d\n", + (long)size, + (long)item->bytes, errno); + } // Signal there is a log buffer available to // any waiters. + data->log_start = _STP_D_T_LOG_INC(data->log_start); pthread_mutex_lock(&(data->log_mutex)); pthread_cond_signal(&(data->log_space_avail)); pthread_mutex_unlock(&(data->log_mutex)); diff --git a/stapdyn/dynutil.cxx b/stapdyn/dynutil.cxx index 393a14231..f6d36f728 100644 --- a/stapdyn/dynutil.cxx +++ b/stapdyn/dynutil.cxx @@ -178,7 +178,7 @@ static ostream nullstream(NULL); unsigned stapdyn_log_level = 0; // Whether to suppress warnings, set by -w -bool stapdyn_supress_warnings = false; +bool stapdyn_suppress_warnings = false; // Return a stream for logging at the given verbosity level. ostream& @@ -193,7 +193,7 @@ staplog(unsigned level) ostream& stapwarn(void) { - if (stapdyn_supress_warnings) + if (stapdyn_suppress_warnings) return nullstream; return clog << program_invocation_short_name << ": WARNING: "; } diff --git a/stapdyn/dynutil.h b/stapdyn/dynutil.h index dbb2abe8b..cc438411e 100644 --- a/stapdyn/dynutil.h +++ b/stapdyn/dynutil.h @@ -47,7 +47,7 @@ set_dlsym(T*& pointer, void* handle, const char* symbol, bool required=true) extern unsigned stapdyn_log_level; // Whether to suppress warnings, set by -w -extern bool stapdyn_supress_warnings; +extern bool stapdyn_suppress_warnings; // Return a stream for logging at the given verbosity level. std::ostream& staplog(unsigned level=0); diff --git a/stapdyn/mutator.cxx b/stapdyn/mutator.cxx index 2265cf8ff..682bb3c2b 100644 --- a/stapdyn/mutator.cxx +++ b/stapdyn/mutator.cxx @@ -321,6 +321,34 @@ mutator::init_modoptions() return true; } +void +mutator::init_session_attributes() +{ + typeof(&stp_global_setter) global_setter = NULL; + set_dlsym(global_setter, module, "stp_global_setter", false); + + if (global_setter == NULL) + { + // Just return. + return; + } + + // Note that the list of supported attributes should match with the + // list in 'struct _stp_sesion_attributes' in + // runtime/dyninst/session_attributes.h. + + int rc = global_setter("@log_level", lex_cast(stapdyn_log_level).c_str()); + if (rc != 0) + stapwarn() << "couldn't set 'log_level' global" << endl; + + rc = global_setter("@suppress_warnings", + lex_cast(stapdyn_suppress_warnings).c_str()); + if (rc != 0) + stapwarn() << "couldn't set 'suppress_warnings' global" << endl; + + return; +} + // Initialize the module session bool mutator::run_module_init() @@ -371,6 +399,8 @@ mutator::run_module_init() if (!modoptions.empty() && !init_modoptions()) return false; + init_session_attributes(); + int rc = session_init(); if (rc) { diff --git a/stapdyn/mutator.h b/stapdyn/mutator.h index 280c6080f..e4cade219 100644 --- a/stapdyn/mutator.h +++ b/stapdyn/mutator.h @@ -51,6 +51,9 @@ class mutator { // Initialize the module global variables bool init_modoptions(); + // Initialize the session attributes + void init_session_attributes(); + // Initialize the module session bool run_module_init(); diff --git a/stapdyn/stapdyn.cxx b/stapdyn/stapdyn.cxx index a3ae31412..063bb65e6 100644 --- a/stapdyn/stapdyn.cxx +++ b/stapdyn/stapdyn.cxx @@ -75,7 +75,7 @@ main(int argc, char * const argv[]) break; case 'w': - stapdyn_supress_warnings = true; + stapdyn_suppress_warnings = true; break; case 'V': diff --git a/testsuite/systemtap.base/warnings2.exp b/testsuite/systemtap.base/warnings2.exp index 961f7c396..156f693bc 100644 --- a/testsuite/systemtap.base/warnings2.exp +++ b/testsuite/systemtap.base/warnings2.exp @@ -1,18 +1,9 @@ if {! [installtest_p]} { return } -proc dyninst_kfails {index} { - # With no real transport layer, dyninst can't eliminate duplicate - # warnings. PR15110 - setup_kfail 15110 "*-*-*" -} - foreach runtime [get_runtime_list] { set test "warnings2 default" if {$runtime != ""} { lappend test "($runtime)" - if { [info procs ${runtime}_kfails] ne "" } { - ${runtime}_kfails $i - } spawn stap --runtime=$runtime \ -e {probe begin {warn ("1") warn ("2") warn ("1") exit ()}} } else { diff --git a/translate.cxx b/translate.cxx index 24edd5977..d8f9b08ac 100644 --- a/translate.cxx +++ b/translate.cxx @@ -1538,7 +1538,10 @@ c_unparser::emit_global_init_setters () o->newline(-1) << "} else "; } - o->line() << "return -EINVAL;"; + + // Call the runtime function that handles session attributes, like + // log_level, etc. + o->line() << "return stp_session_attribute_setter(name, value);"; o->newline(-1) << "}"; o->newline(); } -- 2.43.5