2006-06-05 David Smith <dsmith@redhat.com>

	* tapsets.cxx (dwflpp::express_as_string): New function.
	Extracted from dwflpp::literal_stmt_for_local() so that
	dwflpp::literal_stmt_for_return() could also call it.
	(dwflpp::literal_stmt_for_local): Portion extracted to create
	dwflpp::express_as_string().
	(dwflpp::literal_stmt_for_return): New function.  Adds support for
	new symbolic access ("$return") to return value in .return
	probes.  Fixes PR 1132.
	(target_variable_flavour_calculating_visitor::visit_target_symbol):
	Calls dwflpp::literal_stmt_for_return() when in a return probe and
	the variable name is "$return".
	(dwarf_var_expanding_copy_visitor::visit_target_symbol): Ditto.
	* stapfuncs.5.in: Noted that the retval() function is deprecated.
	* stapprobes.5.in: Corrected the name of the return value
	variable.
	* tapset/return.stp: Marked the retval() function as deprecated.
	* testsuite/semko/return01.stp: Added new test.
	* testsuite/semko/return02.stp: Ditto.

7 files changed, 169 insertions, 40 deletions

diff --git a/ChangeLog b/ChangeLog
index f35abe25..016fe1b2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,24 @@
+2006-06-05  David Smith  <dsmith@redhat.com>
+
+	* tapsets.cxx (dwflpp::express_as_string): New function.
+	Extracted from dwflpp::literal_stmt_for_local() so that
+	dwflpp::literal_stmt_for_return() could also call it.
+	(dwflpp::literal_stmt_for_local): Portion extracted to create
+	dwflpp::express_as_string().
+	(dwflpp::literal_stmt_for_return): New function.  Adds support for
+	new symbolic access ("$return") to return value in .return
+	probes.  Fixes PR 1132.
+	(target_variable_flavour_calculating_visitor::visit_target_symbol):
+	Calls dwflpp::literal_stmt_for_return() when in a return probe and
+	the variable name is "$return".
+	(dwarf_var_expanding_copy_visitor::visit_target_symbol): Ditto.
+	* stapfuncs.5.in: Noted that the retval() function is deprecated.
+	* stapprobes.5.in: Corrected the name of the return value
+	variable.
+	* tapset/return.stp: Marked the retval() function as deprecated.
+	* testsuite/semko/return01.stp: Added new test.
+	* testsuite/semko/return02.stp: Ditto.
+
 2006-06-05  Frank Ch. Eigler  <fche@elastic.org>
 
 	PR 2645 cont'd.
diff --git a/stapfuncs.5.in b/stapfuncs.5.in
index 7a4cfafc..6406cf00 100644
--- a/stapfuncs.5.in
+++ b/stapfuncs.5.in
@@ -166,6 +166,7 @@ Return 1 if the probe point is a return probe.  Deprecated.
 .TP
 retval:long ()
 Return the pending return value of the function being return-probed.
+Deprecated by the "$return" target variable. 
 
 .SS ERRNO
 .TP
diff --git a/stapprobes.5.in b/stapprobes.5.in
index 0685ba2d..90f746aa 100644
--- a/stapprobes.5.in
+++ b/stapprobes.5.in
@@ -135,7 +135,7 @@ variant places a probe near the beginning of the named function, so that
 parameters are available as context variables.  The
 .B .return
 variant places a probe at the moment of return from the named function, so
-the return value is available as the "$retvalue" context variable. 
+the return value is available as the "$return" context variable. 
 The
 .B .inline
 variant is similar to 
diff --git a/tapset/return.stp b/tapset/return.stp
index 98ff8a73..fdf6e248 100644
--- a/tapset/return.stp
+++ b/tapset/return.stp
@@ -1,3 +1,5 @@
+/* DEPRECATED by the "$return" target variable. */
+
 function retval:long() %{ /* pure */
        if (CONTEXT->regs) {
 #if defined (__i386__)
diff --git a/tapsets.cxx b/tapsets.cxx
index 866c6757..92655070 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -1479,6 +1479,42 @@ dwflpp
       }
   }
 
+  string
+  express_as_string (string prelude,
+		     string postlude,
+		     struct location *head)
+  {
+    size_t bufsz = 1024;
+    char *buf = static_cast<char*>(malloc(bufsz));
+    assert(buf);
+
+    FILE *memstream = open_memstream (&buf, &bufsz);
+    assert(memstream);
+
+    fprintf(memstream, "{\n");
+    fprintf(memstream, prelude.c_str());
+    bool deref = c_emit_location (memstream, head, 1);
+    fprintf(memstream, postlude.c_str());
+    fprintf(memstream, "  goto out;\n");
+
+    // dummy use of deref_fault label, to disable warning if deref() not used
+    fprintf(memstream, "if (0) goto deref_fault;\n");
+
+    // XXX: deref flag not reliable; emit fault label unconditionally
+    // XXX: print the faulting address, like the user_string/kernel_string
+    // tapset functions do
+    if (deref) ;
+    fprintf(memstream,
+            "deref_fault:\n"
+            "  c->last_error = \"pointer dereference fault\";\n"
+            "  goto out;\n");
+    fprintf(memstream, "}\n");
+
+    fclose (memstream);
+    string result(buf);
+    free (buf);
+    return result;
+  }
 
   string
   literal_stmt_for_local (Dwarf_Die *scope_die,
@@ -1546,39 +1582,75 @@ dwflpp
 				    prelude, postlude, ty);
 
     /* Write the translation to a string. */
+    return express_as_string(prelude, postlude, head);
+  }
 
-    size_t bufsz = 1024;
-    char *buf = static_cast<char*>(malloc(bufsz));
-    assert(buf);
 
-    FILE *memstream = open_memstream (&buf, &bufsz);
-    assert(memstream);
+  string
+  literal_stmt_for_return (Dwarf_Die *scope_die,
+			   Dwarf_Addr pc,
+			   vector<pair<target_symbol::component_type,
+			   std::string> > const & components,
+			   bool lvalue,
+			   exp_type & ty)
+  {
+    if (sess.verbose>2)
+	clog << "literal_stmt_for_return: finding return value for "
+	     << dwarf_diename (scope_die)
+	     << "("
+	     << dwarf_diename (cu)
+	     << ")\n";
 
-    fprintf(memstream, "{\n");
-    fprintf(memstream, prelude.c_str());
-    bool deref = c_emit_location (memstream, head, 1);
-    fprintf(memstream, postlude.c_str());
-    fprintf(memstream, "  goto out;\n");
+    struct obstack pool;
+    obstack_init (&pool);
+    struct location *tail = NULL;
 
-    // dummy use of deref_fault label, to disable warning if deref() not used
-    fprintf(memstream, "if (0) goto deref_fault;\n");
+    /* Given $return->bar->baz[NN], translate the location of return. */
+    const Dwarf_Op *locops;
+    int nlocops = dwfl_module_return_value_location (module, scope_die,
+						     &locops);
+    if (nlocops < 0)
+      {
+	throw semantic_error("failed to retrieve return value location");
+      }
+    // the function has no return value (e.g. "void" in C)
+    else if (nlocops == 0)
+      {
+	throw semantic_error("function has no return value");
+      }
 
-    // XXX: deref flag not reliable; emit fault label unconditionally
-    // XXX: print the faulting address, like the user_string/kernel_string
-    // tapset functions do
-    if (deref) ;
-    fprintf(memstream,
-            "deref_fault:\n"
-            "  c->last_error = \"pointer dereference fault\";\n"
-            "  goto out;\n");
-    fprintf(memstream, "}\n");
+    struct location  *head = c_translate_location (&pool, &loc2c_error, this,
+						   &loc2c_emit_address,
+						   1, module_bias,
+						   pc, locops, nlocops,
+						   &tail, NULL);
 
-    fclose (memstream);
-    string result(buf);
-    free (buf);
-    return result;
-  }
+    /* Translate the ->bar->baz[NN] parts. */
 
+    Dwarf_Attribute attr_mem;
+    Dwarf_Attribute *attr = dwarf_attr (scope_die, DW_AT_type, &attr_mem);
+
+    Dwarf_Die vardie_mem;
+    Dwarf_Die *vardie = dwarf_formref_die (attr, &vardie_mem);
+
+    Dwarf_Die die_mem, *die = NULL;
+    die = translate_components (&pool, &tail, pc, components,
+				vardie, &die_mem, &attr_mem);
+
+    /* Translate the assignment part, either
+       x = $return->bar->baz[NN]
+       or
+       $return->bar->baz[NN] = x
+    */
+
+    string prelude, postlude;
+    translate_final_fetch_or_store (&pool, &tail, module_bias,
+				    die, &attr_mem, lvalue,
+				    prelude, postlude, ty);
+
+    /* Write the translation to a string. */
+    return express_as_string(prelude, postlude, head);
+  }
 
 
   ~dwflpp()
@@ -1996,12 +2068,19 @@ target_variable_flavour_calculating_visitor::visit_target_symbol (target_symbol
   string expr;
   try 
     {
-      expr = q.dw.literal_stmt_for_local(scope_die,
-                                         addr,
-                                         e->base_name.substr(1),
-                                         e->components,
-                                         lvalue,
-                                         ty);
+      if (q.has_return && e->base_name == "$return")
+	expr = q.dw.literal_stmt_for_return (scope_die,
+					     addr,
+					     e->components,
+					     lvalue,
+					     ty);
+      else
+	expr = q.dw.literal_stmt_for_local(scope_die,
+					   addr,
+					   e->base_name.substr(1),
+					   e->components,
+					   lvalue,
+					   ty);
     }
   catch (const semantic_error& e)
     {
@@ -2747,17 +2826,29 @@ dwarf_var_expanding_copy_visitor::visit_target_symbol (target_symbol *e)
 		  + "_" + e->base_name.substr(1)
 		  + "_" + lex_cast<string>(tick++));
 
-  if (q.has_return)
+  if (q.has_return && e->base_name != "$return")
     throw semantic_error ("target variables not available to .return probes");
 
   try
     {
-      ec->code = q.dw.literal_stmt_for_local (scope_die,
-					      addr,
-					      e->base_name.substr(1),
-					      e->components,
-					      lvalue,
-					      fdecl->type);
+      if (q.has_return && e->base_name == "$return")
+        {
+	  ec->code = q.dw.literal_stmt_for_return (scope_die,
+						   addr,
+						   e->components,
+						   lvalue,
+						   fdecl->type);
+	}
+      else
+        {
+	  ec->code = q.dw.literal_stmt_for_local (scope_die,
+						  addr,
+						  e->base_name.substr(1),
+						  e->components,
+						  lvalue,
+						  fdecl->type);
+	}
+
       if (! lvalue)
         ec->code += "/* pure */";
     }
diff --git a/testsuite/semko/return01.stp b/testsuite/semko/return01.stp
new file mode 100755
index 00000000..9e3c5f44
--- /dev/null
+++ b/testsuite/semko/return01.stp
@@ -0,0 +1,7 @@
+#! stap -p2
+
+probe kernel.function("get_page_state").return
+{
+    # this should fail, since get_page_state() is a void function
+    printf("return is %d\n", $return)
+}
diff --git a/testsuite/semko/return02.stp b/testsuite/semko/return02.stp
new file mode 100755
index 00000000..db4cd8c7
--- /dev/null
+++ b/testsuite/semko/return02.stp
@@ -0,0 +1,7 @@
+#! stap -p2
+
+probe kernel.function("sys_read")
+{
+    # this should fail - using $return not in a '.return' probe
+    printf("in sys_read - return = %d\n", $return)
+}