Merge #1427

1427: unsafe: Check for unsafe function/method calls r=CohenArthur a=CohenArthur

Addresses #1411 

Co-authored-by: Arthur Cohen <arthur.cohen@embecosm.com>

7 files changed, 144 insertions, 19 deletions

diff --git a/gcc/rust/backend/rust-compile-resolve-path.cc b/gcc/rust/backend/rust-compile-resolve-path.cc
index 95d8841..b5bfa3c 100644
--- a/gcc/rust/backend/rust-compile-resolve-path.cc
+++ b/gcc/rust/backend/rust-compile-resolve-path.cc
@@ -159,8 +159,9 @@ HIRCompileBase::query_compile (HirId ref, TyTy::BaseType *lookup,
 			       Location expr_locus, bool is_qualified_path)
 {
   HIR::Item *resolved_item = ctx->get_mappings ()->lookup_hir_item (ref);
+  HirId parent_block;
   HIR::ExternalItem *resolved_extern_item
-    = ctx->get_mappings ()->lookup_hir_extern_item (ref);
+    = ctx->get_mappings ()->lookup_hir_extern_item (ref, &parent_block);
   bool is_hir_item = resolved_item != nullptr;
   bool is_hir_extern_item = resolved_extern_item != nullptr;
   if (is_hir_item)
diff --git a/gcc/rust/checks/errors/rust-unsafe-checker.cc b/gcc/rust/checks/errors/rust-unsafe-checker.cc
index 0d1e0e9..174901f0 100644
--- a/gcc/rust/checks/errors/rust-unsafe-checker.cc
+++ b/gcc/rust/checks/errors/rust-unsafe-checker.cc
@@ -69,7 +69,10 @@ UnsafeChecker::check_use_of_static (HirId node_id, Location locus)
     return;
 
   auto maybe_static_mut = mappings.lookup_hir_item (node_id);
-  auto maybe_extern_static = mappings.lookup_hir_extern_item (node_id);
+
+  HirId extern_block;
+  auto maybe_extern_static
+    = mappings.lookup_hir_extern_item (node_id, &extern_block);
 
   if (maybe_static_mut)
     check_static_mut (maybe_static_mut, locus);
@@ -79,6 +82,52 @@ UnsafeChecker::check_use_of_static (HirId node_id, Location locus)
 			 locus);
 }
 
+static void
+check_unsafe_call (HIR::Function *fn, Location locus, const std::string &kind)
+{
+  if (fn->get_qualifiers ().is_unsafe ())
+    rust_error_at (locus, "call to unsafe %s requires unsafe function or block",
+		   kind.c_str ());
+}
+
+static void
+check_extern_call (HIR::ExternalItem *maybe_fn, HIR::ExternBlock *parent_block,
+		   Location locus)
+{
+  // We have multiple operations to perform here
+  //     1. Is the item an actual function we're calling
+  //     2. Is the block it's defined in an FFI block or an `extern crate` block
+  //
+  // It is not unsafe to call into other crates, so items defined in an `extern
+  // crate` must be callable without being in an unsafe context. On the other
+  // hand, any function defined in a block with a specific ABI (even `extern
+  // "Rust"` blocks) is unsafe to call
+
+  if (maybe_fn->get_extern_kind () == ExternalItem::ExternKind::Function)
+    rust_error_at (locus,
+		   "call to extern function requires unsafe function or block");
+}
+
+void
+UnsafeChecker::check_function_call (HirId node_id, Location locus)
+{
+  if (unsafe_context.is_in_context ())
+    return;
+
+  HirId parent_extern_block;
+  auto maybe_fn = mappings.lookup_hir_item (node_id);
+  auto maybe_extern
+    = mappings.lookup_hir_extern_item (node_id, &parent_extern_block);
+
+  if (maybe_fn && maybe_fn->get_item_kind () == Item::ItemKind::Function)
+    check_unsafe_call (static_cast<Function *> (maybe_fn), locus, "function");
+
+  if (maybe_extern)
+    check_extern_call (static_cast<ExternalItem *> (maybe_extern),
+		       mappings.lookup_hir_extern_block (parent_extern_block),
+		       locus);
+}
+
 void
 UnsafeChecker::visit (IdentifierExpr &ident_expr)
 {
@@ -297,6 +346,28 @@ UnsafeChecker::visit (StructExprStructBase &expr)
 void
 UnsafeChecker::visit (CallExpr &expr)
 {
+  auto fn = expr.get_fnexpr ();
+  if (!fn)
+    return;
+
+  NodeId ast_node_id = fn->get_mappings ().get_nodeid ();
+  NodeId ref_node_id;
+  HirId definition_id;
+
+  // There are no unsafe types, and functions are defined in the name resolver.
+  // If we can't find the name, then we're dealing with a type and should return
+  // early.
+  if (!resolver.lookup_resolved_name (ast_node_id, &ref_node_id))
+    return;
+
+  rust_assert (mappings.lookup_node_to_hir (ref_node_id, &definition_id));
+
+  // At this point we have the function's HIR Id. There are two checks we
+  // must perform:
+  //     1. The function is an unsafe one
+  //     2. The function is an extern one
+  check_function_call (definition_id, expr.get_locus ());
+
   if (expr.has_params ())
     for (auto &arg : expr.get_arguments ())
       arg->accept_vis (*this);
@@ -304,7 +375,23 @@ UnsafeChecker::visit (CallExpr &expr)
 
 void
 UnsafeChecker::visit (MethodCallExpr &expr)
-{}
+{
+  TyTy::BaseType *method_type;
+  context.lookup_type (expr.get_method_name ().get_mappings ().get_hirid (),
+		       &method_type);
+
+  auto fn = *static_cast<TyTy::FnType *> (method_type);
+  auto method = mappings.lookup_hir_implitem (fn.get_ref (), nullptr);
+
+  if (!unsafe_context.is_in_context () && method)
+    check_unsafe_call (static_cast<Function *> (method), expr.get_locus (),
+		       "method");
+
+  expr.get_receiver ()->accept_vis (*this);
+
+  for (auto &arg : expr.get_arguments ())
+    arg->accept_vis (*this);
+}
 
 void
 UnsafeChecker::visit (FieldAccessExpr &expr)
diff --git a/gcc/rust/checks/errors/rust-unsafe-checker.h b/gcc/rust/checks/errors/rust-unsafe-checker.h
index b9d06ef..087bdb7 100644
--- a/gcc/rust/checks/errors/rust-unsafe-checker.h
+++ b/gcc/rust/checks/errors/rust-unsafe-checker.h
@@ -40,7 +40,14 @@ private:
    */
   void check_use_of_static (HirId node_id, Location locus);
 
+  /**
+   * Check if a call to an unsafe or external function is outside of an unsafe
+   * context
+   */
+  void check_function_call (HirId node_id, Location locus);
+
   StackedContexts<HirId> unsafe_context;
+
   Resolver::TypeCheckContext &context;
   Resolver::Resolver &resolver;
   Analysis::Mappings &mappings;
diff --git a/gcc/rust/hir/rust-ast-lower-base.cc b/gcc/rust/hir/rust-ast-lower-base.cc
index cae4428..a674617 100644
--- a/gcc/rust/hir/rust-ast-lower-base.cc
+++ b/gcc/rust/hir/rust-ast-lower-base.cc
@@ -1038,6 +1038,11 @@ ASTLoweringBase::lower_extern_block (AST::ExternBlock &extern_block)
 {
   HIR::Visibility vis = translate_visibility (extern_block.get_visibility ());
 
+  auto crate_num = mappings->get_current_crate ();
+  Analysis::NodeMapping mapping (crate_num, extern_block.get_node_id (),
+				 mappings->get_next_hir_id (crate_num),
+				 mappings->get_next_localdef_id (crate_num));
+
   std::vector<std::unique_ptr<HIR::ExternalItem>> extern_items;
   for (auto &item : extern_block.get_extern_items ())
     {
@@ -1045,7 +1050,7 @@ ASTLoweringBase::lower_extern_block (AST::ExternBlock &extern_block)
 	continue;
 
       HIR::ExternalItem *lowered
-	= ASTLoweringExternItem::translate (item.get ());
+	= ASTLoweringExternItem::translate (item.get (), mapping.get_hirid ());
       extern_items.push_back (std::unique_ptr<HIR::ExternalItem> (lowered));
     }
 
@@ -1058,17 +1063,14 @@ ASTLoweringBase::lower_extern_block (AST::ExternBlock &extern_block)
 	rust_error_at (extern_block.get_locus (), "unknown ABI option");
     }
 
-  auto crate_num = mappings->get_current_crate ();
-  Analysis::NodeMapping mapping (crate_num, extern_block.get_node_id (),
-				 mappings->get_next_hir_id (crate_num),
-				 mappings->get_next_localdef_id (crate_num));
-
   HIR::ExternBlock *hir_extern_block
     = new HIR::ExternBlock (mapping, abi, std::move (extern_items),
 			    std::move (vis), extern_block.get_inner_attrs (),
 			    extern_block.get_outer_attrs (),
 			    extern_block.get_locus ());
 
+  mappings->insert_hir_extern_block (hir_extern_block);
+
   return hir_extern_block;
 }
 
diff --git a/gcc/rust/hir/rust-ast-lower-extern.h b/gcc/rust/hir/rust-ast-lower-extern.h
index 28d160b..eeb59c9 100644
--- a/gcc/rust/hir/rust-ast-lower-extern.h
+++ b/gcc/rust/hir/rust-ast-lower-extern.h
@@ -31,13 +31,15 @@ class ASTLoweringExternItem : public ASTLoweringBase
   using Rust::HIR::ASTLoweringBase::visit;
 
 public:
-  static HIR::ExternalItem *translate (AST::ExternalItem *item)
+  static HIR::ExternalItem *translate (AST::ExternalItem *item,
+				       HirId parent_hirid)
   {
     ASTLoweringExternItem resolver;
     item->accept_vis (resolver);
 
     rust_assert (resolver.translated != nullptr);
-    resolver.mappings->insert_hir_extern_item (resolver.translated);
+    resolver.mappings->insert_hir_extern_item (resolver.translated,
+					       parent_hirid);
     resolver.mappings->insert_location (
       resolver.translated->get_mappings ().get_hirid (),
       resolver.translated->get_locus ());
diff --git a/gcc/rust/util/rust-hir-map.cc b/gcc/rust/util/rust-hir-map.cc
index c7bf182..6a6deeb 100644
--- a/gcc/rust/util/rust-hir-map.cc
+++ b/gcc/rust/util/rust-hir-map.cc
@@ -371,23 +371,45 @@ Mappings::lookup_hir_trait_item (HirId id)
 }
 
 void
-Mappings::insert_hir_extern_item (HIR::ExternalItem *item)
+Mappings::insert_hir_extern_block (HIR::ExternBlock *block)
+{
+  auto id = block->get_mappings ().get_hirid ();
+  rust_assert (lookup_hir_extern_block (id) == nullptr);
+
+  hirExternBlockMappings[id] = block;
+  insert_node_to_hir (block->get_mappings ().get_nodeid (), id);
+}
+
+HIR::ExternBlock *
+Mappings::lookup_hir_extern_block (HirId id)
+{
+  auto it = hirExternBlockMappings.find (id);
+  if (it == hirExternBlockMappings.end ())
+    return nullptr;
+
+  return it->second;
+}
+
+void
+Mappings::insert_hir_extern_item (HIR::ExternalItem *item, HirId parent_block)
 {
   auto id = item->get_mappings ().get_hirid ();
-  rust_assert (lookup_hir_extern_item (id) == nullptr);
+  rust_assert (lookup_hir_extern_item (id, nullptr) == nullptr);
 
-  hirExternItemMappings[id] = item;
+  hirExternItemMappings[id] = {item, parent_block};
   insert_node_to_hir (item->get_mappings ().get_nodeid (), id);
 }
 
 HIR::ExternalItem *
-Mappings::lookup_hir_extern_item (HirId id)
+Mappings::lookup_hir_extern_item (HirId id, HirId *parent_block)
 {
   auto it = hirExternItemMappings.find (id);
   if (it == hirExternItemMappings.end ())
     return nullptr;
 
-  return it->second;
+  *parent_block = it->second.second;
+
+  return it->second.first;
 }
 
 void
diff --git a/gcc/rust/util/rust-hir-map.h b/gcc/rust/util/rust-hir-map.h
index c8cebef..98fcfe6 100644
--- a/gcc/rust/util/rust-hir-map.h
+++ b/gcc/rust/util/rust-hir-map.h
@@ -115,8 +115,11 @@ public:
   void insert_hir_trait_item (HIR::TraitItem *item);
   HIR::TraitItem *lookup_hir_trait_item (HirId id);
 
-  void insert_hir_extern_item (HIR::ExternalItem *item);
-  HIR::ExternalItem *lookup_hir_extern_item (HirId id);
+  void insert_hir_extern_block (HIR::ExternBlock *block);
+  HIR::ExternBlock *lookup_hir_extern_block (HirId id);
+
+  void insert_hir_extern_item (HIR::ExternalItem *item, HirId parent_block);
+  HIR::ExternalItem *lookup_hir_extern_item (HirId id, HirId *parent_block);
 
   void insert_hir_impl_block (HIR::ImplBlock *item);
   HIR::ImplBlock *lookup_hir_impl_block (HirId id);
@@ -312,7 +315,8 @@ private:
   std::map<HirId, HIR::ImplBlock *> hirImplItemsToImplMappings;
   std::map<HirId, HIR::ImplBlock *> hirImplBlockMappings;
   std::map<HirId, HIR::TraitItem *> hirTraitItemMappings;
-  std::map<HirId, HIR::ExternalItem *> hirExternItemMappings;
+  std::map<HirId, HIR::ExternBlock *> hirExternBlockMappings;
+  std::map<HirId, std::pair<HIR::ExternalItem *, HirId>> hirExternItemMappings;
   std::map<HirId, HIR::PathExprSegment *> hirPathSegMappings;
   std::map<HirId, HIR::GenericParam *> hirGenericParamMappings;
   std::map<HirId, HIR::Trait *> hirTraitItemsToTraitMappings;