commit 3fd61802286c81e5fb672f682d9e661181184d1f
author Ian Lance Taylor <iant@golang.org> Fri Sep 14 07:39:40 2018 -0700
committer Ian Lance Taylor <iant@golang.org> Fri Sep 14 16:54:57 2018 +0000
tree 38ac834f4fa641c59efcb23cf947b70d2509374d
parent 218c9159635e06e39ae43d0efe1ac1e694fead2e

compiler: don't use address of temporary for deferred delete

This CL 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.

Change-Id: I55cbc5e755d7543f119773e50df4696eed5fa982
Reviewed-on: https://go-review.googlesource.com/135358
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

go/expressions.cc

diff --git a/go/expressions.cc b/go/expressions.cc
index a296777..46a1e69 100644
--- a/go/expressions.cc
+++ b/go/expressions.cc
@@ -7409,7 +7409,32 @@
 								  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 @@
 
   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 @@
 
   // 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 @@
 
   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;
 }
 

go/parse.cc

diff --git a/go/parse.cc b/go/parse.cc
index cc901db..6ed4377 100644
--- a/go/parse.cc
+++ b/go/parse.cc
@@ -4304,9 +4304,15 @@
   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);