PR11005: @defined part 2: clarify/standardize internal handling of unresolvable $variables

* staptree.cxx (target_symbol::chain): New function.
* staptree.h: Declare it.
* (*): Use it instead of hand-chaining to target_symbol->saved_conversion_error.
* tapset-mark.cxx (*::visit_target_symbol): Chain resolution error object,
  do not throw.
* tapset-procfs.cxx (*::visit_target_symbol): Ditto.
* tapset-utrace.cxx (*::visit_target_symbol): Ditto.
* tapsets.cxx (*::visit_target_symbol): Ditto.
  (*::visit_defined_op): Explain & enforce the above.
* testsuite/semok/thirtysix.stp: Expand.

* testsuite/systemtap.base/sdt_misc.exp: Bonus fix: make work with blddir != srcdir.

8 files changed, 400 insertions, 357 deletions

diff --git a/staptree.cxx b/staptree.cxx
index b6ee3fdf..f01e895e 100644
--- a/staptree.cxx
+++ b/staptree.cxx
@@ -223,6 +223,14 @@ target_symbol::assert_no_components(const std::string& tapset)
 }
 
 
+void target_symbol::chain (semantic_error *e)
+{
+  assert (e->chain == 0);
+  e->chain = this->saved_conversion_error;
+  this->saved_conversion_error = e;
+}
+
+
 // ------------------------------------------------------------------------
 // parse tree printing
 
diff --git a/staptree.h b/staptree.h
index 62ce37fe..0ad6e9c2 100644
--- a/staptree.h
+++ b/staptree.h
@@ -255,8 +255,9 @@ struct target_symbol: public symbol
   std::string base_name;
   std::vector<component> components;
   std::string probe_context_var; // NB: this being set implies that target_symbol is *resolved*
-  semantic_error* saved_conversion_error;
+  semantic_error* saved_conversion_error; // hand-made linked list
   target_symbol(): addressof(false), saved_conversion_error (0) {}
+  void chain (semantic_error* e);
   void print (std::ostream& o) const;
   void visit (visitor* u);
   void visit_components (visitor* u);
diff --git a/tapset-mark.cxx b/tapset-mark.cxx
index 3fd6f11f..5c5fb0ce 100644
--- a/tapset-mark.cxx
+++ b/tapset-mark.cxx
@@ -182,21 +182,28 @@ mark_var_expanding_visitor::visit_target_symbol (target_symbol* e)
 {
   assert(e->base_name.size() > 0 && e->base_name[0] == '$');
 
-  if (e->addressof)
-    throw semantic_error("cannot take address of marker variable", e->tok);
-
-  if (e->base_name.substr(0,4) == "$arg")
-    visit_target_symbol_arg (e);
-  else if (e->base_name == "$format" || e->base_name == "$name" 
-           || e->base_name == "$$parms" || e->base_name == "$$vars")
-    visit_target_symbol_context (e);
-  else
-    throw semantic_error ("invalid target symbol for marker, $argN, $name, $format, $$parms or $$vars expected",
-			  e->tok);
+  try
+    {
+      if (e->addressof)
+        throw semantic_error("cannot take address of marker variable", e->tok);
+      
+      if (e->base_name.substr(0,4) == "$arg")
+        visit_target_symbol_arg (e);
+      else if (e->base_name == "$format" || e->base_name == "$name" 
+               || e->base_name == "$$parms" || e->base_name == "$$vars")
+        visit_target_symbol_context (e);
+      else
+        throw semantic_error ("invalid target symbol for marker, $argN, $name, $format, $$parms or $$vars expected",
+                              e->tok);
+    }
+  catch (const semantic_error &er)
+    {
+      e->chain (new semantic_error(er));
+      provide (e);
+    }
 }
 
 
-
 mark_derived_probe::mark_derived_probe (systemtap_session &s,
                                         const string& p_n,
                                         const string& p_f,
diff --git a/tapset-procfs.cxx b/tapset-procfs.cxx
index 1926582b..517a4a22 100644
--- a/tapset-procfs.cxx
+++ b/tapset-procfs.cxx
@@ -1,5 +1,5 @@
 // tapset for procfs
-// Copyright (C) 2005-2009 Red Hat Inc.
+// Copyright (C) 2005-2010 Red Hat Inc.
 // Copyright (C) 2005-2007 Intel Corporation.
 // Copyright (C) 2008 James.Bottomley@HansenPartnership.com
 //
@@ -355,100 +355,108 @@ procfs_var_expanding_visitor::procfs_var_expanding_visitor (systemtap_session& s
 void
 procfs_var_expanding_visitor::visit_target_symbol (target_symbol* e)
 {
-  assert(e->base_name.size() > 0 && e->base_name[0] == '$');
-
-  if (e->base_name != "$value")
-    throw semantic_error ("invalid target symbol for procfs probe, $value expected",
-                          e->tok);
-
-  e->assert_no_components("procfs");
-
-  bool lvalue = is_active_lvalue(e);
-  if (write_probe && lvalue)
-    throw semantic_error("procfs $value variable is read-only in a procfs write probe", e->tok);
+  try
+    {
+      assert(e->base_name.size() > 0 && e->base_name[0] == '$');
+
+      if (e->base_name != "$value")
+        throw semantic_error ("invalid target symbol for procfs probe, $value expected",
+                              e->tok);
+      
+      e->assert_no_components("procfs");
+      
+      bool lvalue = is_active_lvalue(e);
+      if (write_probe && lvalue)
+        throw semantic_error("procfs $value variable is read-only in a procfs write probe", e->tok);
   else if (! write_probe && ! lvalue)
     throw semantic_error("procfs $value variable cannot be read in a procfs read probe", e->tok);
-
-  if (e->addressof)
-    throw semantic_error("cannot take address of procfs variable", e->tok);
-
-  // Remember that we've seen a target variable.
-  target_symbol_seen = true;
-
-  // Synthesize a function.
-  functiondecl *fdecl = new functiondecl;
-  fdecl->tok = e->tok;
-  embeddedcode *ec = new embeddedcode;
-  ec->tok = e->tok;
-
-  string fname;
-  string locvalue = "CONTEXT->data";
-
-  if (! lvalue)
-    {
-      fname = "_procfs_value_get";
-      ec->code = string("    struct _stp_procfs_data *data = (struct _stp_procfs_data *)(") + locvalue + string("); /* pure */\n")
-	
-	  + string("    _stp_copy_from_user(THIS->__retvalue, data->buffer, data->count);\n")
-	  + string("    THIS->__retvalue[data->count] = '\\0';\n");
-    }
-  else					// lvalue
-    {
-      if (*op == "=")
+      
+      if (e->addressof)
+        throw semantic_error("cannot take address of procfs variable", e->tok);
+      
+      // Remember that we've seen a target variable.
+      target_symbol_seen = true;
+      
+      // Synthesize a function.
+      functiondecl *fdecl = new functiondecl;
+      fdecl->tok = e->tok;
+      embeddedcode *ec = new embeddedcode;
+      ec->tok = e->tok;
+      
+      string fname;
+      string locvalue = "CONTEXT->data";
+      
+      if (! lvalue)
         {
-	  fname = "_procfs_value_set";
-	  ec->code = string("struct _stp_procfs_data *data = (struct _stp_procfs_data *)(") + locvalue + string(");\n")
-	      + string("    strlcpy(data->buffer, THIS->value, MAXSTRINGLEN);\n")
-	      + string("    data->count = strlen(data->buffer);\n");
-	}
-      else if (*op == ".=")
+          fname = "_procfs_value_get";
+          ec->code = string("    struct _stp_procfs_data *data = (struct _stp_procfs_data *)(") + locvalue + string("); /* pure */\n")
+            
+            + string("    _stp_copy_from_user(THIS->__retvalue, data->buffer, data->count);\n")
+            + string("    THIS->__retvalue[data->count] = '\\0';\n");
+        }
+      else					// lvalue
         {
-	  fname = "_procfs_value_append";
-	  ec->code = string("struct _stp_procfs_data *data = (struct _stp_procfs_data *)(") + locvalue + string(");\n")
-	      + string("    strlcat(data->buffer, THIS->value, MAXSTRINGLEN);\n")
-	      + string("    data->count = strlen(data->buffer);\n");
-	}
-      else
+          if (*op == "=")
+            {
+              fname = "_procfs_value_set";
+              ec->code = string("struct _stp_procfs_data *data = (struct _stp_procfs_data *)(") + locvalue + string(");\n")
+                + string("    strlcpy(data->buffer, THIS->value, MAXSTRINGLEN);\n")
+                + string("    data->count = strlen(data->buffer);\n");
+            }
+          else if (*op == ".=")
+            {
+              fname = "_procfs_value_append";
+              ec->code = string("struct _stp_procfs_data *data = (struct _stp_procfs_data *)(") + locvalue + string(");\n")
+                + string("    strlcat(data->buffer, THIS->value, MAXSTRINGLEN);\n")
+                + string("    data->count = strlen(data->buffer);\n");
+            }
+          else
+            {
+              throw semantic_error ("Only the following assign operators are"
+                                    " implemented on procfs read target variables:"
+                                    " '=', '.='", e->tok);
+            }
+        }
+      
+      fdecl->name = fname;
+      fdecl->body = ec;
+      fdecl->type = pe_string;
+      
+      if (lvalue)
         {
-	  throw semantic_error ("Only the following assign operators are"
-				" implemented on procfs read target variables:"
-				" '=', '.='", e->tok);
-	}
-    }
-
-  fdecl->name = fname;
-  fdecl->body = ec;
-  fdecl->type = pe_string;
-
-  if (lvalue)
-    {
-      // Modify the fdecl so it carries a single pe_string formal
-      // argument called "value".
-
-      vardecl *v = new vardecl;
-      v->type = pe_string;
-      v->name = "value";
-      v->tok = e->tok;
-      fdecl->formal_args.push_back(v);
+          // Modify the fdecl so it carries a single pe_string formal
+          // argument called "value".
+          
+          vardecl *v = new vardecl;
+          v->type = pe_string;
+          v->name = "value";
+          v->tok = e->tok;
+          fdecl->formal_args.push_back(v);
+        }
+      sess.functions[fdecl->name]=fdecl;
+      
+      // Synthesize a functioncall.
+      functioncall* n = new functioncall;
+      n->tok = e->tok;
+      n->function = fname;
+      n->referent = 0; // NB: must not resolve yet, to ensure inclusion in session
+      
+      if (lvalue)
+        {
+          // Provide the functioncall to our parent, so that it can be
+          // used to substitute for the assignment node immediately above
+          // us.
+          assert(!target_symbol_setter_functioncalls.empty());
+          *(target_symbol_setter_functioncalls.top()) = n;
+        }
+      
+      provide (n);
     }
-  sess.functions[fdecl->name]=fdecl;
-
-  // Synthesize a functioncall.
-  functioncall* n = new functioncall;
-  n->tok = e->tok;
-  n->function = fname;
-  n->referent = 0; // NB: must not resolve yet, to ensure inclusion in session
-
-  if (lvalue)
+  catch (const semantic_error &er)
     {
-      // Provide the functioncall to our parent, so that it can be
-      // used to substitute for the assignment node immediately above
-      // us.
-      assert(!target_symbol_setter_functioncalls.empty());
-      *(target_symbol_setter_functioncalls.top()) = n;
+      e->chain (new semantic_error(er));
+      provide (e);
     }
-
-  provide (n);
 }
 
 
diff --git a/tapset-utrace.cxx b/tapset-utrace.cxx
index 4820ecf9..64433a24 100644
--- a/tapset-utrace.cxx
+++ b/tapset-utrace.cxx
@@ -582,20 +582,29 @@ utrace_var_expanding_visitor::visit_target_symbol (target_symbol* e)
 {
   assert(e->base_name.size() > 0 && e->base_name[0] == '$');
 
-  if (flags != UDPF_SYSCALL && flags != UDPF_SYSCALL_RETURN)
-    throw semantic_error ("only \"process(PATH_OR_PID).syscall\" and \"process(PATH_OR_PID).syscall.return\" probes support target symbols",
-			  e->tok);
-
-  if (e->addressof)
-    throw semantic_error("cannot take address of utrace variable", e->tok);
-
-  if (e->base_name.substr(0,4) == "$arg" || e->base_name == "$$parms")
-    visit_target_symbol_arg(e);
-  else if (e->base_name == "$syscall" || e->base_name == "$return")
-    visit_target_symbol_context(e);
-  else
-    throw semantic_error ("invalid target symbol for utrace probe, $syscall, $return, $argN or $$parms expected",
-			  e->tok);
+  try 
+    {
+      if (flags != UDPF_SYSCALL && flags != UDPF_SYSCALL_RETURN)
+        throw semantic_error ("only \"process(PATH_OR_PID).syscall\" and \"process(PATH_OR_PID).syscall.return\" probes support target symbols",
+                              e->tok);
+      
+      if (e->addressof)
+        throw semantic_error("cannot take address of utrace variable", e->tok);
+      
+      if (e->base_name.substr(0,4) == "$arg" || e->base_name == "$$parms")
+        visit_target_symbol_arg(e);
+      else if (e->base_name == "$syscall" || e->base_name == "$return")
+        visit_target_symbol_context(e);
+      else
+        throw semantic_error ("invalid target symbol for utrace probe, $syscall, $return, $argN or $$parms expected",
+                              e->tok);
+    }
+  catch (const semantic_error &er) 
+    {
+      e->chain (new semantic_error(er));
+      provide(e);
+      return;
+    }
 }
 
 
diff --git a/tapsets.cxx b/tapsets.cxx
index fcb22e03..46cc3a65 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -1949,10 +1949,20 @@ var_expanding_visitor::visit_defined_op (defined_op* e)
     expression *foo1 = e->operand;
     foo1 = require (foo1);
 
-    // NB: we have some curious cases to consider here, depending on what
+    // NB: Formerly, we had some curious cases to consider here, depending on what
     // various visit_target_symbol() implementations do for successful or
-    // erroneous resolutions.
+    // erroneous resolutions.  Some would signal a visit_target_symbol failure
+    // with an exception, with a set flag within the target_symbol, or nothing
+    // at all.
     //
+    // Now, failures always have to be signalled with a
+    // saved_conversion_error being chained to the target_symbol.
+    // Successes have to result in an attempted rewrite of the
+    // target_symbol (via provide()), or setting the probe_context_var
+    // (ugh).
+    // 
+    // Edna Mode: "no capes".  fche: "no exceptions".
+
     // dwarf stuff: success: rewrites to a function; failure: retains target_symbol, sets saved_conversion_error
     //
     // sdt-kprobes sdt.h: success: string or functioncall; failure: semantic_error
@@ -1967,16 +1977,16 @@ var_expanding_visitor::visit_defined_op (defined_op* e)
     // procfs: success: sets probe_context_var; failure: semantic_error
 
     target_symbol* foo2 = dynamic_cast<target_symbol*> (foo1);
-    if (foo2 && foo2->saved_conversion_error) // failing dwarf
+    if (foo2 && foo2->saved_conversion_error) // failing
       resolved = false;
-    else if (foo2 && foo2->probe_context_var != "") // successful procfs etc.
+    else if (foo2 && foo2->probe_context_var != "") // successful
       resolved = true;
-    else if (foo2) // unresolved but otherwise unremarked, for catching at translate time
-      resolved = false;
+    else if (foo2) // unresolved but not marked either way
+      assert (0); // should not happen
     else // resolved, rewritten to some other expression type
       resolved = true;
   } catch (const semantic_error& e) { 
-    resolved = false;
+    assert (0); // should not happen
   }
   defined_ops.pop ();
 
@@ -2437,56 +2447,55 @@ dwarf_var_expanding_visitor::visit_target_symbol (target_symbol *e)
 {
   assert(e->base_name.size() > 0 && e->base_name[0] == '$');
   visited = true;
-  
   bool defined_being_checked = (defined_ops.size() > 0 && (defined_ops.top()->operand == e));
   // In this mode, we avoid hiding errors or generating extra code such as for .return saved $vars
 
-  bool lvalue = is_active_lvalue(e);
-  if (lvalue && !q.sess.guru_mode)
-    throw semantic_error("write to target variable not permitted", e->tok);
-  // XXX: process $context vars should be writeable
-
-  // See if we need to generate a new probe to save/access function
-  // parameters from a return probe.  PR 1382.
-  if (q.has_return
-      && !defined_being_checked
-      && e->base_name != "$return" // not the special return-value variable handled below
-      && e->base_name != "$$return") // nor the other special variable handled below
-    {
-      if (lvalue)
-	throw semantic_error("write to target variable not permitted in .return probes", e->tok);
-
-      visit_target_symbol_saved_return(e);
-      return;
-    }
-
-  if (e->base_name == "$$vars"
-      || e->base_name == "$$parms"
-      || e->base_name == "$$locals"
-      || (q.has_return && (e->base_name == "$$return")))
+  try 
     {
-      if (lvalue)
-	throw semantic_error("cannot write to context variable", e->tok);
-
-      if (e->addressof)
-        throw semantic_error("cannot take address of context variable", e->tok);
+      bool lvalue = is_active_lvalue(e);
+      if (lvalue && !q.sess.guru_mode)
+        throw semantic_error("write to target variable not permitted", e->tok);
 
-      visit_target_symbol_context(e);
-      return;
-    }
-
-  // Synthesize a function.
-  functiondecl *fdecl = new functiondecl;
-  fdecl->tok = e->tok;
-  embeddedcode *ec = new embeddedcode;
-  ec->tok = e->tok;
-
-  string fname = (string(lvalue ? "_dwarf_tvar_set" : "_dwarf_tvar_get")
-		  + "_" + e->base_name.substr(1)
-		  + "_" + lex_cast(tick++));
+      // XXX: process $context vars should be writeable
+      
+      // See if we need to generate a new probe to save/access function
+      // parameters from a return probe.  PR 1382.
+      if (q.has_return
+          && !defined_being_checked
+          && e->base_name != "$return" // not the special return-value variable handled below
+          && e->base_name != "$$return") // nor the other special variable handled below
+        {
+          if (lvalue)
+            throw semantic_error("write to target variable not permitted in .return probes", e->tok);
+          visit_target_symbol_saved_return(e);
+          return;
+        }
 
-  try
-    {
+      if (e->base_name == "$$vars"
+          || e->base_name == "$$parms"
+          || e->base_name == "$$locals"
+          || (q.has_return && (e->base_name == "$$return")))
+        {
+          if (lvalue)
+            throw semantic_error("cannot write to context variable", e->tok);
+          
+          if (e->addressof)
+            throw semantic_error("cannot take address of context variable", e->tok);
+          
+          visit_target_symbol_context(e);
+          return;
+        }
+      
+      // Synthesize a function.
+      functiondecl *fdecl = new functiondecl;
+      fdecl->tok = e->tok;
+      embeddedcode *ec = new embeddedcode;
+      ec->tok = e->tok;
+      
+      string fname = (string(lvalue ? "_dwarf_tvar_set" : "_dwarf_tvar_get")
+                      + "_" + e->base_name.substr(1)
+                      + "_" + lex_cast(tick++));
+      
       // PR10601: adapt to kernel-vs-userspace loc2c-runtime
       ec->code += "\n#define fetch_register " + string(q.has_process?"u":"k") + "_fetch_register\n";
       ec->code += "#define store_register " + string(q.has_process?"u":"k") + "_store_register\n";
@@ -2517,12 +2526,66 @@ dwarf_var_expanding_visitor::visit_target_symbol (target_symbol *e)
       // PR10601
       ec->code += "\n#undef fetch_register\n";
       ec->code += "\n#undef store_register\n";
+
+      fdecl->name = fname;
+      fdecl->body = ec;
+      
+      // Any non-literal indexes need to be passed in too.
+      for (unsigned i = 0; i < e->components.size(); ++i)
+        if (e->components[i].type == target_symbol::comp_expression_array_index)
+          {
+            vardecl *v = new vardecl;
+            v->type = pe_long;
+            v->name = "index" + lex_cast(i);
+            v->tok = e->tok;
+            fdecl->formal_args.push_back(v);
+          }
+      
+      if (lvalue)
+        {
+          // Modify the fdecl so it carries a single pe_long formal
+          // argument called "value".
+          
+          // FIXME: For the time being we only support setting target
+          // variables which have base types; these are 'pe_long' in
+          // stap's type vocabulary.  Strings and pointers might be
+          // reasonable, some day, but not today.
+          
+          vardecl *v = new vardecl;
+          v->type = pe_long;
+          v->name = "value";
+          v->tok = e->tok;
+          fdecl->formal_args.push_back(v);
+        }
+      q.sess.functions[fdecl->name]=fdecl;
+      
+      // Synthesize a functioncall.
+      functioncall* n = new functioncall;
+      n->tok = e->tok;
+      n->function = fname;
+      n->referent = 0;  // NB: must not resolve yet, to ensure inclusion in session
+      
+      // Any non-literal indexes need to be passed in too.
+      for (unsigned i = 0; i < e->components.size(); ++i)
+        if (e->components[i].type == target_symbol::comp_expression_array_index)
+          n->args.push_back(require(e->components[i].expr_index));
+      
+      if (lvalue)
+        {
+          // Provide the functioncall to our parent, so that it can be
+          // used to substitute for the assignment node immediately above
+          // us.
+          assert(!target_symbol_setter_functioncalls.empty());
+          *(target_symbol_setter_functioncalls.top()) = n;
+        }
+      
+      provide (n);
     }
   catch (const semantic_error& er)
     {
       // NB: with --skip-badvars, @defined() still wins in that it may expand to 0
       // for nonexistent $variables.
-      if (!q.sess.skip_badvars || defined_being_checked)
+      if (!q.sess.skip_badvars)
 	{
 	  // We suppress this error message, and pass the unresolved
 	  // target_symbol to the next pass.  We hope that this value ends
@@ -2534,8 +2597,7 @@ dwarf_var_expanding_visitor::visit_target_symbol (target_symbol *e)
 	  // NB: we can have multiple errors, since a $target variable
 	  // may be expanded in several different contexts:
 	  //     function ("*") { $var }
-	  saveme->chain = e->saved_conversion_error;
-	  e->saved_conversion_error = saveme;
+          e->chain (saveme);
 	}
       else 
 	{
@@ -2548,64 +2610,8 @@ dwarf_var_expanding_visitor::visit_target_symbol (target_symbol *e)
             q.sess.print_warning ("Bad $context variable being substituted with literal 0",
                                   e->tok);
 	}
-      delete fdecl;
-      delete ec;
       return;
     }
-
-  fdecl->name = fname;
-  fdecl->body = ec;
-
-  // Any non-literal indexes need to be passed in too.
-  for (unsigned i = 0; i < e->components.size(); ++i)
-    if (e->components[i].type == target_symbol::comp_expression_array_index)
-      {
-        vardecl *v = new vardecl;
-        v->type = pe_long;
-        v->name = "index" + lex_cast(i);
-        v->tok = e->tok;
-        fdecl->formal_args.push_back(v);
-      }
-
-  if (lvalue)
-    {
-      // Modify the fdecl so it carries a single pe_long formal
-      // argument called "value".
-
-      // FIXME: For the time being we only support setting target
-      // variables which have base types; these are 'pe_long' in
-      // stap's type vocabulary.  Strings and pointers might be
-      // reasonable, some day, but not today.
-
-      vardecl *v = new vardecl;
-      v->type = pe_long;
-      v->name = "value";
-      v->tok = e->tok;
-      fdecl->formal_args.push_back(v);
-    }
-  q.sess.functions[fdecl->name]=fdecl;
-
-  // Synthesize a functioncall.
-  functioncall* n = new functioncall;
-  n->tok = e->tok;
-  n->function = fname;
-  n->referent = 0;  // NB: must not resolve yet, to ensure inclusion in session
-
-  // Any non-literal indexes need to be passed in too.
-  for (unsigned i = 0; i < e->components.size(); ++i)
-    if (e->components[i].type == target_symbol::comp_expression_array_index)
-      n->args.push_back(require(e->components[i].expr_index));
-
-  if (lvalue)
-    {
-      // Provide the functioncall to our parent, so that it can be
-      // used to substitute for the assignment node immediately above
-      // us.
-      assert(!target_symbol_setter_functioncalls.empty());
-      *(target_symbol_setter_functioncalls.top()) = n;
-    }
-
-  provide (n);
 }
 
 
@@ -2701,9 +2707,7 @@ dwarf_cast_query::handle_query_cu(Dwarf_Die * cudie)
 	  // NB: we can have multiple errors, since a @cast
 	  // may be expanded in several different contexts:
 	  //     function ("*") { @cast(...) }
-	  semantic_error* new_er = new semantic_error(er);
-	  new_er->chain = e.saved_conversion_error;
-	  e.saved_conversion_error = new_er;
+          e.chain (new semantic_error(er));
         }
       return DWARF_CB_ABORT;
     }
@@ -3725,90 +3729,98 @@ struct sdt_var_expanding_visitor: public var_expanding_visitor
 void
 sdt_var_expanding_visitor::visit_target_symbol (target_symbol *e)
 {
-  if (e->base_name == "$$name")
-    {
-      if (e->addressof)
-        throw semantic_error("cannot take address of sdt variable", e->tok);
-
-      literal_string *myname = new literal_string (probe_name);
-      myname->tok = e->tok;
-      provide(myname);
-      return;
-    }
-
-  if (e->base_name.find("$arg") == string::npos || ! have_reg_args)
-    {
-      // NB: uprobes-based sdt.h; $argFOO gets resolved later.
-      // XXX: We don't even know the arg_count in this case.
-      provide(e);
-      return;
-    }
-
-  int argno = 0;
-  try
-    {
-      argno = lex_cast<int>(e->base_name.substr(4));
-    }
-  catch (const runtime_error& f) // non-integral $arg suffix: e.g. $argKKKSDF
+  try 
     {
-      throw semantic_error ("invalid argument number", e->tok);
-    }
-  if (argno < 1 || argno > arg_count)
-    throw semantic_error ("invalid argument number", e->tok);
-
-  bool lvalue = is_active_lvalue(e);
-  functioncall *fc = new functioncall;
+      if (e->base_name == "$$name")
+        {
+          if (e->addressof)
+            throw semantic_error("cannot take address of sdt context variable", e->tok);
+          
+          literal_string *myname = new literal_string (probe_name);
+          myname->tok = e->tok;
+          provide(myname);
+          return;
+        }
 
-  // First two args are hidden: 1. pointer to probe name 2. task id
-  if (arg_count < 2)
-    {
-      fc->function = "ulong_arg";
-      fc->type = pe_long;
-      fc->tok = e->tok;
-      literal_number* num = new literal_number(argno + 2);
-      num->tok = e->tok;
-      fc->args.push_back(num);
-    }
-  else				// args passed as a struct
-    {
-      fc->function = "user_long";
-      fc->tok = e->tok;
-      binary_expression *be = new binary_expression;
-      be->tok = e->tok;
-      functioncall *get_arg1 = new functioncall;
-      get_arg1->function = "pointer_arg";
-      get_arg1->tok = e->tok;
-      literal_number* num = new literal_number(3);
-      num->tok = e->tok;
-      get_arg1->args.push_back(num);
-
-      be->left = get_arg1;
-      be->op = "+";
-      literal_number* inc = new literal_number((argno - 1) * 8);
-      be->right = inc;
-      fc->args.push_back(be);
+      if (e->base_name.find("$arg") == string::npos || ! have_reg_args)
+        {
+          // NB: uprobes-based sdt.h; $argFOO gets resolved later.
+          // XXX: We don't even know the arg_count in this case.
+          provide(e);
+          return;
+        }
+      
+      int argno = 0;
+      try
+        {
+          argno = lex_cast<int>(e->base_name.substr(4));
+        }
+      catch (const runtime_error& f) // non-integral $arg suffix: e.g. $argKKKSDF
+        {
+          throw semantic_error("invalid argument number", e->tok);
+        }
+      if (argno < 1 || argno > arg_count)
+        throw semantic_error("invalid argument number", e->tok);
+      
+      bool lvalue = is_active_lvalue(e);
+      functioncall *fc = new functioncall;
+      
+      // First two args are hidden: 1. pointer to probe name 2. task id
+      if (arg_count < 2)
+        {
+          fc->function = "ulong_arg";
+          fc->type = pe_long;
+          fc->tok = e->tok;
+          literal_number* num = new literal_number(argno + 2);
+          num->tok = e->tok;
+          fc->args.push_back(num);
+        }
+      else				// args passed as a struct
+        {
+          fc->function = "user_long";
+          fc->tok = e->tok;
+          binary_expression *be = new binary_expression;
+          be->tok = e->tok;
+          functioncall *get_arg1 = new functioncall;
+          get_arg1->function = "pointer_arg";
+          get_arg1->tok = e->tok;
+          literal_number* num = new literal_number(3);
+          num->tok = e->tok;
+          get_arg1->args.push_back(num);
+          
+          be->left = get_arg1;
+          be->op = "+";
+          literal_number* inc = new literal_number((argno - 1) * 8);
+          be->right = inc;
+          fc->args.push_back(be);
+        }
+      
+      if (lvalue)
+        *(target_symbol_setter_functioncalls.top()) = fc;
+      
+      if (e->components.empty())
+        {
+          if (e->addressof)
+            throw semantic_error("cannot take address of sdt variable", e->tok);
+          
+          provide(fc);
+          return;
+        }
+      cast_op *cast = new cast_op;
+      cast->base_name = "@cast";
+      cast->tok = e->tok;
+      cast->operand = fc;
+      cast->components = e->components;
+      cast->type = probe_name + "_arg" + lex_cast(argno);
+      cast->module = process_name;
+      
+      cast->visit(this);
     }
-  
-  if (lvalue)
-    *(target_symbol_setter_functioncalls.top()) = fc;
-
-  if (e->components.empty())
+  catch (const semantic_error &er)
     {
-      if (e->addressof)
-        throw semantic_error("cannot take address of sdt variable", e->tok);
-
-      provide(fc);
-      return;
+      e->chain (new semantic_error(er));
+      provide (e);
     }
-  cast_op *cast = new cast_op;
-  cast->base_name = "@cast";
-  cast->tok = e->tok;
-  cast->operand = fc;
-  cast->components = e->components;
-  cast->type = probe_name + "_arg" + lex_cast(argno);
-  cast->module = process_name;
-
-  cast->visit(this);
 }
 
 
@@ -5878,16 +5890,11 @@ tracepoint_var_expanding_visitor::visit_target_symbol_arg (target_symbol* e)
 
       // 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("unable to find tracepoint variable '" + e->base_name
+      throw semantic_error("unable to find tracepoint variable '" + e->base_name
                            + "' (alternatives:" + alternatives.str () + ")", e->tok);
       // NB: we can have multiple errors, since a target variable
       // may be expanded in several different contexts:
       //     trace ("*") { $foo->bar }
-      saveme->chain = e->saved_conversion_error;
-      e->saved_conversion_error = saveme;
-      provide (e);
-      return;
     }
 
   // make sure we're not dereferencing base types
@@ -5899,6 +5906,7 @@ tracepoint_var_expanding_visitor::visit_target_symbol_arg (target_symbol* e)
   if (lvalue && (!dw.sess.guru_mode || e->components.empty()))
     throw semantic_error("write to tracepoint variable '" + e->base_name
                          + "' not permitted", e->tok);
+
   // XXX: if a struct/union arg is passed by value, then writing to its fields
   // is also meaningless until you dereference past a pointer member.  It's
   // harder to detect and prevent that though...
@@ -5907,7 +5915,7 @@ tracepoint_var_expanding_visitor::visit_target_symbol_arg (target_symbol* e)
     {
       if (e->addressof)
         throw semantic_error("cannot take address of tracepoint variable", e->tok);
-
+      
       // Just grab the value from the probe locals
       e->probe_context_var = "__tracepoint_arg_" + arg->name;
       e->type = pe_long;
@@ -5932,27 +5940,9 @@ tracepoint_var_expanding_visitor::visit_target_symbol_arg (target_symbol* e)
       // PR10601: adapt to kernel-vs-userspace loc2c-runtime
       ec->code += "\n#define fetch_register k_fetch_register\n";
       ec->code += "#define store_register k_store_register\n";
-     
-      try
-        {
-          ec->code += dw.literal_stmt_for_pointer (&arg->type_die, e,
+
+      ec->code += dw.literal_stmt_for_pointer (&arg->type_die, e,
                                                   lvalue, fdecl->type);
-        }
-      catch (const semantic_error& er)
-        {
-          // We suppress this error message, and pass the unresolved
-          // variable 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
-          // NB: we can have multiple errors, since a target variable
-          // may be expanded in several different contexts:
-          //     trace ("*") { $foo->bar }
-          saveme->chain = e->saved_conversion_error;
-          e->saved_conversion_error = saveme;
-          provide (e);
-          return;
-        }
 
       // Give the fdecl an argument for the raw tracepoint value
       vardecl *v1 = new vardecl;
@@ -6091,14 +6081,22 @@ tracepoint_var_expanding_visitor::visit_target_symbol_context (target_symbol* e)
 void
 tracepoint_var_expanding_visitor::visit_target_symbol (target_symbol* e)
 {
-  assert(e->base_name.size() > 0 && e->base_name[0] == '$');
-
-  if (e->base_name == "$$name" ||
-      e->base_name == "$$parms" ||
-      e->base_name == "$$vars")
-    visit_target_symbol_context (e);
-  else
-    visit_target_symbol_arg (e);
+  try 
+    {
+      assert(e->base_name.size() > 0 && e->base_name[0] == '$');
+      
+      if (e->base_name == "$$name" ||
+          e->base_name == "$$parms" ||
+          e->base_name == "$$vars")
+        visit_target_symbol_context (e);
+      else
+        visit_target_symbol_arg (e);
+    }
+  catch (const semantic_error &er)
+    {
+      e->chain (new semantic_error(er));
+      provide (e);
+    }
 }
 
 
diff --git a/testsuite/semok/thirtysix.stp b/testsuite/semok/thirtysix.stp
index 4f27d9bf..ee19852c 100755
--- a/testsuite/semok/thirtysix.stp
+++ b/testsuite/semok/thirtysix.stp
@@ -1,19 +1,31 @@
 #! stap -p4
 
-probe kernel.function("sys_open") { println(@defined($foobar))  } # invalid
-probe kernel.function("sys_open") { println(@defined($mode))  }  # valid
-probe kernel.function("sys_open").return { println(@defined($foobar))  } # invalid
-probe kernel.function("sys_open").return { println(@defined($mode))  } # valid
-probe kernel.trace("*")? { println(@defined($rw)) } # valid and invalid in places
-probe kernel.mark("*")? { println(@defined($foo)) } # invalid
-probe kernel.mark("*")? { println(@defined($name)) } # valid
-probe process("stap").mark("*")? { println(@defined($arg1)) } # valid
-probe process("stap").mark("*")? { println(@defined($nosuchvar)) } # invalid
-probe procfs("file").read { println(@defined($nosuchvar)) } # invalid
-probe procfs("file").write { println(@defined($value)) } # valid
+probe kernel.function("sys_open") { println(@defined($foobar)?$foobar:0)  } # invalid
+probe kernel.function("sys_open") { println(@defined($mode)?$mode:0)  }  # valid
+
+/*
+There is a problem with saved $target vars for retprobes.  The synthetic entry probe
+to collect $foobar is generated before pass-2's constant-folding can eliminate the
+$foobar in the middle operand of ? : 
+*/
+/*
+probe kernel.function("sys_open").return { println(@defined($foobar)?$foobar:0)  } # invalid
+*/
+probe kernel.function("sys_open").return { println(@defined($foobar)?1:0)  } # invalid
+
+probe kernel.function("sys_open").return { println(@defined($mode)?$mode:0)  } # valid
+probe kernel.trace("*")? { println(@defined($rw)?$rw:0) } # valid and invalid in places
+probe kernel.mark("*")? { println(@defined($foo)?$foo:0) } # invalid
+probe kernel.mark("*")? { println(@defined($name)?$name:0) } # valid
+probe kernel.mark("*")? { println(@defined($argJJJ)?$argJJJ:0) }
+probe process("stap").mark("*")? { println(@defined($arg1)?$arg1:0) } # valid
+probe process("stap").mark("*")? { println(@defined($nosuchvar)?$nosuchvar:0) } # invalid
+probe procfs("file").read { println(@defined($nosuchvar)?$nosuchvar:0) } # invalid
+probe procfs("file").write { println(@defined($value)?$value:0) } # valid
 %( CONFIG_UTRACE == "y" %?
-probe process("stap").syscall { println(@defined($arg1)) } # valid
-probe process("stap").syscall { println(@defined($argZZ)) } # invalid
-probe process("stap").syscall.return { println(@defined($nosuchvar)) } # invalid
-probe process("stap").syscall.return { println(@defined($syscall)) } # valid
+probe process("stap").syscall { println(@defined($arg1)?$arg1:0) } # valid
+probe process("stap").begin { println(@defined($arg7)?$arg7:0) } # invalid
+probe process("stap").syscall { println(@defined($argZZ)?$argZZ:0) } # invalid
+probe process("stap").syscall.return { println(@defined($nosuchvar)?$nosuchvar:0) } # invalid
+probe process("stap").syscall.return { println(@defined($syscall)?$syscall:0) } # valid
 %)
diff --git a/testsuite/systemtap.base/sdt_misc.exp b/testsuite/systemtap.base/sdt_misc.exp
index 920cdd97..a7ebf215 100644
--- a/testsuite/systemtap.base/sdt_misc.exp
+++ b/testsuite/systemtap.base/sdt_misc.exp
@@ -170,7 +170,7 @@ close $fp
 if {[installtest_p]} { 
     set dtrace $env(SYSTEMTAP_PATH)/dtrace 
 } else {
-    set dtrace $srcdir/../dtrace
+    set dtrace ../dtrace
 }
 if {[catch {exec $dtrace --types -h -s $sup_dpath} res]} {
     verbose -log "unable to run $dtrace: $res"