diff mbox series

Go patch committed: Don't use address of temporary for deferred delete

Message ID CAOyqgcW=vngq5j+Ax=pyO8ubeCU41sEajmLh9Bt=C64nQaSGNA@mail.gmail.com
State New
Headers show
Series Go patch committed: Don't use address of temporary for deferred delete | expand

Commit Message

Ian Lance Taylor Sept. 14, 2018, 4:55 p.m. UTC 
This patch changes the Go frontend to nott use the address of a
temporary for a deferred delete.  This corrects the handling of a
deferred delete in a loop, to not use a temporary whose value will, at
deferred execution time, wind up being the last value in the loop.
The test for this is TestDeferDeleteSlow in the 1.11 runtime package.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian
diff mbox series

Patch

Index: gcc/go/gofrontend/MERGE
===================================================================
--- gcc/go/gofrontend/MERGE	(revision 264295)
+++ gcc/go/gofrontend/MERGE	(working copy)
@@ -1,4 +1,4 @@ 
-218c9159635e06e39ae43d0efe1ac1e694fead2e
+3fd61802286c81e5fb672f682d9e661181184d1f
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/expressions.cc
===================================================================
--- gcc/go/gofrontend/expressions.cc	(revision 264259)
+++ gcc/go/gofrontend/expressions.cc	(working copy)
@@ -7409,7 +7409,32 @@  Builtin_call_expression::do_lower(Gogo*,
 								  loc);
 	    Expression* e3 = Expression::make_temporary_reference(key_temp,
 								  loc);
-	    e3 = Expression::make_unary(OPERATOR_AND, e3, loc);
+
+	    // If the call to delete is deferred, and is in a loop,
+	    // then the loop will only have a single instance of the
+	    // temporary variable.  Passing the address of the
+	    // temporary variable here means that the deferred call
+	    // will see the last value in the loop, not the current
+	    // value.  So for this unusual case copy the value into
+	    // the heap.
+	    if (!this->is_deferred())
+	      e3 = Expression::make_unary(OPERATOR_AND, e3, loc);
+	    else
+	      {
+		Expression* a = Expression::make_allocation(mt->key_type(),
+							    loc);
+		Temporary_statement* atemp =
+		  Statement::make_temporary(NULL, a, loc);
+		inserter->insert(atemp);
+
+		a = Expression::make_temporary_reference(atemp, loc);
+		a = Expression::make_dereference(a, NIL_CHECK_NOT_NEEDED, loc);
+		Statement* s = Statement::make_assignment(a, e3, loc);
+		inserter->insert(s);
+
+		e3 = Expression::make_temporary_reference(atemp, loc);
+	      }
+
 	    return Runtime::make_call(Runtime::MAPDELETE, this->location(),
 				      3, e1, e2, e3);
 	  }
@@ -9024,6 +9049,10 @@  Builtin_call_expression::do_copy()
 
   if (this->varargs_are_lowered())
     bce->set_varargs_are_lowered();
+  if (this->is_deferred())
+    bce->set_is_deferred();
+  if (this->is_concurrent())
+    bce->set_is_concurrent();
   return bce;
 }
 
@@ -9606,8 +9635,16 @@  Call_expression::do_lower(Gogo* gogo, Na
 
   // Recognize a call to a builtin function.
   if (fntype->is_builtin())
-    return new Builtin_call_expression(gogo, this->fn_, this->args_,
-				       this->is_varargs_, loc);
+    {
+      Builtin_call_expression* bce =
+	new Builtin_call_expression(gogo, this->fn_, this->args_,
+				    this->is_varargs_, loc);
+      if (this->is_deferred_)
+	bce->set_is_deferred();
+      if (this->is_concurrent_)
+	bce->set_is_concurrent();
+      return bce;
+    }
 
   // If this call returns multiple results, create a temporary
   // variable to hold them.
@@ -10275,6 +10312,10 @@  Call_expression::do_copy()
 
   if (this->varargs_are_lowered_)
     call->set_varargs_are_lowered();
+  if (this->is_deferred_)
+    call->set_is_deferred();
+  if (this->is_concurrent_)
+    call->set_is_concurrent();
   return call;
 }
 
Index: gcc/go/gofrontend/parse.cc
===================================================================
--- gcc/go/gofrontend/parse.cc	(revision 264283)
+++ gcc/go/gofrontend/parse.cc	(working copy)
@@ -4304,9 +4304,15 @@  Parse::go_or_defer_stat()
   this->gogo_->start_block(stat_location);
   Statement* stat;
   if (is_go)
-    stat = Statement::make_go_statement(call_expr, stat_location);
+    {
+      stat = Statement::make_go_statement(call_expr, stat_location);
+      call_expr->set_is_concurrent();
+    }
   else
-    stat = Statement::make_defer_statement(call_expr, stat_location);
+    {
+      stat = Statement::make_defer_statement(call_expr, stat_location);
+      call_expr->set_is_deferred();
+    }
   this->gogo_->add_statement(stat);
   this->gogo_->add_block(this->gogo_->finish_block(stat_location),
 			 stat_location);