diff options
| -rw-r--r-- | ChangeLog | 9 | ||||
| -rw-r--r-- | elaborate.cxx | 32 | ||||
| -rw-r--r-- | staptree.cxx | 33 |
3 files changed, 68 insertions, 6 deletions
@@ -1,3 +1,12 @@ +2008-05-20 Frank Ch. Eigler <fche@elastic.org> + + PR 6538 + * elaborate.cxx (semantic_pass_opt2): Warn about read-only locals and + globals. + * staptree.cxx (varuse_collecting_visitor::visit_symbol): Recognize + initialized global as written-to. + (visit_foreach_loop): Belatedly recognize index symbols as lvalues. + 2008-05-20 Tim Moore <timoore@redhat.com> * configure.ac: Check for tr1/unordered_map header. diff --git a/elaborate.cxx b/elaborate.cxx index 4bc43832..f8bfabdb 100644 --- a/elaborate.cxx +++ b/elaborate.cxx @@ -1624,7 +1624,13 @@ void semantic_pass_opt2 (systemtap_session& s, bool& relaxed_p) // don't increment j } else - j++; + { + if (vut.written.find (l) == vut.written.end()) + if (! s.suppress_warnings) + clog << "WARNING: read-only local variable " << *l->tok << endl; + + j++; + } } for (unsigned i=0; i<s.functions.size(); i++) for (unsigned j=0; j<s.functions[i]->locals.size(); /* see below */) @@ -1649,7 +1655,12 @@ void semantic_pass_opt2 (systemtap_session& s, bool& relaxed_p) // don't increment j } else - j++; + { + if (vut.written.find (l) == vut.written.end()) + if (! s.suppress_warnings) + clog << "WARNING: read-only local variable " << *l->tok << endl; + j++; + } } for (unsigned i=0; i<s.globals.size(); /* see below */) { @@ -1671,7 +1682,12 @@ void semantic_pass_opt2 (systemtap_session& s, bool& relaxed_p) // don't increment i } else - i++; + { + if (vut.written.find (l) == vut.written.end()) + if (! s.suppress_warnings) + clog << "WARNING: read-only global variable " << *l->tok << endl; + i++; + } } } @@ -1834,6 +1850,8 @@ void semantic_pass_opt3 (systemtap_session& s, bool& relaxed_p) for (unsigned i=0; i<s.functions.size(); i++) s.functions[i]->body->visit (& dar); // The rewrite operation is performed within the visitor. + + // XXX: we could also zap write-only globals here } @@ -2057,6 +2075,8 @@ void semantic_pass_opt4 (systemtap_session& s, bool& relaxed_p) p->body = new null_statement(); p->body->tok = p->tok; + + // XXX: possible duplicate warnings; see below } } for (unsigned i=0; i<s.functions.size(); i++) @@ -2080,6 +2100,12 @@ void semantic_pass_opt4 (systemtap_session& s, bool& relaxed_p) fn->body = new null_statement(); fn->body->tok = fn->tok; + + // XXX: the next iteration of the outer optimization loop may + // take this new null_statement away again, and thus give us a + // fresh warning. It would be better if this fixup was performed + // only after the relaxation iterations. + // XXX: or else see bug #6469. } } } diff --git a/staptree.cxx b/staptree.cxx index 02a6c8dc..b5cbd5c9 100644 --- a/staptree.cxx +++ b/staptree.cxx @@ -1713,6 +1713,10 @@ varuse_collecting_visitor::visit_symbol (symbol *e) if (e->referent == 0) throw semantic_error ("symbol without referent", e->tok); + // handle initialized globals + if (e->referent->init) + written.insert (e->referent); + if (current_lvalue == e || current_lrvalue == e) { written.insert (e->referent); @@ -1789,10 +1793,19 @@ varuse_collecting_visitor::visit_post_crement (post_crement *e) void varuse_collecting_visitor::visit_foreach_loop (foreach_loop* s) { - functioncall_traversing_visitor::visit_foreach_loop (s); + // NB: we duplicate so don't bother call + // functioncall_traversing_visitor::visit_foreach_loop (s); + + symbol *array = NULL; + hist_op *hist = NULL; + classify_indexable (s->base, array, hist); + if (array) + array->visit(this); + else + hist->visit(this); + // If the collection is sorted, imply a "write" access to the - // array in addition to the "read" one already noted in the - // base class call above. + // array in addition to the "read" one already noted above. if (s->sort_direction) { symbol *array = NULL; @@ -1801,6 +1814,20 @@ varuse_collecting_visitor::visit_foreach_loop (foreach_loop* s) if (array) this->written.insert (array->referent); // XXX: Can hist_op iterations be sorted? } + + // NB: don't forget to visit the index expressions, which are lvalues. + for (unsigned i=0; i<s->indexes.size(); i++) + { + expression* last_lvalue = current_lvalue; + current_lvalue = s->indexes[i]; // leave a mark for ::visit_symbol + s->indexes[i]->visit (this); + current_lvalue = last_lvalue; + } + + if (s->limit) + s->limit->visit (this); + + s->block->visit (this); } |