Merge #814

814: Set TREE_ADDRESSABLE when we need to borrow any expression r=philberty a=philberty

GCC requires VAR_DECL's and PARAM_DECL's to be marked with TREE_ADDRESSABLE
when the declaration will be used in borrow's ('&' getting the address).
This takes into account the implicit addresses when we do autoderef in
method resolution/operator-overloading.

If it is not set we end up in cases like this:

```c
i32 main ()
{
  i32 a.1;
  i32 D.86;
  i32 a;

  a = 1;
  a.1 = a; // this is wrong
  <i32 as AddAssign>::add_assign (&a.1, 2);
  D.86 = 0;
  return D.86;
}
```

You can see GCC will automatically make a copy of the VAR_DECL resulting bad code-generation.

Fixes #804


Co-authored-by: Philip Herron <philip.herron@embecosm.com>

9 files changed, 344 insertions, 24 deletions

diff --git a/gcc/rust/Make-lang.in b/gcc/rust/Make-lang.in
index f0e9bc3..6aaa0fc 100644
--- a/gcc/rust/Make-lang.in
+++ b/gcc/rust/Make-lang.in
@@ -86,6 +86,7 @@ GRS_OBJS = \
     rust/rust-hir-const-fold.o \
     rust/rust-hir-type-check-type.o \
     rust/rust-hir-type-check-struct.o \
+    rust/rust-hir-address-taken.o \
     rust/rust-substitution-mapper.o \
     rust/rust-lint-marklive.o \
     rust/rust-hir-type-check-path.o \
diff --git a/gcc/rust/backend/rust-compile-fnparam.h b/gcc/rust/backend/rust-compile-fnparam.h
index 629f0f5..126b49f 100644
--- a/gcc/rust/backend/rust-compile-fnparam.h
+++ b/gcc/rust/backend/rust-compile-fnparam.h
@@ -20,6 +20,7 @@
 #define RUST_COMPILE_FNPARAM
 
 #include "rust-compile-base.h"
+#include "rust-hir-address-taken.h"
 
 namespace Rust {
 namespace Compile {
@@ -33,9 +34,9 @@ public:
 			     HIR::FunctionParam *param, tree decl_type,
 			     Location locus)
   {
-    CompileFnParam compiler (ctx, fndecl, decl_type, locus);
+    CompileFnParam compiler (ctx, fndecl, decl_type, locus, *param);
     param->get_param_name ()->accept_vis (compiler);
-    return compiler.translated;
+    return compiler.compiled_param;
   }
 
   void visit (HIR::IdentifierPattern &pattern) override
@@ -43,23 +44,31 @@ public:
     if (!pattern.is_mut ())
       decl_type = ctx->get_backend ()->immutable_type (decl_type);
 
-    translated
+    bool address_taken = false;
+    address_taken_context->lookup_addess_taken (
+      param.get_mappings ().get_hirid (), &address_taken);
+
+    compiled_param
       = ctx->get_backend ()->parameter_variable (fndecl, pattern.variable_ident,
-						 decl_type,
-						 false /* address_taken */,
+						 decl_type, address_taken,
 						 locus);
   }
 
 private:
-  CompileFnParam (Context *ctx, tree fndecl, tree decl_type, Location locus)
+  CompileFnParam (Context *ctx, tree fndecl, tree decl_type, Location locus,
+		  const HIR::FunctionParam &param)
     : HIRCompileBase (ctx), fndecl (fndecl), decl_type (decl_type),
-      locus (locus), translated (nullptr)
+      locus (locus), param (param),
+      compiled_param (ctx->get_backend ()->error_variable ()),
+      address_taken_context (Resolver::AddressTakenContext::get ())
   {}
 
   tree fndecl;
   tree decl_type;
   Location locus;
-  ::Bvariable *translated;
+  const HIR::FunctionParam &param;
+  Bvariable *compiled_param;
+  const Resolver::AddressTakenContext *address_taken_context;
 };
 
 class CompileSelfParam : public HIRCompileBase
@@ -74,9 +83,13 @@ public:
     if (is_immutable)
       decl_type = ctx->get_backend ()->immutable_type (decl_type);
 
+    const auto &address_taken_context = Resolver::AddressTakenContext::get ();
+    bool address_taken = false;
+    address_taken_context->lookup_addess_taken (
+      self.get_mappings ().get_hirid (), &address_taken);
+
     return ctx->get_backend ()->parameter_variable (fndecl, "self", decl_type,
-						    false /* address_taken */,
-						    locus);
+						    address_taken, locus);
   }
 };
 
diff --git a/gcc/rust/backend/rust-compile-var-decl.h b/gcc/rust/backend/rust-compile-var-decl.h
index a964fa2..097f5b8 100644
--- a/gcc/rust/backend/rust-compile-var-decl.h
+++ b/gcc/rust/backend/rust-compile-var-decl.h
@@ -20,6 +20,7 @@
 #define RUST_COMPILE_VAR_DECL
 
 #include "rust-compile-base.h"
+#include "rust-hir-address-taken.h"
 
 namespace Rust {
 namespace Compile {
@@ -33,10 +34,9 @@ public:
   {
     CompileVarDecl compiler (ctx, fndecl);
     stmt->accept_vis (compiler);
-    rust_assert (compiler.translated != nullptr);
     ctx->insert_var_decl (stmt->get_mappings ().get_hirid (),
-			  compiler.translated);
-    return compiler.translated;
+			  compiler.compiled_variable);
+    return compiler.compiled_variable;
   }
 
   void visit (HIR::LetStmt &stmt) override
@@ -47,6 +47,8 @@ public:
 					      &resolved_type);
     rust_assert (ok);
 
+    address_taken_context->lookup_addess_taken (
+      stmt.get_mappings ().get_hirid (), &address_taken);
     translated_type = TyTyResolveCompile::compile (ctx, resolved_type);
     stmt.get_pattern ()->accept_vis (*this);
   }
@@ -56,22 +58,27 @@ public:
     if (!pattern.is_mut ())
       translated_type = ctx->get_backend ()->immutable_type (translated_type);
 
-    translated
+    compiled_variable
       = ctx->get_backend ()->local_variable (fndecl, pattern.variable_ident,
 					     translated_type, NULL /*decl_var*/,
-					     false /*address_taken*/, locus);
+					     address_taken, locus);
   }
 
 private:
   CompileVarDecl (Context *ctx, tree fndecl)
-    : HIRCompileBase (ctx), fndecl (fndecl), translated_type (nullptr),
-      translated (nullptr)
+    : HIRCompileBase (ctx), fndecl (fndecl),
+      translated_type (ctx->get_backend ()->error_type ()),
+      compiled_variable (ctx->get_backend ()->error_variable ()),
+      address_taken (false),
+      address_taken_context (Resolver::AddressTakenContext::get ())
   {}
 
   tree fndecl;
   tree translated_type;
   Location locus;
-  ::Bvariable *translated;
+  Bvariable *compiled_variable;
+  bool address_taken;
+  const Resolver::AddressTakenContext *address_taken_context;
 };
 
 } // namespace Compile
diff --git a/gcc/rust/hir/tree/rust-hir-item.h b/gcc/rust/hir/tree/rust-hir-item.h
index 0dd7ac8..99a3c48 100644
--- a/gcc/rust/hir/tree/rust-hir-item.h
+++ b/gcc/rust/hir/tree/rust-hir-item.h
@@ -560,7 +560,7 @@ public:
 
   Type *get_type () { return type.get (); }
 
-  Analysis::NodeMapping &get_mappings () { return mappings; }
+  const Analysis::NodeMapping &get_mappings () const { return mappings; }
 };
 
 // Visibility of item - if the item has it, then it is some form of public
diff --git a/gcc/rust/typecheck/rust-autoderef.h b/gcc/rust/typecheck/rust-autoderef.h
index db34755..ef3827d 100644
--- a/gcc/rust/typecheck/rust-autoderef.h
+++ b/gcc/rust/typecheck/rust-autoderef.h
@@ -73,32 +73,51 @@ class Adjuster
 public:
   Adjuster (const TyTy::BaseType *ty) : base (ty) {}
 
-  TyTy::BaseType *adjust_type (std::vector<Adjustment> adjustments)
+  static bool needs_address (const std::vector<Adjustment> &adjustments)
+  {
+    for (auto &adjustment : adjustments)
+      {
+	switch (adjustment.get_type ())
+	  {
+	  case Adjustment::AdjustmentType::IMM_REF:
+	  case Adjustment::AdjustmentType::MUT_REF:
+	    return true;
+
+	  default:
+	    break;
+	  }
+      }
+
+    return false;
+  }
+
+  TyTy::BaseType *adjust_type (const std::vector<Adjustment> &adjustments)
   {
     TyTy::BaseType *ty = base->clone ();
     for (auto &adjustment : adjustments)
       {
 	switch (adjustment.get_type ())
 	  {
-	  case Resolver::Adjustment::AdjustmentType::IMM_REF:
+	  case Adjustment::AdjustmentType::IMM_REF:
 	    ty = new TyTy::ReferenceType (ty->get_ref (),
 					  TyTy::TyVar (ty->get_ref ()),
 					  Mutability::Imm);
 	    break;
 
-	  case Resolver::Adjustment::AdjustmentType::MUT_REF:
+	  case Adjustment::AdjustmentType::MUT_REF:
 	    ty = new TyTy::ReferenceType (ty->get_ref (),
 					  TyTy::TyVar (ty->get_ref ()),
 					  Mutability::Mut);
 	    break;
 
-	  case Resolver::Adjustment::AdjustmentType::DEREF_REF:
+	  case Adjustment::AdjustmentType::DEREF_REF:
 	    // FIXME this really needs to support deref lang-item operator
 	    // overloads
 	    rust_assert (ty->get_kind () == TyTy::TypeKind::REF);
 	    const TyTy::ReferenceType *rr
 	      = static_cast<const TyTy::ReferenceType *> (ty);
 	    ty = rr->get_base ();
+
 	    break;
 	  }
       }
diff --git a/gcc/rust/typecheck/rust-hir-address-taken.cc b/gcc/rust/typecheck/rust-hir-address-taken.cc
new file mode 100644
index 0000000..ae77763
--- /dev/null
+++ b/gcc/rust/typecheck/rust-hir-address-taken.cc
@@ -0,0 +1,65 @@
+// Copyright (C) 2020-2021 Free Software Foundation, Inc.
+
+// This file is part of GCC.
+
+// GCC is free software; you can redistribute it and/or modify it under
+// the terms of the GNU General Public License as published by the Free
+// Software Foundation; either version 3, or (at your option) any later
+// version.
+
+// GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+// WARRANTY; without even the implied warranty of MERCHANTABILITY or
+// FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+// for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with GCC; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+#include "rust-hir-address-taken.h"
+#include "rust-hir-full.h"
+
+namespace Rust {
+namespace Resolver {
+
+AddressTakenContext *
+AddressTakenContext::get ()
+{
+  static AddressTakenContext *instance;
+  if (instance == nullptr)
+    instance = new AddressTakenContext ();
+
+  return instance;
+}
+
+AddressTakenContext::~AddressTakenContext () {}
+
+bool
+AddressTakenContext::lookup_addess_taken (HirId id, bool *address_taken) const
+{
+  const auto &it = ctx.find (id);
+  if (it == ctx.end ())
+    return false;
+
+  *address_taken = it->second;
+  return true;
+}
+
+void
+AddressTakenContext::insert_address_taken (HirId id, bool address_taken)
+{
+  const auto &it = ctx.find (id);
+  if (it != ctx.end ())
+    {
+      // assert that we never change a true result to a negative
+      if (it->second == true)
+	{
+	  rust_assert (address_taken != false);
+	}
+    }
+
+  ctx[id] = address_taken;
+}
+
+} // namespace Resolver
+} // namespace Rust
diff --git a/gcc/rust/typecheck/rust-hir-address-taken.h b/gcc/rust/typecheck/rust-hir-address-taken.h
new file mode 100644
index 0000000..19cbdef
--- /dev/null
+++ b/gcc/rust/typecheck/rust-hir-address-taken.h
@@ -0,0 +1,159 @@
+// Copyright (C) 2020-2021 Free Software Foundation, Inc.
+
+// This file is part of GCC.
+
+// GCC is free software; you can redistribute it and/or modify it under
+// the terms of the GNU General Public License as published by the Free
+// Software Foundation; either version 3, or (at your option) any later
+// version.
+
+// GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+// WARRANTY; without even the implied warranty of MERCHANTABILITY or
+// FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+// for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with GCC; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+#ifndef RUST_HIR_ADDRESS_TAKEN
+#define RUST_HIR_ADDRESS_TAKEN
+
+#include "rust-hir-type-check-base.h"
+
+namespace Rust {
+namespace Resolver {
+
+class AddressTakenContext
+{
+public:
+  static AddressTakenContext *get ();
+
+  ~AddressTakenContext ();
+
+  bool lookup_addess_taken (HirId id, bool *address_taken) const;
+
+  void insert_address_taken (HirId id, bool address_taken);
+
+private:
+  std::map<HirId, bool> ctx;
+};
+
+class AddressTakenResolver : public TypeCheckBase
+{
+  using Rust::Resolver::TypeCheckBase::visit;
+
+public:
+  static void SetAddressTaken (HIR::Expr &expr)
+  {
+    AddressTakenResolver resolver;
+    expr.accept_vis (resolver);
+  }
+
+  void visit (HIR::IdentifierExpr &expr) override
+  {
+    NodeId ast_node_id = expr.get_mappings ().get_nodeid ();
+    NodeId ref_node_id = UNKNOWN_NODEID;
+    if (resolver->lookup_resolved_name (ast_node_id, &ref_node_id))
+      {
+	// these ref_node_ids will resolve to a pattern declaration but we are
+	// interested in the definition that this refers to get the parent id
+	Definition def;
+	if (!resolver->lookup_definition (ref_node_id, &def))
+	  {
+	    rust_error_at (expr.get_locus (),
+			   "unknown reference for resolved name");
+	    return;
+	  }
+	ref_node_id = def.parent;
+      }
+
+    if (ref_node_id == UNKNOWN_NODEID)
+      return;
+
+    // node back to HIR
+    HirId ref = UNKNOWN_HIRID;
+    bool reverse_lookup
+      = mappings->lookup_node_to_hir (expr.get_mappings ().get_crate_num (),
+				      ref_node_id, &ref);
+    rust_assert (reverse_lookup);
+    context->insert_address_taken (ref, true);
+  }
+
+  void visit (HIR::PathInExpression &expr) override
+  {
+    NodeId ast_node_id = expr.get_mappings ().get_nodeid ();
+    NodeId ref_node_id = UNKNOWN_NODEID;
+    if (resolver->lookup_resolved_name (ast_node_id, &ref_node_id))
+      {
+	// these ref_node_ids will resolve to a pattern declaration but we are
+	// interested in the definition that this refers to get the parent id
+	Definition def;
+	if (!resolver->lookup_definition (ref_node_id, &def))
+	  {
+	    rust_error_at (expr.get_locus (),
+			   "unknown reference for resolved name");
+	    return;
+	  }
+	ref_node_id = def.parent;
+      }
+
+    if (ref_node_id == UNKNOWN_NODEID)
+      return;
+
+    // node back to HIR
+    HirId ref = UNKNOWN_HIRID;
+    bool reverse_lookup
+      = mappings->lookup_node_to_hir (expr.get_mappings ().get_crate_num (),
+				      ref_node_id, &ref);
+    rust_assert (reverse_lookup);
+    context->insert_address_taken (ref, true);
+  }
+
+  void visit (HIR::QualifiedPathInExpression &expr) override
+  {
+    NodeId ast_node_id = expr.get_mappings ().get_nodeid ();
+    NodeId ref_node_id = UNKNOWN_NODEID;
+    if (resolver->lookup_resolved_name (ast_node_id, &ref_node_id))
+      {
+	// these ref_node_ids will resolve to a pattern declaration but we are
+	// interested in the definition that this refers to get the parent id
+	Definition def;
+	if (!resolver->lookup_definition (ref_node_id, &def))
+	  {
+	    rust_error_at (expr.get_locus (),
+			   "unknown reference for resolved name");
+	    return;
+	  }
+	ref_node_id = def.parent;
+      }
+
+    if (ref_node_id == UNKNOWN_NODEID)
+      return;
+
+    // node back to HIR
+    HirId ref = UNKNOWN_HIRID;
+    bool reverse_lookup
+      = mappings->lookup_node_to_hir (expr.get_mappings ().get_crate_num (),
+				      ref_node_id, &ref);
+    rust_assert (reverse_lookup);
+    context->insert_address_taken (ref, true);
+  }
+
+  void visit (HIR::DereferenceExpr &expr) override
+  {
+    expr.get_expr ()->accept_vis (*this);
+  }
+
+private:
+  AddressTakenResolver ()
+    : TypeCheckBase (), context (AddressTakenContext::get ())
+  {}
+
+  AddressTakenContext *context;
+};
+
+} // namespace Resolver
+} // namespace Rust
+
+#endif // RUST_HIR_ADDRESS_TAKEN
diff --git a/gcc/rust/typecheck/rust-hir-type-check-expr.h b/gcc/rust/typecheck/rust-hir-type-check-expr.h
index 9eef755..fb1cd97 100644
--- a/gcc/rust/typecheck/rust-hir-type-check-expr.h
+++ b/gcc/rust/typecheck/rust-hir-type-check-expr.h
@@ -31,6 +31,7 @@
 #include "rust-hir-trait-resolve.h"
 #include "rust-hir-type-bounds.h"
 #include "rust-hir-dot-operator.h"
+#include "rust-hir-address-taken.h"
 
 namespace Rust {
 namespace Resolver {
@@ -313,6 +314,10 @@ public:
     Adjuster adj (receiver_tyty);
     TyTy::BaseType *adjusted_self = adj.adjust_type (adjustments);
 
+    // mark the required tree addressable
+    if (Adjuster::needs_address (adjustments))
+      AddressTakenResolver::SetAddressTaken (*expr.get_receiver ().get ());
+
     // store the adjustments for code-generation to know what to do
     context->insert_autoderef_mappings (expr.get_mappings ().get_hirid (),
 					std::move (adjustments));
@@ -1191,11 +1196,18 @@ public:
     TyTy::BaseType *resolved_base
       = TypeCheckExpr::Resolve (expr.get_expr ().get (), false);
 
-    // FIXME double_reference
+    if (expr.get_is_double_borrow ())
+      {
+	// FIXME double_reference
+	gcc_unreachable ();
+      }
 
     infered = new TyTy::ReferenceType (expr.get_mappings ().get_hirid (),
 				       TyTy::TyVar (resolved_base->get_ref ()),
 				       expr.get_mut ());
+
+    // mark the borrowed as address_taken
+    AddressTakenResolver::SetAddressTaken (*expr.get_expr ().get ());
   }
 
   void visit (HIR::DereferenceExpr &expr) override
@@ -1277,6 +1289,10 @@ protected:
     PathProbeCandidate *resolved_candidate
       = MethodResolution::Select (candidates, lhs, adjustments);
 
+    // mark the required tree addressable
+    if (Adjuster::needs_address (adjustments))
+      AddressTakenResolver::SetAddressTaken (*expr.get_expr ().get ());
+
     // is this the case we are recursive
     // handle the case where we are within the impl block for this lang_item
     // otherwise we end up with a recursive operator overload such as the i32
diff --git a/gcc/testsuite/rust/execute/torture/operator_overload_6.rs b/gcc/testsuite/rust/execute/torture/operator_overload_6.rs
new file mode 100644
index 0000000..f6ad5c4
--- /dev/null
+++ b/gcc/testsuite/rust/execute/torture/operator_overload_6.rs
@@ -0,0 +1,40 @@
+/* { dg-output "add_assign\n3\n" } */
+extern "C" {
+    fn printf(s: *const i8, ...);
+}
+
+#[lang = "add_assign"]
+pub trait AddAssign<Rhs = Self> {
+    fn add_assign(&mut self, rhs: Rhs);
+    // { dg-warning "unused name .self." "" { target *-*-* } .-1 }
+    // { dg-warning "unused name .rhs." "" { target *-*-* } .-2 }
+    // { dg-warning "unused name .AddAssign::add_assign." "" { target *-*-* } .-3 }
+}
+
+impl AddAssign for i32 {
+    fn add_assign(&mut self, other: i32) {
+        unsafe {
+            let a = "add_assign\n\0";
+            let b = a as *const str;
+            let c = b as *const i8;
+
+            printf(c);
+        }
+        *self += other
+    }
+}
+
+fn main() -> i32 {
+    let mut res = 1;
+    res += 2;
+
+    unsafe {
+        let a = "%i\n\0";
+        let b = a as *const str;
+        let c = b as *const i8;
+
+        printf(c, res);
+    }
+
+    0
+}