PR11346: Move the skip-badvars logic into const-folding

The const_folder is where we prune constant expressions, like
conditionals based on a @defined($foo), so we want to give this a chance
to work before skip-badvars comes in and throws a warning.

As an added bonus, this makes skip-badvars more generic, so it will work
even for non-dwarf $target variables.

* elaborate.cxx (const_folder::visit_target_symbol): Enact skip-badvars.
* tapsets.cxx (dwarf_var_expanding_visitor::visit_target_symbol): Don't
  worry about badvars here anymore.  Save the error now, clean up later.
* testsuite/semok/badvar_undefined.stp: Test that @defined with
  skip-badvars doesn't throw any warnings.

3 files changed, 42 insertions, 28 deletions

diff --git a/elaborate.cxx b/elaborate.cxx
index db8abfbb..717192f2 100644
--- a/elaborate.cxx
+++ b/elaborate.cxx
@@ -3067,6 +3067,7 @@ struct const_folder: public update_visitor
   void visit_concatenation (concatenation* e);
   void visit_ternary_expression (ternary_expression* e);
   void visit_defined_op (defined_op* e);
+  void visit_target_symbol (target_symbol* e);
 };
 
 void
@@ -3542,6 +3543,29 @@ const_folder::visit_defined_op (defined_op* e)
   n->visit (this);
 }
 
+void
+const_folder::visit_target_symbol (target_symbol* e)
+{
+  if (e->probe_context_var.empty() && session.skip_badvars)
+    {
+      // Upon user request for ignoring context, the symbol is replaced
+      // with a literal 0 and a warning message displayed
+      // XXX this ignores possible side-effects, e.g. in array indexes
+      literal_number* ln_zero = new literal_number (0);
+      ln_zero->tok = e->tok;
+      provide (ln_zero);
+      if (!session.suppress_warnings)
+        session.print_warning ("Bad $context variable being substituted with literal 0",
+                               e->tok);
+      else if (session.verbose > 2)
+        clog << "Bad $context variable being substituted with literal 0, "
+             << *e->tok << endl;
+      relaxed_p = false;
+    }
+  else
+    update_visitor::visit_target_symbol (e);
+}
+
 static void semantic_pass_const_fold (systemtap_session& s, bool& relaxed_p)
 {
   // Let's simplify statements with constant values.
diff --git a/tapsets.cxx b/tapsets.cxx
index 4b6a3781..886c2de5 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -2613,34 +2613,17 @@ dwarf_var_expanding_visitor::visit_target_symbol (target_symbol *e)
     }
   catch (const semantic_error& er)
     {
-      // NB: @defined() should maintain the same behavior whether or not
-      // --skip-badvars is enabled, so don't zero its errors.  PR11346
-      if (!q.sess.skip_badvars || defined_being_checked)
-	{
-	  // We suppress this error message, and pass the unresolved
-	  // target_symbol to the next pass.  We hope that this value ends
-	  // up not being referenced after all, so it can be optimized out
-	  // quietly.
-	  provide (e);
-	  semantic_error* saveme = new semantic_error (er); // copy it
-          if (! saveme->tok1) { saveme->tok1 = e->tok; } // fill in token if needed
-	  // NB: we can have multiple errors, since a $target variable
-	  // may be expanded in several different contexts:
-	  //     function ("*") { $var }
-          e->chain (saveme);
-	}
-      else
-	{
-	  // Upon user request for ignoring context, the symbol is replaced
-	  // with a literal 0 and a warning message displayed
-	  literal_number* ln_zero = new literal_number (0);
-	  ln_zero->tok = e->tok;
-	  provide (ln_zero);
-          if (!q.sess.suppress_warnings)
-            q.sess.print_warning ("Bad $context variable being substituted with literal 0",
-                                  e->tok);
-	}
-      return;
+      // We suppress this error message, and pass the unresolved
+      // target_symbol to the next pass.  We hope that this value ends
+      // up not being referenced after all, so it can be optimized out
+      // quietly.
+      semantic_error* saveme = new semantic_error (er); // copy it
+      if (! saveme->tok1) { saveme->tok1 = e->tok; } // fill in token if needed
+      // NB: we can have multiple errors, since a $target variable
+      // may be expanded in several different contexts:
+      //     function ("*") { $var }
+      e->chain (saveme);
+      provide (e);
     }
 }
 
diff --git a/testsuite/semok/badvar_undefined.stp b/testsuite/semok/badvar_undefined.stp
new file mode 100755
index 00000000..05023ef0
--- /dev/null
+++ b/testsuite/semok/badvar_undefined.stp
@@ -0,0 +1,7 @@
+#!/bin/sh
+
+# PR11346: We shouldn't even see a warning when a would-be skip-badvars
+# replacement is masked by a @defined condition.
+
+stap -W -p2 --skip-badvars -e 'probe kernel.function("sys_getxattr")
+{ printf("%s\n", user_string(@defined($pathname) ? $pathname : $path)) }'