[PATCH] Add support for return statements without values

["Yichun Zhang (agentzh)" <agentzh@gmail.com>]

For user-defined stap functions which do not return any values, it can
be convenient to allow *bare* return statements to take shortcuts in
the control flow of the function body.

Added tests to cover various cases like use of plain `return` in a
function actually returning some values. Both the "kernel" and "dyninst"
runtimes are covered.

A fix for the bpf translator is also included.
---
 bpf-translate.cxx                            |   3 +-
 elaborate.cxx                                |   8 ++
 parse.cxx                                    |   7 +-
 staptree.cxx                                 |   8 +-
 testsuite/systemtap.base/return_no_val.exp   | 135 +++++++++++++++++++++++++++
 testsuite/systemtap.base/return_no_val_1.stp |  11 +++
 testsuite/systemtap.base/return_no_val_2.stp |  11 +++
 testsuite/systemtap.base/return_no_val_3.stp |  10 ++
 translate.cxx                                |  11 ++-
 9 files changed, 198 insertions(+), 6 deletions(-)
 create mode 100644 testsuite/systemtap.base/return_no_val.exp
 create mode 100644 testsuite/systemtap.base/return_no_val_1.stp
 create mode 100644 testsuite/systemtap.base/return_no_val_2.stp
 create mode 100644 testsuite/systemtap.base/return_no_val_3.stp

diff --git a/bpf-translate.cxx b/bpf-translate.cxx
index 95c68ce8d..26ceb9fcf 100644
--- a/bpf-translate.cxx
+++ b/bpf-translate.cxx
@@ -915,7 +915,8 @@ bpf_unparser::visit_return_statement (return_statement* s)
   if (func_return.empty ())
     throw SEMANTIC_ERROR (_("cannot 'return' outside function"), s->tok);
   assert (!func_return_val.empty ());
-  emit_mov (func_return_val.back (), emit_expr (s->value));
+  if (s->value)
+    emit_mov (func_return_val.back (), emit_expr (s->value));
   emit_jmp (func_return.back ());
 }
 
diff --git a/elaborate.cxx b/elaborate.cxx
index c55818f02..cd03df408 100644
--- a/elaborate.cxx
+++ b/elaborate.cxx
@@ -6892,6 +6892,14 @@ typeresolution_info::visit_return_statement (return_statement* e)
 
   exp_type& e_type = current_function->type;
   t = current_function->type;
+
+  if (!e->value)
+    {
+      if (e_type != pe_unknown)
+        mismatch (e->tok, pe_unknown, current_function);
+      return;
+    }
+
   e->value->visit (this);
 
   if (e_type != pe_unknown && e->value->type != pe_unknown
diff --git a/parse.cxx b/parse.cxx
index 751f56add..95abfeeea 100644
--- a/parse.cxx
+++ b/parse.cxx
@@ -2868,7 +2868,12 @@ parser::parse_return_statement ()
     throw PARSE_ERROR (_("found 'return' not in function context"));
   return_statement* s = new return_statement;
   s->tok = t;
-  s->value = parse_expression ();
+
+  t = peek ();
+  if (t->type == tok_operator && (t->content == ";" || t->content == "}"))
+    s->value = NULL;  // no return value
+  else
+    s->value = parse_expression ();
   return s;
 }
 
diff --git a/staptree.cxx b/staptree.cxx
index d1193e336..25cc9fe7c 100644
--- a/staptree.cxx
+++ b/staptree.cxx
@@ -1313,7 +1313,10 @@ void expr_statement::print (ostream& o) const
 
 void return_statement::print (ostream& o) const
 {
-  o << "return " << *value;
+  if (value)
+    o << "return " << *value;
+  else
+    o << "return";
 }
 
 
@@ -1964,7 +1967,8 @@ traversing_visitor::visit_foreach_loop (foreach_loop* s)
 void
 traversing_visitor::visit_return_statement (return_statement* s)
 {
-  s->value->visit (this);
+  if (s->value)
+    s->value->visit (this);
 }
 
 void
diff --git a/testsuite/systemtap.base/return_no_val.exp b/testsuite/systemtap.base/return_no_val.exp
new file mode 100644
index 000000000..96af59286
--- /dev/null
+++ b/testsuite/systemtap.base/return_no_val.exp
@@ -0,0 +1,135 @@
+set test "return_no_val"
+set testpath "$srcdir/$subdir"
+
+if {! [installtest_p]} { untested "$test"; return }
+
+# --- TEST 1 ---
+
+set subtest1 "TEST 1: return no value in void type func"
+foreach runtime [get_runtime_list] {
+    if {$runtime eq ""} {
+        set runtime "kernel"
+    }
+    set test_name "$test: $subtest1 ($runtime)"
+
+    set cmd "stap --runtime=$runtime '$srcdir/$subdir/${test}_1.stp'"
+    send_log "executing: $cmd\n"
+    set pipe [open "| sh -c {$cmd}" r]
+    set out [read $pipe]
+    set failed 0
+    if {[catch {close $pipe} stderr] != 0} {
+        if {$stderr ne "" && [string index $stderr end] ne "\n"} {
+            append stderr "\n"
+        }
+        global errorCode
+        if {"CHILDSTATUS" == [lindex $errorCode 0]} {
+            set failed [lindex $errorCode 2]
+        }
+    }
+
+    set exp_out "enter f\nexit\n"
+    regsub -all -- {\n} $exp_out {\n} escaped_exp_out
+    if {$out eq $exp_out} {
+        pass "${test_name}: stdout matches \"$escaped_exp_out\""
+    } else {
+        fail "${test_name}: stdout fails to match \"$escaped_exp_out\": got \"$out\""
+    }
+
+    set stderr_pat "\\AWARNING: statement will never be reached: identifier 'println' at \[^\\n\]*?\\.stp:4:5\\n source:     println\\(\"leave f\"\\);\\n             \\^\\n\\Z"
+    regsub -all -- {\n} $stderr_pat {\n} escaped_stderr_pat
+    if {[regexp -linestop -- $stderr_pat $stderr]} {
+        pass "${test_name}: stderr matches \"$escaped_stderr_pat\""
+    } else {
+        fail "${test_name}: stderr fails to match \"$escaped_stderr_pat\": got \"$stderr\""
+    }
+
+    if {$failed} {
+        fail "${test_name}: exit code should be zero but is $failed"
+    } else {
+        pass "${test_name}: exit code is zero"
+    }
+}
+
+# --- TEST 2 ---
+
+set subtest2 "TEST 2: return nothing in long type func (inferred by another return stmt)"
+set test_name "$test: $subtest2"
+
+set cmd "stap '$srcdir/$subdir/${test}_2.stp'"
+send_log "executing: $cmd\n"
+set pipe [open "| sh -c {$cmd}" r]
+set out [read $pipe]
+set failed 0
+if {[catch {close $pipe} stderr] != 0} {
+    if {$stderr ne "" && [string index $stderr end] ne "\n"} {
+        append stderr "\n"
+    }
+    global errorCode
+    if {"CHILDSTATUS" == [lindex $errorCode 0]} {
+        set failed [lindex $errorCode 2]
+    }
+}
+
+set exp_out ""
+regsub -all -- {\n} $exp_out {\n} escaped_exp_out
+if {$out eq $exp_out} {
+    pass "${test_name}: stdout matches \"$escaped_exp_out\""
+} else {
+    fail "${test_name}: stdout fails to match \"$escaped_exp_out\": got \"$out\""
+}
+
+if {$failed} {
+    pass "${test_name}: exit code should be non-zero"
+} else {
+    fail "${test_name}: exit code should be non-zero but is zero"
+}
+
+set stderr_pat "\\Asemantic error: type mismatch \\(unknown\\): keyword at \[^\\n\]*?\\.stp:3:9\\n        source:         return;\\n                        \\^\\n\\nsemantic error: type was first inferred here \\(long\\): number '1' at :5:12\\n        source:     return 1;\\n                           \\^\\n"
+regsub -all -- {\n} $stderr_pat {\n} escaped_stderr_pat
+if {[regexp -linestop -- $stderr_pat $stderr]} {
+    pass "${test_name}: stderr matches \"$escaped_stderr_pat\""
+} else {
+    fail "${test_name}: stderr fails to match \"$escaped_stderr_pat\": got \"$stderr\""
+}
+
+# --- TEST 3 ---
+
+set subtest3 "TEST 3: return nothing in long type func (inferred by caller)"
+set test_name "$test: $subtest3"
+
+set cmd "stap '$srcdir/$subdir/${test}_3.stp'"
+send_log "executing: $cmd\n"
+set pipe [open "| sh -c {$cmd}" r]
+set out [read $pipe]
+set failed 0
+if {[catch {close $pipe} stderr] != 0} {
+    if {$stderr ne "" && [string index $stderr end] ne "\n"} {
+        append stderr "\n"
+    }
+    global errorCode
+    if {"CHILDSTATUS" == [lindex $errorCode 0]} {
+        set failed [lindex $errorCode 2]
+    }
+}
+
+set exp_out ""
+regsub -all -- {\n} $exp_out {\n} escaped_exp_out
+if {$out eq $exp_out} {
+    pass "${test_name}: stdout matches \"$escaped_exp_out\""
+} else {
+    fail "${test_name}: stdout fails to match \"$escaped_exp_out\": got \"$out\""
+}
+
+if {$failed} {
+    pass "${test_name}: exit code should be non-zero"
+} else {
+    fail "${test_name}: exit code should be non-zero but is zero"
+}
+
+set stderr_pat "\\Asemantic error: type mismatch \\(unknown\\): keyword at \[^\\n\]*?\\.stp:3:9\\n        source:         return;\\n                        \\^\\n\\nsemantic error: type was first inferred here \\(long\\): identifier 'f' at :8:17\\n        source:     println\\(1 \\+ f\\(3\\)\\)\\n                                \\^\\n\\n"
+regsub -all -- {\n} $stderr_pat {\n} escaped_stderr_pat
+if {[regexp -linestop -- $stderr_pat $stderr]} {
+    pass "${test_name}: stderr matches \"$escaped_stderr_pat\""
+} else {
+    fail "${test_name}: stderr fails to match \"$escaped_stderr_pat\": got \"$stderr\""
+}
diff --git a/testsuite/systemtap.base/return_no_val_1.stp b/testsuite/systemtap.base/return_no_val_1.stp
new file mode 100644
index 000000000..34e6b5525
--- /dev/null
+++ b/testsuite/systemtap.base/return_no_val_1.stp
@@ -0,0 +1,11 @@
+function f() {
+    println("enter f");
+    return;
+    println("leave f");
+}
+
+probe begin {
+    f();
+    printf("exit\n");
+    exit();
+}
diff --git a/testsuite/systemtap.base/return_no_val_2.stp b/testsuite/systemtap.base/return_no_val_2.stp
new file mode 100644
index 000000000..98a15f85f
--- /dev/null
+++ b/testsuite/systemtap.base/return_no_val_2.stp
@@ -0,0 +1,11 @@
+function f(a) {
+    if (a > 0) {
+        return;
+    }
+    return 1;
+}
+
+probe begin {
+    println(f(3))
+    exit();
+}
diff --git a/testsuite/systemtap.base/return_no_val_3.stp b/testsuite/systemtap.base/return_no_val_3.stp
new file mode 100644
index 000000000..0d008f249
--- /dev/null
+++ b/testsuite/systemtap.base/return_no_val_3.stp
@@ -0,0 +1,10 @@
+function f(a) {
+    if (a > 0) {
+        return;
+    }
+}
+
+probe begin {
+    println(1 + f(3))
+    exit();
+}
diff --git a/translate.cxx b/translate.cxx
index c7e7aa53f..8c78df8a1 100644
--- a/translate.cxx
+++ b/translate.cxx
@@ -4405,11 +4405,18 @@ c_unparser::visit_return_statement (return_statement* s)
   if (current_function == 0)
     throw SEMANTIC_ERROR (_("cannot 'return' from probe"), s->tok);
 
-  if (s->value->type != current_function->type)
+  if (s->value)
+    {
+      if (s->value->type != current_function->type)
+        throw SEMANTIC_ERROR (_("return type mismatch"), current_function->tok,
+                              s->tok);
+
+      c_assign ("l->__retvalue", s->value, "return value");
+    }
+  else if (current_function->type != pe_unknown)
     throw SEMANTIC_ERROR (_("return type mismatch"), current_function->tok,
                           s->tok);
 
-  c_assign ("l->__retvalue", s->value, "return value");
   record_actions(1, s->tok, true);
   o->newline() << "goto out;";
 }
-- 
2.11.0.295.gd7dffce