]> gcc.gnu.org Git - gcc.git/commitdiff
compiler: create temporaries for heap variables
authorIan Lance Taylor <iant@golang.org>
Thu, 11 Mar 2021 03:38:21 +0000 (19:38 -0800)
committerIan Lance Taylor <iant@golang.org>
Thu, 11 Mar 2021 23:48:10 +0000 (15:48 -0800)
The compiler generally doesn't create a temporary for an expression
that is a variable, because it's normally valid to simply reload the
value from the variable.  However, if the variable is in the heap,
then loading the value is a pointer indirection.  The process of
creating GCC IR can cause the variable load and the pointer
indirection to be split, such that the second evaluation only does the
pointer indirection.  If there are conditionals in between the two
uses, this can cause the second use to load the pointer from an
uninitialized register.

Avoid this by introducing a new Expression method that returns whether
it is safe to evaluate an expression multiple times, and use it
everywhere.

The test case is https://golang.org/cl/300789.

Fixes golang/go#44383

Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/300809

gcc/go/gofrontend/MERGE
gcc/go/gofrontend/expressions.cc
gcc/go/gofrontend/expressions.h
gcc/go/gofrontend/gogo.cc
gcc/go/gofrontend/statements.cc
gcc/go/gofrontend/wb.cc

index 5b45f03a26eb108e14c55b4acc937b2d4634d0a0..58c881a787293eb50213326b0eb039d112742fd1 100644 (file)
@@ -1,4 +1,4 @@
-93380a9126e76b71fa208e62c31c7914084c0e37
+bf35249a7c752836741b1cab43a312f87916fcb0
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
index 17b4cfd2c197343ffce51843206f3d586d5e998b..101cbe7ac316688eeb572c286cf25e67afaa01f4 100644 (file)
@@ -526,7 +526,7 @@ Expression::convert_interface_to_interface(Type *lhs_type, Expression* rhs,
   // method table.
 
   // We are going to evaluate RHS multiple times.
-  go_assert(rhs->is_variable());
+  go_assert(rhs->is_multi_eval_safe());
 
   // Get the type descriptor for the right hand side.  This will be
   // NULL for a nil interface.
@@ -569,7 +569,7 @@ Expression::convert_interface_to_type(Gogo* gogo, Type *lhs_type, Expression* rh
                                       Location location)
 {
   // We are going to evaluate RHS multiple times.
-  go_assert(rhs->is_variable());
+  go_assert(rhs->is_multi_eval_safe());
 
   // Build an expression to check that the type is valid.  It will
   // panic with an appropriate runtime type error if the type is not
@@ -707,8 +707,8 @@ Expression::check_bounds(Expression* val, Operator op, Expression* bound,
                         Statement_inserter* inserter,
                         Location loc)
 {
-  go_assert(val->is_variable() || val->is_constant());
-  go_assert(bound->is_variable() || bound->is_constant());
+  go_assert(val->is_multi_eval_safe());
+  go_assert(bound->is_multi_eval_safe());
 
   Type* int_type = Type::lookup_integer_type("int");
   int int_type_size = int_type->integer_type()->bits();
@@ -3976,7 +3976,7 @@ Type_conversion_expression::do_flatten(Gogo*, Named_object*,
   if (((this->type()->is_string_type()
         && this->expr_->type()->is_slice_type())
        || this->expr_->type()->interface_type() != NULL)
-      && !this->expr_->is_variable())
+      && !this->expr_->is_multi_eval_safe())
     {
       Temporary_statement* temp =
           Statement::make_temporary(NULL, this->expr_, this->location());
@@ -4264,7 +4264,7 @@ Type_conversion_expression::do_get_backend(Translate_context* context)
       Array_type* a = expr_type->array_type();
       Type* e = a->element_type()->forwarded();
       go_assert(e->integer_type() != NULL);
-      go_assert(this->expr_->is_variable());
+      go_assert(this->expr_->is_multi_eval_safe());
 
       Expression* buf;
       if (this->no_escape_ && !this->no_copy_)
@@ -4711,7 +4711,7 @@ Unary_expression::do_flatten(Gogo* gogo, Named_object*,
 
   Location location = this->location();
   if (this->op_ == OPERATOR_MULT
-      && !this->expr_->is_variable())
+      && !this->expr_->is_multi_eval_safe())
     {
       go_assert(this->expr_->type()->points_to() != NULL);
       switch (this->requires_nil_check(gogo))
@@ -4731,7 +4731,7 @@ Unary_expression::do_flatten(Gogo* gogo, Named_object*,
         }
     }
 
-  if (this->create_temp_ && !this->expr_->is_variable())
+  if (this->create_temp_ && !this->expr_->is_multi_eval_safe())
     {
       Temporary_statement* temp =
           Statement::make_temporary(NULL, this->expr_, location);
@@ -5326,7 +5326,7 @@ Unary_expression::do_get_backend(Translate_context* context)
           bexpr = gogo->backend()->var_expression(decl, loc);
         }
 
-      go_assert(!this->create_temp_ || this->expr_->is_variable());
+      go_assert(!this->create_temp_ || this->expr_->is_multi_eval_safe());
       ret = gogo->backend()->address_expression(bexpr, loc);
       break;
 
@@ -5347,7 +5347,7 @@ Unary_expression::do_get_backend(Translate_context* context)
               }
             case NIL_CHECK_NEEDED:
               {
-                go_assert(this->expr_->is_variable());
+                go_assert(this->expr_->is_multi_eval_safe());
 
                 // If we're nil-checking the result of a set-and-use-temporary
                 // expression, then pick out the target temp and use that
@@ -6496,13 +6496,13 @@ Binary_expression::do_flatten(Gogo* gogo, Named_object*,
          && (gogo->check_divide_by_zero() || gogo->check_divide_overflow()))
       || is_string_op)
     {
-      if (!this->left_->is_variable() && !this->left_->is_constant())
+      if (!this->left_->is_multi_eval_safe())
         {
           temp = Statement::make_temporary(NULL, this->left_, loc);
           inserter->insert(temp);
           this->left_ = Expression::make_temporary_reference(temp, loc);
         }
-      if (!this->right_->is_variable() && !this->right_->is_constant())
+      if (!this->right_->is_multi_eval_safe())
         {
           temp =
               Statement::make_temporary(NULL, this->right_, loc);
@@ -7478,8 +7478,8 @@ Expression::comparison(Translate_context* context, Type* result_type,
 
   if (left_type->is_string_type() && right_type->is_string_type())
     {
-      go_assert(left->is_variable() || left->is_constant());
-      go_assert(right->is_variable() || right->is_constant());
+      go_assert(left->is_multi_eval_safe());
+      go_assert(right->is_multi_eval_safe());
 
       if (op == OPERATOR_EQEQ || op == OPERATOR_NOTEQ)
        {
@@ -8078,7 +8078,7 @@ Bound_method_expression::do_flatten(Gogo* gogo, Named_object*,
   // we are going to do nil checks below, but it's easy enough to
   // always do it.
   Expression* expr = this->expr_;
-  if (!expr->is_variable())
+  if (!expr->is_multi_eval_safe())
     {
       Temporary_statement* etemp = Statement::make_temporary(NULL, expr, loc);
       inserter->insert(etemp);
@@ -8419,7 +8419,7 @@ Builtin_call_expression::do_lower(Gogo*, Named_object* function,
           pa != this->args()->end();
           ++pa)
        {
-         if (!(*pa)->is_variable() && !(*pa)->is_constant())
+         if (!(*pa)->is_multi_eval_safe())
            {
              Temporary_statement* temp =
                Statement::make_temporary(NULL, *pa, loc);
@@ -8470,7 +8470,7 @@ Builtin_call_expression::do_flatten(Gogo* gogo, Named_object* function,
                Expression* zero = Expression::make_integer_ul(0, NULL, loc);
                *pa = Expression::make_slice_value(at, nil, zero, zero, loc);
              }
-           if (!(*pa)->is_variable())
+           if (!(*pa)->is_multi_eval_safe())
              {
                Temporary_statement* temp =
                   Statement::make_temporary(NULL, *pa, loc);
@@ -8484,8 +8484,8 @@ Builtin_call_expression::do_flatten(Gogo* gogo, Named_object* function,
         go_assert(args != NULL && args->size() == 2);
         Expression* arg1 = args->front();
         Expression* arg2 = args->back();
-        go_assert(arg1->is_variable());
-        go_assert(arg2->is_variable());
+       go_assert(arg1->is_multi_eval_safe());
+       go_assert(arg2->is_multi_eval_safe());
         bool arg2_is_string = arg2->type()->is_string_type();
 
         Expression* ret;
@@ -8573,7 +8573,8 @@ Builtin_call_expression::do_flatten(Gogo* gogo, Named_object* function,
           pa != this->args()->end();
           ++pa)
        {
-         if (!(*pa)->is_variable() && (*pa)->type()->interface_type() != NULL)
+         if (!(*pa)->is_multi_eval_safe()
+             && (*pa)->type()->interface_type() != NULL)
            {
              Temporary_statement* temp =
                Statement::make_temporary(NULL, *pa, loc);
@@ -8587,7 +8588,7 @@ Builtin_call_expression::do_flatten(Gogo* gogo, Named_object* function,
     case BUILTIN_CAP:
       {
        Expression_list::iterator pa = this->args()->begin();
-       if (!(*pa)->is_variable()
+       if (!(*pa)->is_multi_eval_safe()
            && ((*pa)->type()->map_type() != NULL
                || (*pa)->type()->channel_type() != NULL))
          {
@@ -9024,7 +9025,7 @@ Builtin_call_expression::flatten_append(Gogo* gogo, Named_object* function,
       Expression_list::const_iterator pa = args->begin();
       for (++pa; pa != args->end(); ++pa)
        {
-         if ((*pa)->is_variable())
+         if ((*pa)->is_multi_eval_safe())
            add->push_back(*pa);
          else
            {
@@ -11235,7 +11236,7 @@ Call_expression::do_flatten(Gogo* gogo, Named_object*,
            {
              Location loc = (*pa)->location();
              Expression* arg = *pa;
-             if (!arg->is_variable())
+             if (!arg->is_multi_eval_safe())
                {
                  Temporary_statement *temp =
                    Statement::make_temporary(NULL, arg, loc);
@@ -11457,7 +11458,7 @@ Call_expression::intrinsify(Gogo* gogo,
                && this->args_ != NULL && this->args_->size() == 1)
         {
           Expression* arg = this->args_->front();
-          if (!arg->is_variable())
+          if (!arg->is_multi_eval_safe())
             {
               Temporary_statement* ts = Statement::make_temporary(uint32_type, arg, loc);
               inserter->insert(ts);
@@ -11476,7 +11477,7 @@ Call_expression::intrinsify(Gogo* gogo,
                && this->args_ != NULL && this->args_->size() == 1)
         {
           Expression* arg = this->args_->front();
-          if (!arg->is_variable())
+          if (!arg->is_multi_eval_safe())
             {
               Temporary_statement* ts = Statement::make_temporary(uint64_type, arg, loc);
               inserter->insert(ts);
@@ -11526,7 +11527,7 @@ Call_expression::intrinsify(Gogo* gogo,
                && this->args_ != NULL && this->args_->size() == 1)
         {
           Expression* arg = this->args_->front();
-          if (!arg->is_variable())
+          if (!arg->is_multi_eval_safe())
             {
               Temporary_statement* ts = Statement::make_temporary(uint32_type, arg, loc);
               inserter->insert(ts);
@@ -11549,7 +11550,7 @@ Call_expression::intrinsify(Gogo* gogo,
                && this->args_ != NULL && this->args_->size() == 1)
         {
           Expression* arg = this->args_->front();
-          if (!arg->is_variable())
+          if (!arg->is_multi_eval_safe())
             {
               Temporary_statement* ts = Statement::make_temporary(uint64_type, arg, loc);
               inserter->insert(ts);
@@ -13087,14 +13088,14 @@ Array_index_expression::do_flatten(Gogo* gogo, Named_object*,
     }
 
   Temporary_statement* temp;
-  if (array_type->is_slice_type() && !array->is_variable())
+  if (array_type->is_slice_type() && !array->is_multi_eval_safe())
     {
       temp = Statement::make_temporary(NULL, array, loc);
       inserter->insert(temp);
       this->array_ = Expression::make_temporary_reference(temp, loc);
       array = this->array_;
     }
-  if (!start->is_variable() && !start->is_constant())
+  if (!start->is_multi_eval_safe())
     {
       temp = Statement::make_temporary(NULL, start, loc);
       inserter->insert(temp);
@@ -13103,15 +13104,14 @@ Array_index_expression::do_flatten(Gogo* gogo, Named_object*,
     }
   if (end != NULL
       && !end->is_nil_expression()
-      && !end->is_variable()
-      && !end->is_constant())
+      && !end->is_multi_eval_safe())
     {
       temp = Statement::make_temporary(NULL, end, loc);
       inserter->insert(temp);
       this->end_ = Expression::make_temporary_reference(temp, loc);
       end = this->end_;
     }
-  if (cap != NULL && !cap->is_variable() && !cap->is_constant())
+  if (cap != NULL && !cap->is_multi_eval_safe())
     {
       temp = Statement::make_temporary(NULL, cap, loc);
       inserter->insert(temp);
@@ -13270,7 +13270,8 @@ Array_index_expression::do_get_backend(Translate_context* context)
       go_assert(this->array_->type()->is_error());
       return context->backend()->error_expression();
     }
-  go_assert(!array_type->is_slice_type() || this->array_->is_variable());
+  go_assert(!array_type->is_slice_type()
+           || this->array_->is_multi_eval_safe());
 
   Location loc = this->location();
   Gogo* gogo = context->gogo();
@@ -13484,14 +13485,14 @@ String_index_expression::do_flatten(Gogo*, Named_object*,
     }
 
   Temporary_statement* temp;
-  if (!string->is_variable())
+  if (!string->is_multi_eval_safe())
     {
       temp = Statement::make_temporary(NULL, string, loc);
       inserter->insert(temp);
       this->string_ = Expression::make_temporary_reference(temp, loc);
       string = this->string_;
     }
-  if (!start->is_variable())
+  if (!start->is_multi_eval_safe())
     {
       temp = Statement::make_temporary(NULL, start, loc);
       inserter->insert(temp);
@@ -13500,7 +13501,7 @@ String_index_expression::do_flatten(Gogo*, Named_object*,
     }
   if (end != NULL
       && !end->is_nil_expression()
-      && !end->is_variable())
+      && !end->is_multi_eval_safe())
     {
       temp = Statement::make_temporary(NULL, end, loc);
       inserter->insert(temp);
@@ -13659,8 +13660,8 @@ String_index_expression::do_get_backend(Translate_context* context)
       return context->backend()->error_expression();
     }
 
-  go_assert(this->string_->is_variable());
-  go_assert(this->start_->is_variable());
+  go_assert(this->string_->is_multi_eval_safe());
+  go_assert(this->start_->is_multi_eval_safe());
 
   Expression* start = Expression::make_cast(int_type, this->start_, loc);
   Bfunction* bfn = context->function()->func_value()->get_decl();
@@ -13685,7 +13686,7 @@ String_index_expression::do_get_backend(Translate_context* context)
     end = length;
   else
     {
-      go_assert(this->end_->is_variable());
+      go_assert(this->end_->is_multi_eval_safe());
       end = Expression::make_cast(int_type, this->end_, loc);
     }
 
@@ -13815,7 +13816,7 @@ Map_index_expression::do_flatten(Gogo* gogo, Named_object*,
                           NULL))
     {
       if (this->index_->type()->interface_type() != NULL
-         && !this->index_->is_variable())
+         && !this->index_->is_multi_eval_safe())
        {
          Temporary_statement* temp =
            Statement::make_temporary(NULL, this->index_, loc);
@@ -13826,7 +13827,7 @@ Map_index_expression::do_flatten(Gogo* gogo, Named_object*,
                                                        this->index_, loc);
     }
 
-  if (!this->index_->is_variable())
+  if (!this->index_->is_multi_eval_safe())
     {
       Temporary_statement* temp = Statement::make_temporary(NULL, this->index_,
                                                             loc);
@@ -13839,7 +13840,7 @@ Map_index_expression::do_flatten(Gogo* gogo, Named_object*,
   if (this->value_pointer_->is_error_expression()
       || this->value_pointer_->type()->is_error_type())
     return Expression::make_error(loc);
-  if (!this->value_pointer_->is_variable())
+  if (!this->value_pointer_->is_multi_eval_safe())
     {
       Temporary_statement* temp =
        Statement::make_temporary(NULL, this->value_pointer_, loc);
@@ -13923,7 +13924,7 @@ Map_index_expression::do_get_backend(Translate_context* context)
     }
 
   go_assert(this->value_pointer_ != NULL
-            && this->value_pointer_->is_variable());
+            && this->value_pointer_->is_multi_eval_safe());
 
   Expression* val = Expression::make_dereference(this->value_pointer_,
                                                  NIL_CHECK_NOT_NEEDED,
@@ -14268,7 +14269,7 @@ Interface_field_reference_expression::do_flatten(Gogo*, Named_object*,
       return Expression::make_error(this->location());
     }
 
-  if (!this->expr_->is_variable())
+  if (!this->expr_->is_multi_eval_safe())
     {
       Temporary_statement* temp =
        Statement::make_temporary(NULL, this->expr_, this->location());
@@ -15165,7 +15166,7 @@ Struct_construction_expression::do_flatten(Gogo*, Named_object*,
               go_assert(saw_errors());
               return Expression::make_error(loc);
             }
-         if (!(*pv)->is_variable())
+         if (!(*pv)->is_multi_eval_safe())
            {
              Temporary_statement* temp =
                Statement::make_temporary(NULL, *pv, loc);
@@ -15471,7 +15472,7 @@ Array_construction_expression::do_flatten(Gogo*, Named_object*,
               go_assert(saw_errors());
               return Expression::make_error(loc);
             }
-         if (!(*pv)->is_variable())
+         if (!(*pv)->is_multi_eval_safe())
            {
              Temporary_statement* temp =
                Statement::make_temporary(NULL, *pv, loc);
@@ -15914,7 +15915,8 @@ Map_construction_expression::do_flatten(Gogo* gogo, Named_object*,
               go_assert(saw_errors());
               return Expression::make_error(loc);
             }
-         if (key->type()->interface_type() != NULL && !key->is_variable())
+         if (key->type()->interface_type() != NULL
+             && !key->is_multi_eval_safe())
            {
              Temporary_statement* temp =
                Statement::make_temporary(NULL, key, loc);
@@ -15930,7 +15932,8 @@ Map_construction_expression::do_flatten(Gogo* gogo, Named_object*,
               go_assert(saw_errors());
               return Expression::make_error(loc);
             }
-         if (val->type()->interface_type() != NULL && !val->is_variable())
+         if (val->type()->interface_type() != NULL
+             && !val->is_multi_eval_safe())
            {
              Temporary_statement* temp =
                Statement::make_temporary(NULL, val, loc);
@@ -17037,6 +17040,41 @@ Expression::is_local_variable() const
          || (no->is_variable() && !no->var_value()->is_global()));
 }
 
+// Return true if multiple evaluations are OK.
+
+bool
+Expression::is_multi_eval_safe()
+{
+  switch (this->classification_)
+    {
+    case EXPRESSION_VAR_REFERENCE:
+      {
+       // A variable is a simple reference if not stored in the heap.
+       const Named_object* no = this->var_expression()->named_object();
+       if (no->is_variable())
+         return !no->var_value()->is_in_heap();
+       else if (no->is_result_variable())
+         return !no->result_var_value()->is_in_heap();
+       else
+         go_unreachable();
+      }
+
+    case EXPRESSION_TEMPORARY_REFERENCE:
+      return true;
+
+    default:
+      break;
+    }
+
+  if (!this->is_constant())
+    return false;
+
+  // Only numeric and boolean constants are really multi-evaluation
+  // safe.  We don't want multiple copies of string constants.
+  Type* type = this->type();
+  return type->is_numeric_type() || type->is_boolean_type();
+}
+
 const Named_object*
 Expression::named_constant() const
 {
@@ -17070,7 +17108,7 @@ Type_guard_expression::do_flatten(Gogo*, Named_object*,
       return Expression::make_error(this->location());
     }
 
-  if (!this->expr_->is_variable())
+  if (!this->expr_->is_multi_eval_safe())
     {
       Temporary_statement* temp = Statement::make_temporary(NULL, this->expr_,
                                                             this->location());
index 712f6870211088f4fd275a560a1776285fdc01b8..e3747cc65d767e73ea7981c7b4fc3d7bc3c435e0 100644 (file)
@@ -916,6 +916,11 @@ class Expression
   bool
   is_local_variable() const;
 
+  // Return true if multiple evaluations of this expression are OK.
+  // This is true for simple variable references and constants.
+  bool
+  is_multi_eval_safe();
+
   // Return true if two expressions refer to the same variable or
   // struct field.
   static bool
index 9d4150eff7ca600df7961a62a22555e8f1a00e40..93b54fd8c118e722e897f5e94160592e14079f44 100644 (file)
@@ -7579,7 +7579,7 @@ Variable::flatten_init_expression(Gogo* gogo, Named_object* function,
                                  Type::COMPARE_ERRORS | Type::COMPARE_TAGS,
                                  NULL)
          && this->init_->type()->interface_type() != NULL
-         && !this->init_->is_variable())
+         && !this->init_->is_multi_eval_safe())
        {
          Temporary_statement* temp =
            Statement::make_temporary(NULL, this->init_, this->location_);
index da0e0843a735eab5272a8b6155cdf6ece44eec84..7ad7339bda8711d3dee4edb04bc980c89ab26b47 100644 (file)
@@ -588,7 +588,7 @@ Temporary_statement::do_flatten(Gogo*, Named_object*, Block*,
                              Type::COMPARE_ERRORS | Type::COMPARE_TAGS,
                              NULL)
       && this->init_->type()->interface_type() != NULL
-      && !this->init_->is_variable())
+      && !this->init_->is_multi_eval_safe())
     {
       Temporary_statement *temp =
        Statement::make_temporary(NULL, this->init_, this->location());
@@ -1125,7 +1125,7 @@ Assignment_statement::do_flatten(Gogo*, Named_object*, Block*,
                              Type::COMPARE_ERRORS | Type::COMPARE_TAGS,
                              NULL)
       && this->rhs_->type()->interface_type() != NULL
-      && !this->rhs_->is_variable())
+      && !this->rhs_->is_multi_eval_safe())
     {
       Temporary_statement* temp =
        Statement::make_temporary(NULL, this->rhs_, this->location());
@@ -5116,7 +5116,7 @@ Send_statement::do_flatten(Gogo*, Named_object*, Block*,
                           Type::COMPARE_ERRORS | Type::COMPARE_TAGS,
                           NULL)
       && this->val_->type()->interface_type() != NULL
-      && !this->val_->is_variable())
+      && !this->val_->is_multi_eval_safe())
     {
       Temporary_statement* temp =
        Statement::make_temporary(NULL, this->val_, this->location());
index ac1f0a1eff999c7f6de9fe19e9b2fd33b82f9aff..104c5db0b9bc6d3152ed9ef610b3e85c2106733c 100644 (file)
@@ -883,7 +883,7 @@ Gogo::assign_with_write_barrier(Function* function, Block* enclosing,
                           Type::COMPARE_ERRORS | Type::COMPARE_TAGS,
                           NULL)
       && rhs->type()->interface_type() != NULL
-      && !rhs->is_variable())
+      && !rhs->is_multi_eval_safe())
     {
       // May need a temporary for interface conversion.
       Temporary_statement* temp = Statement::make_temporary(NULL, rhs, loc);
@@ -892,7 +892,7 @@ Gogo::assign_with_write_barrier(Function* function, Block* enclosing,
     }
   rhs = Expression::convert_for_assignment(this, type, rhs, loc);
   Temporary_statement* rhs_temp = NULL;
-  if (!rhs->is_variable() && !rhs->is_constant())
+  if (!rhs->is_multi_eval_safe())
     {
       rhs_temp = Statement::make_temporary(NULL, rhs, loc);
       inserter->insert(rhs_temp);
This page took 0.113781 seconds and 5 git commands to generate.