2005-08-24 Frank Ch. Eigler <fche@elastic.org>

	* tapsets.cxx (*::emit_probe_entries): Treat NULL and "" last_errors
	both as clean early returns, not errors.
	* translate.cxx: Revamp last_error handling logic.  Remove all
	"goto out" paths from expression context.
	(visit_statement): Handle last_error exit one nesting level at a time.
	(visit_return_statement, visit_functioncall): Set/reset last_error="".
	(c_tmpcounter::visit_for_loop): New routine.
	(c_unparser::visit_foreach, visit_for_loop): Rewrite to properly
	support continue/breaks, non-local exits, (foreach) locks.
	(emit_global): Emit lock variable.
	(varlock ctor, dtor): Lock/unlock global variable.
	(varlock_w, varlock_r): New concrete subclasses.  Update all users.
	* tapset/builtin_logging.stp (exit): Don't set last_error.
	* src/testsuite/buildok/sixteen.stp: New test.

	* tapsets.cxx: Temporarily rolled back graydon's changes.

5 files changed, 473 insertions, 543 deletions

diff --git a/ChangeLog b/ChangeLog
index 78ae20dd..68674fa7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,22 @@
+2005-08-24  Frank Ch. Eigler  <fche@elastic.org>
+
+	* tapsets.cxx (*::emit_probe_entries): Treat NULL and "" last_errors
+	both as clean early returns, not errors.
+	* translate.cxx: Revamp last_error handling logic.  Remove all
+	"goto out" paths from expression context.
+	(visit_statement): Handle last_error exit one nesting level at a time.
+	(visit_return_statement, visit_functioncall): Set/reset last_error="".
+	(c_tmpcounter::visit_for_loop): New routine.
+	(c_unparser::visit_foreach, visit_for_loop): Rewrite to properly
+	support continue/breaks, non-local exits, (foreach) locks.
+	(emit_global): Emit lock variable.
+	(varlock ctor, dtor): Lock/unlock global variable.
+	(varlock_w, varlock_r): New concrete subclasses.  Update all users.
+	* tapset/builtin_logging.stp (exit): Don't set last_error.
+	* src/testsuite/buildok/sixteen.stp: New test.
+	
+	* tapsets.cxx: Temporarily rolled back graydon's changes.
+
 2005-08-23  Graydon Hoare  <graydon@redhat.com>
 
 	* tapsets.cxx: Re-implement dwarf probe-pattern resolution.
diff --git a/tapset/builtin_logging.stp b/tapset/builtin_logging.stp
index bf01a0fc..b1948877 100644
--- a/tapset/builtin_logging.stp
+++ b/tapset/builtin_logging.stp
@@ -24,9 +24,8 @@ function warn (msg) {
     _warn (msg . "")
 }
 
+// NB: exit() does *not* cause immediate return from current function/probe
 function exit () %{
-    /* not a NULL pointer, but don't cause _stp_error */
-    CONTEXT->last_error = "";
     _stp_exit ();
 %}
 
diff --git a/tapsets.cxx b/tapsets.cxx
index 30f93d46..61811163 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -127,8 +127,8 @@ be_derived_probe::emit_probe_entries (translator_output* o, unsigned j)
       // NB: locals are initialized by probe function itself
       o->newline() << "probe_" << j << " (c);";
 
-      o->newline() << "if (c->last_error) {";
-      o->newline(1) << "if (c->last_error[0]) _stp_error (\"%s near %s\", c->last_error, c->last_stmt);";
+      o->newline() << "if (c->last_error && c->last_error[0]) {";
+      o->newline(1) << "_stp_error (\"%s near %s\", c->last_error, c->last_stmt);";
       o->newline() << "atomic_set (& session_state, STAP_SESSION_ERROR);";
       o->newline(-1) << "}";
 
@@ -178,23 +178,6 @@ lex_cast_hex(IN const & in)
   return out;
 }
 
-struct 
-func_info
-{
-  string name;
-  Dwarf_Die die;
-  Dwarf_Addr prologue_end;
-};
-
-struct
-inline_instance_info
-{
-  string name;
-  Dwarf_Die die;
-};
-
-static int
-query_cu (Dwarf_Die * cudie, void * arg);
 
 
 // Helper for dealing with selected portions of libdwfl in a more readable
@@ -245,7 +228,6 @@ dwflpp
   {
     if (!module_dwarf)
       module_dwarf = dwfl_module_getdwarf(module, &module_bias);
-
     if (module_dwarf == NULL && sess.verbose)
       clog << "WARNING: dwfl_module_getdwarf() : "
 	   << dwfl_errmsg (dwfl_errno ()) << endl;
@@ -310,21 +292,21 @@ dwflpp
   }
 
 
-  void query_cu_containing_global_address(Dwarf_Addr a, void *arg)
+  void focus_on_cu_containing_global_address(Dwarf_Addr a)
   {
     Dwarf_Addr bias;
     assert(dwfl);
     get_module_dwarf();
     if (false && sess.verbose)
       clog << "focusing on cu containing global addr " << a << endl;
-    query_cu (dwfl_module_addrdie(module, a, &bias), arg);
+    focus_on_cu(dwfl_module_addrdie(module, a, &bias));
     assert(bias == module_bias);
   }
 
 
-  void query_cu_containing_module_address(Dwarf_Addr a, void *arg)
+  void focus_on_cu_containing_module_address(Dwarf_Addr a)
   {
-    query_cu_containing_global_address(module_address_to_global(a), arg);
+    focus_on_cu_containing_global_address(module_address_to_global(a));
   }
 
 
@@ -392,24 +374,15 @@ dwflpp
   }
 
 
-  void dwfl_assert(string desc, int rc) // NB: "rc == 0" means OK in this case
+  void dwflpp_assert(string desc, int rc) // NB: "rc == 0" means OK in this case
   {
-    string msg = "libdwfl failure (" + desc + "): ";
+    string msg = "dwfl failure (" + desc + "): ";
     if (rc < 0) msg += dwfl_errmsg (rc);
     else if (rc > 0) msg += strerror (rc);
     if (rc != 0)
       throw semantic_error (msg);
   }
 
-  void dwarf_assert(string desc, int rc) // NB: "rc == 0" means OK in this case
-  {
-    string msg = "libdw failure (" + desc + "): ";
-    if (rc < 0) msg += dwarf_errmsg (rc);
-    else if (rc > 0) msg += strerror (rc);
-    if (rc != 0)
-      throw semantic_error (msg);
-  }
-
 
   dwflpp(systemtap_session & sess)
     :
@@ -448,27 +421,25 @@ dwflpp
 
     if (kernel)
       {
-	dwfl = dwfl_begin (&kernel_callbacks);
+	dwfl = dwfl_begin(&kernel_callbacks);
 	if (!dwfl)
-	  throw semantic_error ("cannot open dwfl");
-	dwfl_report_begin (dwfl);
+	  throw semantic_error("cannot open dwfl");
+	dwfl_report_begin(dwfl);
         // XXX: if we have only kernel.* probe points, we shouldn't waste time
         // looking for module debug-info (and vice versa).
-	dwfl_assert ("dwfl_linux_kernel_report_kernel", 
-		     dwfl_linux_kernel_report_kernel (dwfl));
-	dwfl_assert ("dwfl_linux_kernel_report_modules", 
-		     dwfl_linux_kernel_report_modules (dwfl));
+	dwflpp_assert("find kernel debug-info", dwfl_linux_kernel_report_kernel(dwfl));
+	dwflpp_assert("find modules debug-info", dwfl_linux_kernel_report_modules(dwfl));
       }
     else
       {
-	dwfl = dwfl_begin (&proc_callbacks);
-	dwfl_report_begin (dwfl);
+	dwfl = dwfl_begin(&proc_callbacks);
+	dwfl_report_begin(dwfl);
 	if (!dwfl)
-	  throw semantic_error ("cannot open dwfl");
+	  throw semantic_error("cannot open dwfl");
 	// XXX: Find pids or processes, do userspace stuff.
       }
 
-    dwfl_assert ("dwfl_report_end", dwfl_report_end(dwfl, NULL, NULL));
+    dwflpp_assert("report_end", dwfl_report_end(dwfl, NULL, NULL));
   }
 
   void iterate_over_modules(int (* callback)(Dwfl_Module *, void **,
@@ -482,10 +453,9 @@ dwflpp
 	off = dwfl_getmodules (dwfl, callback, data, off);
       }
     while (off > 0);
-    dwfl_assert("dwfl_getmodules", off);
+    dwflpp_assert("getdwarf", off);
   }
 
-
   void iterate_over_cus (int (*callback)(Dwarf_Die * die, void * arg),
 			 void * data)
   {
@@ -501,167 +471,162 @@ dwflpp
     Dwarf_Off off = 0;
     size_t cuhl;
     Dwarf_Off noff;
-    while (dwarf_nextcu (dw, off, &noff, &cuhl, NULL, NULL, NULL) == 0)
+    while (dwarf_nextcu(dw, off, &noff, &cuhl, NULL, NULL, NULL) == 0)
       {
 	Dwarf_Die die_mem;
 	Dwarf_Die *die;
-	die = dwarf_offdie (dw, off + cuhl, &die_mem);
-	if (callback (die, data) != DWARF_CB_OK)
+	die = dwarf_offdie(dw, off + cuhl, &die_mem);
+	if (callback(die, data) != DWARF_CB_OK)
 	  break;
 	off = noff;
       }
   }
 
-
-  bool func_is_inline()
+  void iterate_over_functions(int (* callback)(Dwarf_Func * func, void * arg),
+			      void * data)
   {
-    assert (function);
-    return dwarf_func_inline (function) != 0;
+    assert(module);
+    assert(cu);
+    dwarf_getfuncs(cu, callback, data, 0);
   }
 
-  void iterate_over_inline_instances (int (* callback)(Dwarf_Die * die, void * arg),
-				      void * data)
+  bool function_entrypc(Dwarf_Addr * addr)
   {
-    assert (function);
-    assert (func_is_inline ());
-    dwarf_assert ("dwarf_func_inline_instances", 
-		  dwarf_func_inline_instances (function, callback, data));
+    return (dwarf_func_entrypc(function, addr) == 0);
   }
 
-
-  void iterate_over_functions (int (* callback)(Dwarf_Func * func, void * arg),
-			       void * data)
+  bool function_includes_global_addr(Dwarf_Addr addr)
   {
-    assert (module);
-    assert (cu);
-    dwarf_getfuncs (cu, callback, data, 0);
+    assert(module_dwarf);
+    assert(cu);
+    assert(function);
+    Dwarf_Addr lo, hi;
+    if (dwarf_func_lowpc(function, &lo) != 0)
+      {
+	if (false && sess.verbose)
+	  clog << "WARNING: cannot find low PC value for function " << function_name << endl;
+	return false;
+      }
+
+    if (dwarf_func_highpc(function, &hi) != 0)
+    {
+      if (false && sess.verbose)
+	clog << "WARNING: cannot find high PC value for function " << function_name << endl;
+      return false;
+    }
+
+    bool t = lo <= addr && addr <= hi;
+    if (t && sess.verbose)
+      clog << "function " << function_name << " = [" << hex << lo << "," << hi << "] "
+	   << "contains global addr " << addr << dec << endl;
+    return t;
   }
 
 
-  void iterate_over_srcfile_lines (char const * srcfile,
-				   int lineno, 
-				   void (* callback) (Dwarf_Line * line, void * arg),				   
-				   void *data)
+  Dwarf_Addr global_addr_of_line_in_cu(int line)
   {
-    Dwarf_Line **srcsp;
-    size_t nsrcs;
+    Dwarf_Lines * lines;
+    Dwarf_Addr addr;
+    size_t nlines;
+    int best_line = -1;
 
-    get_module_dwarf();
+    assert(module);
+    assert(cu);
+    dwflpp_assert("getsrclines", dwarf_getsrclines(cu, &lines, &nlines));
 
-    dwarf_assert ("dwarf_getsrc_file",
-		  dwarf_getsrc_file (module_dwarf, 
-				     srcfile, lineno, 0,
-				     &srcsp, &nsrcs));
-    
-    for (size_t i = 0; i < nsrcs; ++i)
+    for (size_t i = 0; i < nlines; ++i)
+      {
+	int curr_line;
+	Dwarf_Line * line_rec = dwarf_onesrcline(lines, i);
+	dwflpp_assert("lineno", dwarf_lineno (line_rec, &curr_line));
+
+	if (curr_line >= line && (best_line == -1 || curr_line < best_line))
+	  {
+	    best_line = curr_line;
+	    dwflpp_assert("lineaddr", dwarf_lineaddr(line_rec, &addr));
+	  }
+      }
+
+    if (best_line != -1)
       {
-	callback (srcsp[i], data);
+	if (sess.verbose)
+	  clog << "line " << best_line
+	       << " (given query line " << line << ")"
+	       << " of CU " << cu_name
+	       << " has module address " << hex << addr
+	       << " in " << module_name << dec << endl;
+	    return module_address_to_global(addr);
       }
+
+    if (sess.verbose)
+      clog << "WARNING: could not find line " << line
+	   << " in CU " << cu_name << endl;
+    return 0;
   }
 
 
-  void collect_srcfiles_matching (string const & pattern,
-				  set<char const *> & filtered_srcfiles)
+  bool function_prologue_end(Dwarf_Addr * addr)
   {
-    assert (module);
-    assert (cu);
+    Dwarf_Lines * lines;
+    size_t nlines;
 
-    size_t nfiles;
-    Dwarf_Files *srcfiles;
+    assert(addr);
+    dwflpp_assert("getsrclines", dwarf_getsrclines(cu, &lines, &nlines));
 
-    dwarf_assert ("dwarf_getsrcfiles", 
-		  dwarf_getsrcfiles (cu, &srcfiles, &nfiles));
-    {
-    for (size_t i = 0; i < nfiles; ++i)
+
+    // If GCC output the right information we would do this:
+    /*
+
+    for (size_t i = 0; i < nlines; ++i)
       {
-	char const * fname = dwarf_filesrc (srcfiles, i, NULL, NULL);
-	if (fnmatch (pattern.c_str(), fname, 0) == 0)
+	bool flag;
+	Dwarf_Line * line_rec = dwarf_onesrcline(lines, i);
+
+	dwflpp_assert("lineprologueend", dwarf_lineprologueend (line_rec, &flag));
+
+	if (sess.verbose)
+	  clog << "checked line record " << i
+	       << ", is " << (flag ? "" : " not")
+	       << " prologue end" << endl;
+
+	if (flag)
 	  {
-	    filtered_srcfiles.insert (fname);
-	    if (sess.verbose)
-	      clog << "selected source file '" << fname << "'" << endl;
+	    dwflpp_assert("lineaddr", dwarf_lineaddr(line_rec, addr));
+	    return true;
 	  }
       }
-    }
-  }  
+    return false;
+    */
 
-  void resolve_prologue_endings (map<Dwarf_Addr, func_info> & funcs)
-  {
-    assert(module);
-    assert(cu);
+    // Since GCC does not output the right information, we do this:
 
-    size_t nlines;
-    Dwarf_Lines *lines;
-    Dwarf_Addr previous_addr;
-    bool choose_next_line = false;
+    Dwarf_Addr entrypc;
+    if (!function_entrypc(&entrypc))
+      return false;
 
-    dwarf_assert ("dwarf_getsrclines", 
-		  dwarf_getsrclines(cu, &lines, &nlines));    
+    bool choose_next_line = false;
 
     for (size_t i = 0; i < nlines; ++i)
       {
-	Dwarf_Addr addr;
+	Dwarf_Addr line_addr;
 	Dwarf_Line * line_rec = dwarf_onesrcline(lines, i);
-	dwarf_lineaddr (line_rec, &addr);
-	
+	dwflpp_assert("lineaddr", dwarf_lineaddr(line_rec, &line_addr));
 	if (choose_next_line)
 	  {
-	    map<Dwarf_Addr, func_info>::iterator i = funcs.find (previous_addr);
-	    assert (i != funcs.end());
-	    i->second.prologue_end = addr;
-	    choose_next_line = false;
-	  }
-	
-	else 
-	  {
-	    map<Dwarf_Addr, func_info>::const_iterator i = funcs.find (addr);
-	    if (i != funcs.end())
-	      choose_next_line = true;
+	    *addr = line_addr;
+	    if (sess.verbose)
+	      clog << "function " << function_name
+		   << " entrypc: " << hex << entrypc
+		   << " prologue-end: " << line_addr << dec
+		   << endl;
+	    return true;
 	  }
-	previous_addr = addr;
+	else if (line_addr == entrypc)
+	  choose_next_line = true;
       }
+    return false;
   }
 
-
-  bool function_entrypc (Dwarf_Addr * addr)
-  {
-    assert (function);
-    return (dwarf_func_entrypc (function, addr) == 0);
-  }
-
-
-  bool die_entrypc (Dwarf_Die * die, Dwarf_Addr * addr)
-  {
-    Dwarf_Attribute attr_mem;
-    Dwarf_Attribute *attr = dwarf_attr (die, DW_AT_entry_pc, &attr_mem);
-    if (attr != NULL)
-      return (dwarf_formaddr (attr, addr) == 0);
-
-    return ( dwarf_lowpc (die, addr) == 0);
-  }
-
-
-  char const * function_srcfile ()
-  {
-    assert (function);
-    return dwarf_func_file (function);
-  }
-
-  void function_die (Dwarf_Die *d)
-  {
-    assert (function);
-    dwarf_func_die (function, d);
-  }
-
-  bool die_has_pc (Dwarf_Die * die, Dwarf_Addr pc)
-  {
-    int res = dwarf_haspc (die, pc);
-    if (res == -1)
-      dwarf_assert ("dwarf_haspc", res);
-    return res == 1;
-  }
-
-
   static void loc2c_error (void *arg, const char *fmt, ...)
   {
     char *msg = NULL;
@@ -672,7 +637,6 @@ dwflpp
     throw semantic_error (msg);
   }
 
-
   static void loc2c_emit_address (void *arg, struct obstack *pool,
 				  Dwarf_Addr address)
   {
@@ -736,13 +700,8 @@ dwflpp
 
     Dwarf_Attribute attr_mem;
     if (dwarf_attr_integrate (&vardie, DW_AT_location, &attr_mem) == NULL)
-      {
-	throw semantic_error("failed to retrieve location "
-			     "attribute for local '" + local 
-			     + "' (dieoffset: " 
-			     + lex_cast_hex<string>(dwarf_dieoffset (&vardie)) 
-			     + ")");
-      }
+      throw semantic_error("failed to retrieve location "
+			   "attribute for local '" + local + "'");
 
 #define obstack_chunk_alloc malloc
 #define obstack_chunk_free free
@@ -1017,14 +976,6 @@ dwarf_query
   string file;
   int line;
 
-  set<char const *> filtered_srcfiles;
-
-  // Map official entrypc -> func_info object
-  map<Dwarf_Addr, inline_instance_info> filtered_inlines;
-  map<Dwarf_Addr, func_info> filtered_functions;
-  bool choose_next_line;
-  Dwarf_Addr entrypc_for_next_line;
-
   probe * base_probe;
   probe_point * base_loc;
   dwflpp & dw;
@@ -1187,35 +1138,8 @@ dwarf_query::parse_function_spec(string & spec)
 }
 
 
-
-      // The critical determining factor when interpreting a pattern
-      // string is, perhaps surprisingly: "presence of a lineno". The
-      // presence of a lineno changes the search strategy completely.
-      //
-      // Compare the two cases:
-      //
-      //   1. {statement,function}(foo@file.c:lineno)
-      //      - find the files matching file.c
-      //      - in each file, find the functions matching foo
-      //      - query the file for line records matching lineno
-      //      - iterate over the line records,
-      //        - and iterate over the functions, 
-      //          - if(haspc(function.DIE, line.addr)) 
-      //            - if looking for statements: probe(lineno.addr)
-      //            - if looking for functions: probe(function.{entrypc,return,etc.})
-      //   
-      //   2. {statement,function}(foo@file.c)
-      //      - find the files matching file.c
-      //      - in each file, find the functions matching foo
-      //        - probe(function.{entrypc,return,etc.})
-      //
-      // Thus the first decision we make is based on the presence of a
-      // lineno, and we enter entirely different sets of callbacks
-      // depending on that decision.
-
-
 static void
-query_statement (Dwarf_Addr stmt_addr, dwarf_query * q)
+query_statement(Dwarf_Addr stmt_addr, dwarf_query * q)
 {
   try
     {
@@ -1232,195 +1156,129 @@ query_statement (Dwarf_Addr stmt_addr, dwarf_query * q)
     }
 }
 
-static void
-query_inline_instance_info (Dwarf_Addr entrypc,
-			    inline_instance_info const & ii,
-			    dwarf_query * q)
-{
-  if (q->has_return)
-    {
-      throw semantic_error ("cannot probe .return of inline function '" + ii.name + "'");
-    }
-  else
-    {
-      if (q->sess.verbose)
-	clog << "querying entrypc " 
-	     << hex << entrypc << dec 
-	     << " of instance of inline '" << ii.name << "'" << endl;
-      query_statement (entrypc, q);
-    }
-}
-
-static void
-query_func_info (Dwarf_Addr entrypc,
-		 func_info const & fi,
-		 dwarf_query * q)
-{
-  if (q->has_return)
-    {
-      // NB. dwarf_derived_probe::emit_registrations will emit a
-      // kretprobe based on the entrypc in this case.
-      if (q->sess.verbose)
-	clog << "querying entrypc of function '" << fi.name << "' for return probe" << endl;
-      query_statement (entrypc, q);
-    }
-  else
-    {
-      if (q->sess.verbose)
-	clog << "querying prologue-end of function '" << fi.name << "'" << endl;
-      query_statement (fi.prologue_end, q);
-    }
-}
-
-
-static void
-query_srcfile_line (Dwarf_Line * line, void * arg)
-{
-  dwarf_query * q = static_cast<dwarf_query *>(arg);
-
-  Dwarf_Addr addr;
-  dwarf_lineaddr(line, &addr);
-  
-  for (map<Dwarf_Addr, func_info>::iterator i = q->filtered_functions.begin();
-       i != q->filtered_functions.end(); ++i)
-    {
-      if (q->dw.die_has_pc (&(i->second.die), addr))
-	{
-	  if (q->sess.verbose)
-	    clog << "function DIE lands on srcfile" << endl;
-	  query_func_info (i->first, i->second, q);
-	}
-    }  
-
-  for (map<Dwarf_Addr, inline_instance_info>::iterator i = q->filtered_inlines.begin();
-       i != q->filtered_inlines.end(); ++i)
-    {
-      if (q->dw.die_has_pc (&(i->second.die), addr))
-	{
-	  if (q->sess.verbose)
-	    clog << "inline instance DIE lands on srcfile" << endl;
-	  query_inline_instance_info (i->first, i->second, q);
-	}
-    }  
-}
-
-
 static int
-query_dwarf_inline_instance (Dwarf_Die * die, void * arg)
+query_function(Dwarf_Func * func, void * arg)
 {
   dwarf_query * q = static_cast<dwarf_query *>(arg);
-  assert (!q->has_statement_num);
-
-  try
-    {
-
-      bool record_this_inline = false;
-
-      if (q->sess.verbose)
-	clog << "examining inline instance of " << q->dw.function_name << endl;
-
-      if (q->has_function_str || q->has_statement_str)
-	record_this_inline = true;
-      else if (q->has_function_num)
-	{
-	  Dwarf_Addr query_addr = q->function_num_val;
-      
-	  if (q->has_module)
-	    query_addr = q->dw.module_address_to_global(query_addr);
-      
-	  if (q->dw.die_has_pc (die, query_addr))
-	    record_this_inline = true;      
-	}
-
-      if (record_this_inline)
-	{
-	  if (q->sess.verbose)
-	    clog << "selected inline instance of " << q->dw.function_name << endl;
-
-	  Dwarf_Addr entrypc;
-	  if (q->dw.die_entrypc (die, &entrypc))
-	    {
-	      inline_instance_info inl;
-	      inl.die = *die;
-	      inl.name = q->dw.function_name;
-	      q->filtered_inlines[entrypc] = inl;
-	    }
-	}
-      return DWARF_CB_OK;
-    }
-  catch (const semantic_error& e)
-    {
-      q->sess.print_error (e);
-      return DWARF_CB_ABORT;
-    }
-}
-
-static int
-query_dwarf_func (Dwarf_Func * func, void * arg)
-{
-  dwarf_query * q = static_cast<dwarf_query *>(arg);
-  assert (!q->has_statement_num);
 
   try
     {
       // XXX: implement
       if (q->has_callees)
-        throw semantic_error ("incomplete: do not know how to interpret .callees",
-			      q->base_probe->tok);
-      
+        throw semantic_error("incomplete: do not know how to interpret .callees",
+                             q->base_probe->tok);
+
       if (q->has_label)
-        throw semantic_error ("incomplete: do not know how to interpret .label",
-			      q->base_probe->tok);
-
-      q->dw.focus_on_function (func);
-
-      if (q->dw.func_is_inline () 
-	  && (((q->has_statement_str || q->has_function_str)
-	       && q->dw.function_name_matches(q->function))
-	      || q->has_function_num))
-	{
-	  if (q->sess.verbose)
-	    clog << "checking instances of inline " << q->dw.function_name << endl;
-	  q->dw.iterate_over_inline_instances (query_dwarf_inline_instance, arg);
-	}
+        throw semantic_error("incomplete: do not know how to interpret .label",
+                             q->base_probe->tok);
+
+      q->dw.focus_on_function(func);
+
+      Dwarf_Addr entry_addr;
+
+      if (q->has_statement_str || q->has_function_str)
+        {
+          if (q->dw.function_name_matches(q->function))
+            {
+              if (q->sess.verbose)
+                clog << "focused on function '" << q->dw.function_name
+                     << "', in CU '" << q->dw.cu_name
+                     << "', module '" << q->dw.module_name << "'" << endl;
+
+              // XXX: This code is duplicated below, but it's important
+              // for performance reasons to test things in this order.
+
+              if (q->has_statement_str)
+                {
+                  // XXX: look up address corresponding to statement string,
+                  // which could be any old line within a function definition.
+                  cerr << "WARNING: cannot handle statement "
+                       << q->statement_str_val << " address" << endl;
+                  return DWARF_CB_OK;
+                }
+              if (q->has_return)
+                {
+                  bool ok = q->dw.function_entrypc (& entry_addr);
+                  if (! ok)
+                    {
+                      if (q->sess.verbose)
+                        cerr << "WARNING: cannot find entry-pc for function "
+                             << q->dw.function_name << endl;
+                      return DWARF_CB_OK;
+                    }
+                  if (q->sess.verbose)
+                    clog << "function " << q->dw.function_name
+                         << " entrypc: " << hex << entry_addr << dec << endl;
+                }
+              else
+                {
+                  bool ok = q->dw.function_prologue_end(& entry_addr);
+                  if (! ok)
+                    {
+                      // XXX: but this is actually OK for inlined function instances
+                      if (q->sess.verbose)
+                        cerr << "WARNING: cannot find prologue-end PC for function "
+                             << q->dw.function_name << endl;
+                      return DWARF_CB_OK;
+                    }
+                }
+
+              // If this function's name matches a function or statement
+              // pattern, we use its entry pc, but we do not abort iteration
+              // since there might be other functions matching the pattern.
+              query_statement(entry_addr, q);
+            }
+        }
       else
-	{      
-	  bool record_this_function = false;
-
-	  if ((q->has_statement_str || q->has_function_str)
-	      && q->dw.function_name_matches(q->function))
-	    {
-	      record_this_function = true;
-	    }
-	  else if (q->has_function_num)
-	    {
-	      Dwarf_Addr query_addr = q->function_num_val;
-	  
-	      if (q->has_module)
-		query_addr = q->dw.module_address_to_global(query_addr);
-
-	      Dwarf_Die d;
-	      q->dw.function_die (&d);
-	  
-	      if (q->dw.die_has_pc (&d, query_addr))
-		record_this_function = true;
-	    }
-
-	  if (record_this_function)
-	    {
-	      if (q->sess.verbose)
-		clog << "selected function " << q->dw.function_name << endl;
-
-	      Dwarf_Addr entrypc;
-	      if (q->dw.function_entrypc (&entrypc))
-		{
-		  func_info func;
-		  q->dw.function_die (&func.die);
-		  func.name = q->dw.function_name;
-		  q->filtered_functions[entrypc] = func;
-		}
-	    }
-	}
+        {
+          if (q->has_function_num || q->has_statement_num)
+            {
+              Dwarf_Addr query_addr = (q->has_function_num
+                                       ? q->function_num_val
+                                       : q->statement_num_val);
+
+              // Adjust module-relative address to global
+
+              if (q->has_module)
+                query_addr = q->dw.module_address_to_global(query_addr);
+
+              if (q->dw.function_includes_global_addr(query_addr))
+                {
+                  if (q->has_statement_num) // has_statement
+                    entry_addr = 0; // unused, see below
+                  else if (q->has_return) // has_function
+                    {
+                      bool ok = q->dw.function_entrypc (& entry_addr);
+                      if (! ok)
+                        {
+                          if (q->sess.verbose)
+                            cerr << "WARNING: cannot find entry-pc for function "
+                                 << q->dw.function_name << endl;
+                          return DWARF_CB_OK;
+                        }
+                      if (q->sess.verbose)
+                        clog << "function " << q->dw.function_name
+                             << " entrypc: " << hex << entry_addr << dec << endl;
+                    }
+                  else // has_function
+                    {
+                      bool ok = q->dw.function_prologue_end(& entry_addr);
+                      if (! ok)
+                        {
+                          // XXX: but this is actually OK for inlined function instances
+                          if (q->sess.verbose)
+                            cerr << "WARNING: cannot find prologue-end PC for function "
+                                 << q->dw.function_name << endl;
+                          return DWARF_CB_OK;
+                        }
+                    }
+
+                  query_statement(q->has_function_num ? entry_addr : query_addr, q);
+                  return DWARF_CB_ABORT;
+                }
+            }
+        }
+
       return DWARF_CB_OK;
     }
   catch (const semantic_error& e)
@@ -1437,77 +1295,52 @@ query_cu (Dwarf_Die * cudie, void * arg)
 
   try
     {
-      q->dw.focus_on_cu (cudie);
+      q->dw.focus_on_cu(cudie);
+
+      // If we have enough information in the pattern to skip a CU
+      // and the CU does not match that information, return early.
+      if ((q->has_statement_str || q->has_function_str)
+          && (q->spec_type == function_file_and_line ||
+              q->spec_type == function_and_file)
+          && (!q->dw.cu_name_matches(q->file)))
+        return DWARF_CB_OK;
 
       if (false && q->sess.verbose)
         clog << "focused on CU '" << q->dw.cu_name
              << "', in module '" << q->dw.module_name << "'" << endl;
 
-      if (q->has_statement_str || q->has_function_str || q->has_function_num)
-	{
-	  q->filtered_srcfiles.clear();
-	  q->filtered_functions.clear();
-	  q->filtered_inlines.clear();
-
-	  // In this path, we find "abstract functions", record
-	  // information about them, and then (depending on lineno
-	  // matching) possibly emit one or more of the function's
-	  // associated addresses. Unfortunately the control of this
-	  // cannot easily be turned inside out.
-
-	  if ((q->has_statement_str || q->has_function_str)
-	      && (q->spec_type != function_alone))
-	    {
-	      // If we have a pattern string with a filename, we need
-	      // to elaborate the srcfile mask in question first.
-	      q->dw.collect_srcfiles_matching (q->file, q->filtered_srcfiles);
-
-	      // If we have a file pattern and *no* srcfile matches, there's
-	      // no need to look further into this CU, so skip.
-	      if (q->filtered_srcfiles.empty())
-		return DWARF_CB_OK;
-	    }
-
-	  // Pick up [entrypc, name, DIE] tuples for all the functions
-	  // matching the query, and fill in the prologue endings of them
-	  // all in a single pass.
-	  q->dw.iterate_over_functions (query_dwarf_func, q);
-	  q->dw.resolve_prologue_endings (q->filtered_functions);
-
-	  if ((q->has_statement_str || q->has_function_str)
-	      && (q->spec_type == function_file_and_line))
-	    {
-	      // If we have a pattern string with target *line*, we
-	      // have to look at lines in all the matched srcfiles. 
-	      for (set<char const *>::const_iterator i = q->filtered_srcfiles.begin();
-		   i != q->filtered_srcfiles.end(); ++i)
-		q->dw.iterate_over_srcfile_lines (*i, q->line, query_srcfile_line, q);
-	    }
-	  else
-	    {
-	      // Otherwise, simply probe all resolved functions...
-	      for (map<Dwarf_Addr, func_info>::const_iterator i = q->filtered_functions.begin();
-		   i != q->filtered_functions.end(); ++i)
-		query_func_info (i->first, i->second, q);
-
-	      // And all inline instances.
-	      for (map<Dwarf_Addr, inline_instance_info>::const_iterator i 
-		     = q->filtered_inlines.begin(); i != q->filtered_inlines.end(); ++i)
-		query_inline_instance_info (i->first, i->second, q);
-
-	    }
-	}
+      if (q->has_statement_str
+          && (q->spec_type == function_file_and_line)
+          && q->dw.cu_name_matches(q->file))
+        {
+          // If we have a complete file:line statement
+          // functor (not function functor) landing on
+          // this CU, we can look up a specific address
+          // for the statement, and skip scanning
+          // the remaining functions within the CU.
+          query_statement(q->dw.global_addr_of_line_in_cu(q->line), q);
+        }
+      else if (q->has_function_str
+          && (q->spec_type == function_file_and_line)
+          && q->dw.cu_name_matches(q->file))
+        {
+          // If we have a complete file:line *function* functor
+          // landing on this CU, we need to select only the functions
+          // which land on the line in question. We *could* check each
+          // function individually but the line->addr lookup is
+          // expensive, so we do it once here, then temporarily switch
+          // to a .function(addr) query for the remaining function
+          // iteration, switching back when we complete.
+	  q->function_num_val = q->dw.global_addr_of_line_in_cu(q->line);
+	  swap(q->has_function_str, q->has_function_num);
+          q->dw.iterate_over_functions(&query_function, q);
+	  swap(q->has_function_str, q->has_function_num);
+        }
       else
         {
-	  // Otherwise we have a statement number, and we can just
-	  // query it directly within this module.
-
-	  assert (q->has_statement_num);
-	  Dwarf_Addr query_addr = q->statement_num_val;
-	  if (q->has_module)
-	    query_addr = q->dw.module_address_to_global(query_addr);
-
-	  query_statement (query_addr, q);
+          // Otherwise we need to scan all the functions in this CU,
+	  // matching by function name or address, as requested.
+          q->dw.iterate_over_functions(&query_function, q);
         }
       return DWARF_CB_OK;
     }
@@ -1551,38 +1384,35 @@ query_module (Dwfl_Module *mod __attribute__ ((unused)),
           // module("foo").statement(0xbeef), the address is relative
           // to the start of the module, so we seek the function
           // number plus the module's bias.
-
           Dwarf_Addr addr;
           if (q->has_function_num)
             addr = q->function_num_val;
           else
             addr = q->statement_num_val;
 
-	  // NB: We should not have kernel.* here; global addresses
-	  // should have bypassed query_module in dwarf_builder::build
-	  // and gone directly to query_cu.
+          if (q->has_kernel)
+            q->dw.focus_on_cu_containing_global_address(addr);
+          else
+            q->dw.focus_on_cu_containing_module_address(addr);
 
-          assert (!q->has_kernel);
-          assert (q->has_module);
-	  q->dw.query_cu_containing_module_address(addr, q);
+          q->dw.iterate_over_functions(&query_function, q);
         }
       else
         {
           // Otherwise if we have a function("foo") or statement("foo")
           // specifier, we have to scan over all the CUs looking for
-          // the function(s) in question
-
+          // the function in question
           assert(q->has_function_str || q->has_statement_str);
           q->dw.iterate_over_cus(&query_cu, q);
-
-	  // If we just processed the module "kernel", and the user asked for
-	  // the kernel pattern, there's no need to iterate over any further
-	  // modules
-	  
-	  if (q->has_kernel && q->dw.module_name_matches(TOK_KERNEL))
-	    return DWARF_CB_ABORT;
         }
 
+      // If we just processed the module "kernel", and the user asked for
+      // the kernel pattern, there's no need to iterate over any further
+      // modules
+
+      if (q->has_kernel && q->dw.module_name_matches(TOK_KERNEL))
+        return DWARF_CB_ABORT;
+
       return DWARF_CB_OK;
     }
   catch (const semantic_error& e)
@@ -1822,11 +1652,6 @@ probe_entry_struct_kprobe_name(unsigned probenum)
 void
 dwarf_derived_probe::emit_registrations (translator_output* o, unsigned probenum)
 {
-  if (! (module_name.empty() || module_name == "kernel"))
-    {
-      // XXX: lock module_name in memory
-    }
-
   if (has_return)
     {
       o->newline() << probe_entry_struct_kprobe_name(probenum)
@@ -1856,11 +1681,6 @@ dwarf_derived_probe::emit_deregistrations (translator_output* o, unsigned proben
     o->newline() << "unregister_kprobe (& "
                  << probe_entry_struct_kprobe_name(probenum)
                  << ");";
-
-  if (! (module_name.empty() || module_name == "kernel"))
-    {
-      // XXX: unlock module_name
-    }
 }
 
 void
@@ -1901,11 +1721,11 @@ dwarf_derived_probe::emit_probe_entries (translator_output* o, unsigned probenum
   // NB: locals are initialized by probe function itself
   o->newline() << "probe_" << probenum << " (c);";
 
-  o->newline() << "if (c->last_error) {";
-  o->newline(1) << "if (c->last_error[0]) _stp_error (\"%s near %s\", c->last_error, c->last_stmt);";
+  o->newline() << "if (c->last_error && c->last_error[0]) {";
+  o->newline(1) << "_stp_error (\"%s near %s\", c->last_error, c->last_stmt);";
   o->newline() << "atomic_set (& session_state, STAP_SESSION_ERROR);";
   o->newline(-1) << "}";
-
+  
   o->newline() << "c->busy --;";
   o->newline() << "mb ();";
 
@@ -1919,7 +1739,9 @@ dwarf_derived_probe::emit_probe_entries (translator_output* o, unsigned probenum
                    << probe_entry_struct_kprobe_name(probenum)
                    << "= {";
       o->newline(1) << ".kp.addr = 0," ;
-      o->newline() << ".handler = &" << probe_entry_function_name(probenum);
+      o->newline() << ".handler = &"
+                   << probe_entry_function_name(probenum) << ",";
+      o->newline() << ".maxactive = 1";
       o->newline(-1) << "};";
     }
   else
@@ -1949,7 +1771,8 @@ dwarf_builder::build(systemtap_session & sess,
 
   dw.setup(q.has_kernel || q.has_module);
 
-  if (q.has_kernel && (q.has_function_num || q.has_statement_num))
+  if (q.has_kernel
+      && (q.has_function_num || q.has_statement_num))
     {
       // If we have kernel.function(0xbeef), or
       // kernel.statement(0xbeef) the address is global (relative to
@@ -1959,7 +1782,8 @@ dwarf_builder::build(systemtap_session & sess,
 		      ? q.function_num_val
 		      : q.statement_num_val);
       dw.focus_on_module_containing_global_address(a);
-      dw.query_cu_containing_global_address(a, &q);
+      dw.focus_on_cu_containing_global_address(a);
+      dw.iterate_over_functions(&query_function, &q);
     }
   else
     {
@@ -2067,8 +1891,8 @@ timer_derived_probe::emit_probe_entries (translator_output* o, unsigned j)
   // NB: locals are initialized by probe function itself
   o->newline() << "probe_" << j << " (c);";
   
-  o->newline() << "if (c->last_error) {";
-  o->newline(1) << "if (c->last_error[0]) _stp_error (\"%s near %s\", c->last_error, c->last_stmt);";
+  o->newline() << "if (c->last_error && c->last_error[0]) {";
+  o->newline(1) << "_stp_error (\"%s near %s\", c->last_error, c->last_stmt);";
   o->newline() << "atomic_set (& session_state, STAP_SESSION_ERROR);";
   o->newline(-1) << "}";
   
diff --git a/testsuite/buildok/sixteen.stp b/testsuite/buildok/sixteen.stp
new file mode 100755
index 00000000..26932b8e
--- /dev/null
+++ b/testsuite/buildok/sixteen.stp
@@ -0,0 +1,21 @@
+#! stap -p4
+
+global a
+
+function foo () {
+  if (a[k] == "sayonara") { return 2 }
+}
+
+
+probe begin {
+  a[1] = "hello"
+  a[2] = "goodbye"
+  foreach (k in a) {
+    log (a[k])
+    for (i=0; i<10; i++) {
+      log ("k=" . string(k) . " i=" . string(i))
+      if (k % (i+1)) break else continue
+    }
+    if (k % 3) { foo() ; next }
+  }
+}
diff --git a/translate.cxx b/translate.cxx
index 9c1167fd..fd53d80f 100644
--- a/translate.cxx
+++ b/translate.cxx
@@ -142,7 +142,7 @@ struct c_tmpcounter:
     parent->tmpvar_counter = 0;
   }
 
-  // void visit_for_loop (for_loop* s);
+  void visit_for_loop (for_loop* s);
   void visit_foreach_loop (foreach_loop* s);
   // void visit_return_statement (return_statement* s);
   // void visit_delete_statement (delete_statement* s);
@@ -272,8 +272,7 @@ struct stmt_expr
   }
   ~stmt_expr()
   {
-    c.o->line() << "})";
-    c.o->indent(-1);
+    c.o->newline(-1) << "})";
   }
 };
 
@@ -281,20 +280,32 @@ struct varlock
 {
   c_unparser & c;
   var const & v;
+  bool w;
 
-  varlock(c_unparser & c, var const & v) : c(c), v(v)
+  varlock(c_unparser & c, var const & v, bool w): c(c), v(v), w(w)
   {
-    if (!v.is_local())
-      c.o->newline() << "/* XXX lock " << v << " */";
+    if (v.is_local()) return;
+    c.o->newline() << (w ? "write_lock" : "read_lock")
+                   << " (& " << v << "_lock);";
   }
 
   ~varlock()
   {
-    if (!v.is_local())
-      c.o->newline() << "/* XXX unlock " << v << " */";
+    if (v.is_local()) return;
+    c.o->newline() << (w ? "write_unlock" : "read_unlock")
+                   << " (& " << v << "_lock);";
   }
 };
 
+struct varlock_r: public varlock {
+  varlock_r (c_unparser& c, var const& v): varlock (c, v, false) {}
+};
+
+struct varlock_w: public varlock {
+  varlock_w (c_unparser& c, var const& v): varlock (c, v, true) {}
+};
+
+
 struct tmpvar
   : public var
 {
@@ -530,10 +541,15 @@ c_unparser::emit_common_header ()
   o->newline() << "atomic_t session_state = ATOMIC_INIT (STAP_SESSION_STARTING);";
   o->newline();
   o->newline() << "struct context {";
-  o->newline(1) << "unsigned busy;";
+  o->newline(1) << "unsigned busy;"; // XXX: should be atomic_t ?
   o->newline() << "unsigned actioncount;";
   o->newline() << "unsigned nesting;";
   o->newline() << "const char *last_error;";
+  // NB: last_error is used as a health flag within a probe.
+  // While it's 0, execution continues
+  // When it's "", current function or probe unwinds and returns early
+  // When it's "something", probe code unwinds, _stp_error's, sets error state
+  // See c_unparser::visit_statement()
   o->newline() << "const char *last_stmt;";
   o->newline() << "struct pt_regs *regs;";
   o->newline() << "union {";
@@ -603,11 +619,9 @@ c_unparser::emit_global (vardecl *v)
   else
     o->newline() << "static MAP global_" 
 		 << c_varname(v->name) << ";";
-  /* XXX
-  o->line() << "static DEFINE_RWLOCK("
-             << c_varname (v->name) << "_lock"
-             << ");";
-  */
+  o->newline() << "static DEFINE_RWLOCK("
+               << "global_" << c_varname (v->name) << "_lock"
+               << ");";
 }
 
 
@@ -1253,7 +1267,18 @@ c_unparser::getiter(foreach_loop *f)
 void
 c_unparser::visit_statement (statement *s, unsigned actions)
 {
-  o->newline() << "if (unlikely (c->last_error)) goto out;";
+  // For some constructs, it is important to avoid an error branch
+  // right to the bottom of the probe/function.  The foreach() locking
+  // construct is one example.  Instead, if we are nested within a
+  // loop, we branch merely to its "break" label.  The next statement
+  // will branch one level higher, and so on, until we can go straight
+  // "out".
+  string outlabel = "out";
+  unsigned loops = loop_break_labels.size();
+  if (loops > 0)
+    outlabel = loop_break_labels[loops-1];
+
+  o->newline() << "if (unlikely (c->last_error)) goto " << outlabel << ";";
   assert (s->tok);
   o->newline() << "c->last_stmt = \"" << *s->tok << "\";";
   if (actions > 0)
@@ -1261,7 +1286,7 @@ c_unparser::visit_statement (statement *s, unsigned actions)
       o->newline() << "c->actioncount += " << actions << ";";
       o->newline() << "if (unlikely (c->actioncount > MAXACTION)) {";
       o->newline(1) << "c->last_error = \"MAXACTION exceeded\";";
-      o->newline() << "goto out;";
+      o->newline() << "goto " << outlabel << ";";
       o->newline(-1) << "}";
     }
 }
@@ -1341,33 +1366,52 @@ c_unparser::visit_if_statement (if_statement *s)
 
 
 void
+c_tmpcounter::visit_for_loop (for_loop *s)
+{
+  s->init->visit (this);
+  s->cond->visit (this);
+  s->block->visit (this);
+  s->incr->visit (this);
+}
+
+
+void
 c_unparser::visit_for_loop (for_loop *s)
 {
   visit_statement (s, 1);
 
-  s->init->visit (this);
   string ctr = stringify (label_counter++);
+  string toplabel = "top_" + ctr;
   string contlabel = "continue_" + ctr;
   string breaklabel = "break_" + ctr;
 
-  o->newline() << contlabel << ":";
+  // initialization
+  s->init->visit (this);
 
-  o->newline() << "if (! (";
+  // condition
+  o->newline(-1) << toplabel << ":";
+  o->newline(1) << "if (! (";
   if (s->cond->type != pe_long)
     throw semantic_error ("expected numeric type", s->cond->tok);
   s->cond->visit (this);
   o->line() << ")) goto " << breaklabel << ";";
 
+  // body
   loop_break_labels.push_back (breaklabel);
   loop_continue_labels.push_back (contlabel);
   s->block->visit (this);
   loop_break_labels.pop_back ();
   loop_continue_labels.pop_back ();
 
+  // iteration
+  o->newline(-1) << contlabel << ":";
+  o->indent(1);
   s->incr->visit (this);
-  o->newline() << "goto " << contlabel << ";";
+  o->newline() << "goto " << toplabel << ";";
 
-  o->newline() << breaklabel << ": ; /* dummy statement */";
+  // exit
+  o->newline(-1) << breaklabel << ":";
+  o->newline(1) << "; /* dummy statement */";
 }
 
 
@@ -1387,27 +1431,47 @@ c_unparser::visit_foreach_loop (foreach_loop *s)
   mapvar mv = getmap (s->base_referent, s->tok);
   itervar iv = getiter (s);
   vector<var> keys;
-    
-  varlock guard (*this, mv);
-    
-  o->newline() << "for ("
-	       << iv << " = " << iv.start (mv) << "; ";
-  o->newline() << "     " << iv << "; ";
-  o->newline() << "     " << iv << " = " << iv.next (mv) << ")";
+
+  string ctr = stringify (label_counter++);
+  string toplabel = "top_" + ctr;
+  string contlabel = "continue_" + ctr;
+  string breaklabel = "break_" + ctr;
+
+  // NB: structure parallels for_loop
+
+  // initialization
+  varlock_r guard (*this, mv);
+  o->newline() << iv << " = " << iv.start (mv) << ";";
+
+  // condition
+  o->newline(-1) << toplabel << ":";
+  o->newline(1) << "if (! (" << iv << ")) goto " << breaklabel << ";";
+
+  // body
+  loop_break_labels.push_back (breaklabel);
+  loop_continue_labels.push_back (contlabel);
   o->newline() << "{";
   o->indent (1);
-    
   for (unsigned i = 0; i < s->indexes.size(); ++i)
     {
       // copy the iter values into the specified locals
       var v = getvar (s->indexes[i]->referent);
       c_assign (v, iv.get_key (v.type(), i), s->tok);
     }
-
   s->block->visit (this);
+  o->newline(-1) << "}";
+  loop_break_labels.pop_back ();
+  loop_continue_labels.pop_back ();
+
+  // iteration
+  o->newline(-1) << contlabel << ":";
+  o->newline(1) << iv << " = " << iv.next (mv) << ";";
+  o->newline() << "goto " << toplabel << ";";
     
-  o->indent (-1);
-  o->newline() << "}";
+  // exit
+  o->newline(-1) << breaklabel << ":";
+  o->indent(1);
+  // varlock dtor will show up here
 }
 
 
@@ -1424,7 +1488,9 @@ c_unparser::visit_return_statement (return_statement* s)
                          "vs", s->tok);
 
   c_assign ("l->__retvalue", s->value, "return value");
-  o->newline() << "goto out;";
+  o->newline() << "c->last_error = \"\";";
+  // NB: last_error needs to get reset to NULL in the caller
+  // probe/function
 }
 
 
@@ -1436,7 +1502,7 @@ c_unparser::visit_next_statement (next_statement* s)
   if (current_probe == 0)
     throw semantic_error ("cannot 'next' from function", s->tok);
 
-  o->newline() << "goto out;";
+  o->newline() << "c->last_error = \"\";";
 }
 
 
@@ -1456,7 +1522,7 @@ void
 delete_statement_operand_visitor::visit_symbol (symbol* e)
 {
   mapvar mvar = parent->getmap(e->referent, e->tok);  
-  varlock guard (*parent, mvar);
+  varlock_w guard (*parent, mvar);
   parent->o->newline() << mvar.fini ();
   parent->o->newline() << mvar.init ();  
 }
@@ -1466,10 +1532,10 @@ delete_statement_operand_visitor::visit_arrayindex (arrayindex* e)
 {
   vector<tmpvar> idx;
   parent->load_map_indices (e, idx);
-  parent->o->newline() << "if (unlikely (c->last_error)) goto out;";
+
   {
     mapvar mvar = parent->getmap (e->referent, e->tok);
-    varlock guard (*parent, mvar);
+    varlock_w guard (*parent, mvar);
     parent->o->newline() << mvar.seek (idx) << ";";
     parent->o->newline() << mvar.del () << ";";
   }
@@ -1666,11 +1732,9 @@ c_unparser::visit_array_in (array_in* e)
 
   tmpvar res = gensym (pe_long);
 
-  o->newline() << "if (unlikely (c->last_error)) goto out;";
-
-  {
+  { // block used to control varlock_r lifespan
     mapvar mvar = getmap (e->operand->referent, e->tok);
-    varlock guard (*this, mvar);
+    varlock_r guard (*this, mvar);
     o->newline() << mvar.seek (idx) << ";";
     c_assign (res, mvar.exists(), e->tok);
   }
@@ -1918,7 +1982,7 @@ c_unparser_assignment::visit_symbol (symbol *e)
 
   {
     var lvar = parent->getvar (e->referent, e->tok);
-    varlock guard (*parent, lvar);
+    varlock_w guard (*parent, lvar);
     c_assignop (res, lvar, rval, e->tok);     
   }
 
@@ -2002,11 +2066,9 @@ c_unparser::visit_arrayindex (arrayindex* e)
   // reentrancy issues that pop up with nested expressions:
   // e.g. a[a[c]=5] could deadlock
   
-  o->newline() << "if (unlikely (c->last_error)) goto out;";
-  
-  {
+  { // block used to control varlock_r lifespan
     mapvar mvar = getmap (e->referent, e->tok);
-    varlock guard (*this, mvar);
+    varlock_r guard (*this, mvar);
     o->newline() << mvar.seek (idx) << ";";
     c_assign (res, mvar.get(), e->tok);
   }
@@ -2067,8 +2129,8 @@ c_unparser_assignment::visit_arrayindex (arrayindex *e)
   // thusly:
   // ({ tmp0=(idx0); ... tmpN=(idxN); rvar=(rhs); lvar; res;
   //    rvar = ...;
-  //    lock (array);n
-  //    lvar = get (array,idx0...N);
+  //    lock (array);
+  //    lvar = get (array,idx0...N); // if necessary
   //    assignop (res, lvar, rvar);
   //    set (array, idx0...N, lvar);
   //    unlock (array);
@@ -2078,15 +2140,14 @@ c_unparser_assignment::visit_arrayindex (arrayindex *e)
   // reentrancy issues that pop up with nested expressions:
   // e.g. ++a[a[c]=5] could deadlock
   
-  o->newline() << "if (unlikely (c->last_error)) goto out;";
-  
   prepare_rvalue (op, rvar, e->tok);
   
-  {
+  { // block used to control varlock_w lifespan
     mapvar mvar = parent->getmap (e->referent, e->tok);
-    varlock guard (*parent, mvar);
+    varlock_w guard (*parent, mvar);
     o->newline() << mvar.seek (idx) << ";";
-    parent->c_assign (lvar, mvar.get(), e->tok);
+    if (op != "=") // don't bother fetch slot if we will just overwrite it
+      parent->c_assign (lvar, mvar.get(), e->tok);
     c_assignop (res, lvar, rvar, e->tok); 
     o->newline() << mvar.set (lvar) << ";";
   }
@@ -2143,9 +2204,8 @@ c_unparser::visit_functioncall (functioncall* e)
   o->newline();
   o->newline() << "if (unlikely (c->nesting+2 >= MAXNESTING)) {";
   o->newline(1) << "c->last_error = \"MAXNESTING exceeded\";";
-  o->newline() << "goto out;";
-  o->newline(-1) << "}";
-  o->newline();
+  o->newline(-1) << "} else {";
+  o->indent(1);
 
   // copy in actual arguments
   for (unsigned i=0; i<e->args.size(); i++)
@@ -2167,7 +2227,14 @@ c_unparser::visit_functioncall (functioncall* e)
   o->newline() << "c->nesting ++;";
   o->newline() << "function_" << c_varname (r->name) << " (c);";
   o->newline() << "c->nesting --;";
-  
+
+  // reset last_error to NULL if it was set to "" by return()
+  o->newline() << "if (c->last_error && ! c->last_error[0])";
+  o->newline(1) << "c->last_error = 0;";
+  o->indent(-1);
+
+  o->newline(-1) << "}";
+
   // return result from retvalue slot
   
   if (r->type == pe_unknown)