[vm] Add Reachability Fence Instruction

The reachability fence keeps a value alive and reachable.

Required for finalizers: https://github.com/dart-lang/sdk/issues/35770

Design: go/dart-ffi-finalizers (See "Premature Cleanup (Single Object)".)

Change-Id: I9742889f0f8d8b15bbcb5dca47f2a4231899dd59
Cq-Include-Trybots: luci.dart.try:vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64-try,app-kernel-linux-debug-x64-try,vm-kernel-linux-debug-ia32-try,vm-kernel-win-debug-x64-try,vm-kernel-win-debug-ia32-try,vm-kernel-precomp-linux-debug-x64-try,vm-dartkb-linux-release-x64-abi-try,vm-kernel-precomp-android-release-arm64-try,vm-kernel-asan-linux-release-x64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-simarm64-try,vm-kernel-precomp-android-release-arm_x64-try,vm-kernel-precomp-obfuscate-linux-release-x64-try,dart-sdk-linux-try,analyzer-analysis-server-linux-try,analyzer-linux-release-try,front-end-linux-release-x64-try,vm-kernel-precomp-win-release-x64-try,vm-kernel-mac-debug-x64-try,vm-kernel-nnbd-linux-debug-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/136188
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>

pkg/vm/lib/target/vm.dart

@@ -385,7 +385,8 @@ class VmTarget extends Target {
   // purposes.
   bool allowPlatformPrivateLibraryAccess(Uri importer, Uri imported) =>
       super.allowPlatformPrivateLibraryAccess(importer, imported) ||
-      importer.path.contains('runtime/tests/vm/dart');
+      importer.path.contains('runtime/tests/vm/dart') ||
+      importer.path.contains('test-lib');
 
   // TODO(sigmund,ahe): limit this to `dart-ext` libraries only (see
   // https://github.com/dart-lang/sdk/issues/29763).

runtime/lib/object.cc

@@ -209,6 +209,10 @@ DEFINE_NATIVE_ENTRY(Internal_unsafeCast, 0, 1) {
   return arguments->NativeArgAt(0);
 }
 
+DEFINE_NATIVE_ENTRY(Internal_reachabilityFence, 0, 1) {
+  return Object::null();
+}
+
 static bool ExtractInterfaceTypeArgs(Zone* zone,
                                      const Class& instance_cls,
                                      const TypeArguments& instance_type_args,

runtime/vm/bootstrap_natives.h

@@ -330,6 +330,7 @@ namespace dart {
   V(GrowableList_setLength, 2)                                                 \
   V(GrowableList_setData, 2)                                                   \
   V(Internal_unsafeCast, 1)                                                    \
+  V(Internal_reachabilityFence, 1)                                             \
   V(Internal_makeListFixedLength, 1)                                           \
   V(Internal_makeFixedListUnmodifiable, 1)                                     \
   V(Internal_inquireIs64Bit, 0)                                                \

runtime/vm/compiler/backend/constant_propagator.cc

@@ -350,6 +350,10 @@ void ConstantPropagator::VisitRedefinition(RedefinitionInstr* instr) {
   }
 }
 
+void ConstantPropagator::VisitReachabilityFence(ReachabilityFenceInstr* instr) {
+  // Nothing to do.
+}
+
 void ConstantPropagator::VisitCheckArrayBound(CheckArrayBoundInstr* instr) {
   // Don't propagate constants through check, since it would eliminate
   // the data dependence between the bound check and the load/store.

runtime/vm/compiler/backend/il.cc

@@ -4064,6 +4064,21 @@ void RedefinitionInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
   UNREACHABLE();
 }
 
+LocationSummary* ReachabilityFenceInstr::MakeLocationSummary(
+    Zone* zone,
+    bool optimizing) const {
+  LocationSummary* summary = new (zone)
+      LocationSummary(zone, 1, 0, LocationSummary::ContainsCall::kNoCall);
+  // Keep the parameter alive and reachable, in any location.
+  summary->set_in(0, Location::Any());
+  return summary;
+}
+
+void ReachabilityFenceInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
+  // No native code, but we rely on the parameter being passed in here so that
+  // it stays alive and reachable.
+}
+
 LocationSummary* ParameterInstr::MakeLocationSummary(Zone* zone,
                                                      bool optimizing) const {
   UNREACHABLE();

runtime/vm/compiler/backend/il.h

@@ -367,6 +367,7 @@ struct InstrAttrs {
   M(CatchBlockEntry, kNoGC)                                                    \
   M(Phi, kNoGC)                                                                \
   M(Redefinition, kNoGC)                                                       \
+  M(ReachabilityFence, kNoGC)                                                  \
   M(Parameter, kNoGC)                                                          \
   M(NativeParameter, kNoGC)                                                    \
   M(LoadIndexedUnsafe, kNoGC)                                                  \
@@ -3296,6 +3297,26 @@ class RedefinitionInstr : public TemplateDefinition<1, NoThrow> {
   DISALLOW_COPY_AND_ASSIGN(RedefinitionInstr);
 };
 
+// Keeps the value alive til after this point.
+//
+// The fence cannot be moved.
+class ReachabilityFenceInstr : public TemplateInstruction<1, NoThrow> {
+ public:
+  explicit ReachabilityFenceInstr(Value* value) { SetInputAt(0, value); }
+
+  DECLARE_INSTRUCTION(ReachabilityFence)
+
+  Value* value() const { return inputs_[0]; }
+
+  virtual bool ComputeCanDeoptimize() const { return false; }
+  virtual bool HasUnknownSideEffects() const { return false; }
+
+  PRINT_OPERANDS_TO_SUPPORT
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ReachabilityFenceInstr);
+};
+
 class ConstraintInstr : public TemplateDefinition<1, NoThrow> {
  public:
   ConstraintInstr(Value* value, Range* constraint)

runtime/vm/compiler/backend/il_printer.cc

@@ -374,6 +374,10 @@ void RedefinitionInstr::PrintOperandsTo(BufferFormatter* f) const {
   }
 }
 
+void ReachabilityFenceInstr::PrintOperandsTo(BufferFormatter* f) const {
+  value()->PrintTo(f);
+}
+
 void Value::PrintTo(BufferFormatter* f) const {
   PrintUse(f, *definition());
 

runtime/vm/compiler/backend/reachability_fence_test.cc

@@ -0,0 +1,205 @@
+// Copyright (c) 2020, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+#include <vector>
+
+#include "vm/compiler/backend/il.h"
+#include "vm/compiler/backend/il_printer.h"
+#include "vm/compiler/backend/il_test_helper.h"
+#include "vm/compiler/call_specializer.h"
+#include "vm/compiler/compiler_pass.h"
+#include "vm/object.h"
+#include "vm/unit_test.h"
+
+namespace dart {
+
+ISOLATE_UNIT_TEST_CASE(ReachabilityFence_Simple) {
+  const char* kScript =
+      R"(
+      import 'dart:_internal' show reachabilityFence;
+
+      int someGlobal = 0;
+
+      class A {
+        int a;
+      }
+
+      void someFunction(int arg) {
+        someGlobal += arg;
+      }
+
+      main() {
+        final object = A()..a = 10;
+        someFunction(object.a);
+        reachabilityFence(object);
+      }
+      )";
+
+  const auto& root_library = Library::Handle(LoadTestScript(kScript));
+
+  Invoke(root_library, "main");
+
+  const auto& function = Function::Handle(GetFunction(root_library, "main"));
+  TestPipeline pipeline(function, CompilerPass::kJIT);
+  FlowGraph* flow_graph = pipeline.RunPasses({});
+  ASSERT(flow_graph != nullptr);
+
+  auto entry = flow_graph->graph_entry()->normal_entry();
+  EXPECT(entry != nullptr);
+
+  //  v2 <- AllocateObject(A <not-aliased>) T{A}
+  //  ...
+  //  [use field of object v2]
+  //  ReachabilityFence(v2)
+  AllocateObjectInstr* allocate_object = nullptr;
+  ReachabilityFenceInstr* fence = nullptr;
+
+  ILMatcher cursor(flow_graph, entry);
+  RELEASE_ASSERT(cursor.TryMatch({
+      kMoveGlob,
+      // Allocate the object.
+      {kMatchAndMoveAllocateObject, &allocate_object},
+      kMoveGlob,
+      // The call.
+      kMatchAndMoveStoreStaticField,
+      // The fence should not be moved before the call.
+      {kMatchAndMoveReachabilityFence, &fence},
+  }));
+
+  EXPECT(fence->value()->definition() == allocate_object);
+}
+
+ISOLATE_UNIT_TEST_CASE(ReachabilityFence_Loop) {
+  const char* kScript =
+      R"(
+      import 'dart:_internal' show reachabilityFence;
+
+      int someGlobal = 0;
+
+      class A {
+        int a;
+      }
+
+      @pragma('vm:never-inline')
+      A makeSomeA() {
+        return A()..a = 10;
+      }
+
+      void someFunction(int arg) {
+        someGlobal += arg;
+      }
+
+      main() {
+        final object = makeSomeA();
+        for(int i = 0; i < 100000; i++) {
+          someFunction(object.a);
+          reachabilityFence(object);
+        }
+      }
+      )";
+
+  const auto& root_library = Library::Handle(LoadTestScript(kScript));
+
+  Invoke(root_library, "main");
+
+  const auto& function = Function::Handle(GetFunction(root_library, "main"));
+  TestPipeline pipeline(function, CompilerPass::kJIT);
+  FlowGraph* flow_graph = pipeline.RunPasses({});
+  ASSERT(flow_graph != nullptr);
+
+  auto entry = flow_graph->graph_entry()->normal_entry();
+  EXPECT(entry != nullptr);
+
+  StaticCallInstr* object = nullptr;
+  LoadFieldInstr* field_load = nullptr;
+  ReachabilityFenceInstr* fence = nullptr;
+
+  ILMatcher cursor(flow_graph, entry);
+  RELEASE_ASSERT(cursor.TryMatch(
+      {
+          // Get the object from some method
+          {kMatchAndMoveStaticCall, &object},
+          // Load the field outside the loop.
+          {kMatchAndMoveLoadField, &field_load},
+          // Go into the loop.
+          kMatchAndMoveBranchTrue,
+          // The fence should not be moved outside of the loop.
+          {kMatchAndMoveReachabilityFence, &fence},
+      },
+      /*insert_before=*/kMoveGlob));
+
+  EXPECT(field_load->instance()->definition() == object);
+  EXPECT(fence->value()->definition() == object);
+}
+
+ISOLATE_UNIT_TEST_CASE(ReachabilityFence_NoCanonicalize) {
+  const char* kScript =
+      R"(
+      import 'dart:_internal' show reachabilityFence;
+
+      int someGlobal = 0;
+
+      class A {
+        int a;
+      }
+
+      @pragma('vm:never-inline')
+      A makeSomeA() {
+        return A()..a = 10;
+      }
+
+      void someFunction(int arg) {
+        someGlobal += arg;
+      }
+
+      main() {
+        final object = makeSomeA();
+        reachabilityFence(object);
+        for(int i = 0; i < 100000; i++) {
+          someFunction(object.a);
+          reachabilityFence(object);
+        }
+        reachabilityFence(object);
+        reachabilityFence(object);
+      }
+      )";
+
+  const auto& root_library = Library::Handle(LoadTestScript(kScript));
+
+  Invoke(root_library, "main");
+
+  const auto& function = Function::Handle(GetFunction(root_library, "main"));
+  TestPipeline pipeline(function, CompilerPass::kJIT);
+  FlowGraph* flow_graph = pipeline.RunPasses({});
+  ASSERT(flow_graph != nullptr);
+
+  auto entry = flow_graph->graph_entry()->normal_entry();
+  EXPECT(entry != nullptr);
+
+  StaticCallInstr* object = nullptr;
+  ReachabilityFenceInstr* fence1 = nullptr;
+  ReachabilityFenceInstr* fence2 = nullptr;
+  ReachabilityFenceInstr* fence3 = nullptr;
+  ReachabilityFenceInstr* fence4 = nullptr;
+
+  ILMatcher cursor(flow_graph, entry);
+  RELEASE_ASSERT(cursor.TryMatch(
+      {
+          {kMatchAndMoveStaticCall, &object},
+          {kMatchAndMoveReachabilityFence, &fence1},
+          kMatchAndMoveBranchTrue,
+          {kMatchAndMoveReachabilityFence, &fence2},
+          kMatchAndMoveBranchFalse,
+          {kMatchAndMoveReachabilityFence, &fence3},
+          {kMatchAndMoveReachabilityFence, &fence4},
+      },
+      /*insert_before=*/kMoveGlob));
+
+  EXPECT(fence1->value()->definition() == object);
+  EXPECT(fence2->value()->definition() == object);
+  EXPECT(fence3->value()->definition() == object);
+  EXPECT(fence4->value()->definition() == object);
+}
+
+}  // namespace dart

runtime/vm/compiler/backend/redundancy_elimination.cc

@@ -3690,7 +3690,7 @@ static bool CanEliminateInstruction(Instruction* current,
   ASSERT(current->GetBlock() == block);
   if (MayHaveVisibleEffect(current) || current->CanDeoptimize() ||
       current == block->last_instruction() || current->IsMaterializeObject() ||
-      current->IsCheckStackOverflow()) {
+      current->IsCheckStackOverflow() || current->IsReachabilityFence()) {
     return false;
   }
   return true;

runtime/vm/compiler/compiler_sources.gni

@@ -185,6 +185,7 @@ compiler_sources_tests = [
   "backend/locations_helpers_test.cc",
   "backend/loops_test.cc",
   "backend/range_analysis_test.cc",
+  "backend/reachability_fence_test.cc",
   "backend/redundancy_elimination_test.cc",
   "backend/sexpression_test.cc",
   "backend/slot_test.cc",

runtime/vm/compiler/frontend/base_flow_graph_builder.cc

@@ -555,6 +555,12 @@ Fragment BaseFlowGraphBuilder::RedefinitionWithType(const AbstractType& type) {
   return Fragment(redefinition);
 }
 
+Fragment BaseFlowGraphBuilder::ReachabilityFence() {
+  Fragment instructions;
+  instructions <<= new (Z) ReachabilityFenceInstr(Pop());
+  return instructions;
+}
+
 Fragment BaseFlowGraphBuilder::StoreStaticField(TokenPosition position,
                                                 const Field& field) {
   return Fragment(

runtime/vm/compiler/frontend/base_flow_graph_builder.h

@@ -198,6 +198,7 @@ class BaseFlowGraphBuilder {
                                          StoreInstanceFieldInstr::Kind::kOther);
   Fragment LoadStaticField(const Field& field);
   Fragment RedefinitionWithType(const AbstractType& type);
+  Fragment ReachabilityFence();
   Fragment StoreStaticField(TokenPosition position, const Field& field);
   Fragment StoreIndexed(classid_t class_id);
   // Takes a [class_id] valid for StoreIndexed.

runtime/vm/compiler/frontend/kernel_to_il.cc

@@ -904,6 +904,7 @@ bool FlowGraphBuilder::IsRecognizedMethodForFlowGraph(
     case MethodRecognizer::kLinkedHashMap_getDeletedKeys:
     case MethodRecognizer::kLinkedHashMap_setDeletedKeys:
     case MethodRecognizer::kFfiAbi:
+    case MethodRecognizer::kReachabilityFence:
       return true;
     case MethodRecognizer::kAsyncStackTraceHelper:
       return !FLAG_causal_async_stacks;
@@ -1187,6 +1188,12 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfRecognizedMethod(
       ASSERT(!FLAG_causal_async_stacks);
       body += NullConstant();
       break;
+    case MethodRecognizer::kReachabilityFence:
+      ASSERT(function.NumParameters() == 1);
+      body += LoadLocal(parsed_function_->RawParameterVariable(0));
+      body += ReachabilityFence();
+      body += NullConstant();
+      break;
     case MethodRecognizer::kFfiAbi:
       ASSERT(function.NumParameters() == 0);
       body += IntConstant(static_cast<int64_t>(compiler::ffi::TargetAbi()));
@@ -2886,6 +2893,9 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfFfiTrampoline(
 }
 
 FlowGraph* FlowGraphBuilder::BuildGraphOfFfiNative(const Function& function) {
+  const intptr_t kClosureParameterOffset = 0;
+  const intptr_t kFirstArgumentParameterOffset = kClosureParameterOffset + 1;
+
   graph_entry_ =
       new (Z) GraphEntryInstr(*parsed_function_, Compiler::kNoOSRDeoptId);
 
@@ -2907,13 +2917,15 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfFfiNative(const Function& function) {
 
   // Unbox and push the arguments.
   for (intptr_t i = 0; i < marshaller.num_args(); i++) {
-    body += LoadLocal(parsed_function_->ParameterVariable(i + 1));
+    body += LoadLocal(
+        parsed_function_->ParameterVariable(kFirstArgumentParameterOffset + i));
     body += FfiConvertArgumentToNative(marshaller, i);
   }
 
   // Push the function pointer, which is stored (as Pointer object) in the
   // first slot of the context.
-  body += LoadLocal(parsed_function_->ParameterVariable(0));
+  body +=
+      LoadLocal(parsed_function_->ParameterVariable(kClosureParameterOffset));
   body += LoadNativeField(Slot::Closure_context());
   body += LoadNativeField(Slot::GetContextVariableSlotFor(
       thread_, *MakeImplicitClosureScope(
@@ -2925,6 +2937,14 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfFfiNative(const Function& function) {
   body += ConvertUntaggedToUnboxed(kUnboxedFfiIntPtr);
   body += FfiCall(marshaller);
 
+  for (intptr_t i = 0; i < marshaller.num_args(); i++) {
+    if (marshaller.IsPointer(i)) {
+      body += LoadLocal(parsed_function_->ParameterVariable(
+          kFirstArgumentParameterOffset + i));
+      body += ReachabilityFence();
+    }
+  }
+
   body += FfiConvertArgumentToDart(marshaller, compiler::ffi::kResultIndex);
 
   body += Return(TokenPosition::kNoSource);

runtime/vm/compiler/recognized_methods_list.h

@@ -171,6 +171,7 @@ namespace dart {
   V(::, _storePointer, FfiStorePointer, 0x3c7143a8)                            \
   V(::, _fromAddress, FfiFromAddress, 0x612a64d5)                              \
   V(Pointer, get:address, FfiGetAddress, 0x29a505a1)                           \
+  V(::, reachabilityFence, ReachabilityFence, 0x0)                             \
 
 // List of intrinsics:
 // (class-name, function-name, intrinsification method, fingerprint).

sdk/lib/_internal/vm/lib/internal_patch.dart

@@ -124,3 +124,9 @@ Int32List _growRegExpStack(Int32List stack) {
 //
 // Important: this is unsafe and must be used with care.
 T unsafeCast<T>(Object v) native "Internal_unsafeCast";
+
+// This function can be used to keep an object alive til that point.
+//
+// This is implemented by a recognized method, but in bytecode through a native.
+@pragma('vm:prefer-inline')
+void reachabilityFence(Object object) native "Internal_reachabilityFence";

sdk_nnbd/lib/_internal/vm/lib/internal_patch.dart

@@ -122,3 +122,9 @@ Int32List _growRegExpStack(Int32List stack) {
 //
 // Important: this is unsafe and must be used with care.
 T unsafeCast<T>(Object? v) native "Internal_unsafeCast";
+
+// This function can be used to keep an object alive til that point.
+//
+// This is implemented by a recognized method, but in bytecode through a native.
+@pragma('vm:prefer-inline')
+void reachabilityFence(Object object) native "Internal_reachabilityFence";

tools/VERSION

@@ -35,5 +35,5 @@ MINOR 8
 PATCH 0
 PRERELEASE 0
 PRERELEASE_PATCH 0
-ABI_VERSION 31
-OLDEST_SUPPORTED_ABI_VERSION 31
+ABI_VERSION 32
+OLDEST_SUPPORTED_ABI_VERSION 32