[vm] Enforce that entry points must be annotated by default.

Changes the default value of the --verify-entry-points flag
to true.

Changes the default value for the check_is_entrypoint argument to
to the Invoke/InvokeGetter/InvokeSetter flags to true. The mirrors
library implementation and calls via vm-service explicitly pass
false for this argument now.

Add annotations as needed, such as annotating classes with
annotated generative constructors. In some cases, the annotations
were more general than needed (e.g., annotating with a no-argument
entry point annotation when only the setter is needed), so make
those annotations more specific.

As this pattern is already common in downstream code, allow
Dart_Invoke on fields as long as the field is annotated for getter
access. (That is, calling Dart_Invoke for a field is equivalent to
retrieving the closure value via Dart_GetField and then calling
Dart_InvokeClosure.)

TEST=vm/cc/DartAPI_MissingEntryPoints
     vm/dart/entrypoints_verification_test

Issue: #50649
Issue: flutter/flutter#118608

Change-Id: Ibb3bf15632ab2958d8791b449af8651d47f871a5
Cq-Include-Trybots: luci.dart.try:vm-aot-linux-product-x64-try,vm-aot-linux-debug-x64-try,vm-aot-mac-release-arm64-try,vm-aot-mac-product-arm64-try,vm-aot-dwarf-linux-product-x64-try
CoreLibraryReviewExempt: adding/editing vm-only pragma annotations
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/363566
Reviewed-by: Martin Kustermann <[email protected]>
Commit-Queue: Tess Strickland <[email protected]>

Author: sstrickl

runtime/bin/entrypoints_verification_test.cc

@@ -4,6 +4,10 @@ The annotation `@pragma("vm:entry-point", ...)` **must** be placed on a class or
 member to indicate that it may be resolved, allocated or invoked directly from
 native or VM code _in AOT mode_.
 
+To reduce the differences between JIT and AOT mode, entry point annotations are
+also checked in JIT mode except for uses of the `dart:mirrors` library and
+debugging uses via the VM service.
+
 ## Background
 
 Dart VM precompiler (AOT compiler) performs whole-program optimizations such as
@@ -88,11 +92,14 @@ three forms may be attached to static fields.
 int foo;
 ```
 
-If the second parameter is missing, `null` or `true, the field is marked for
+If the second parameter is missing, `null` or `true`, the field is marked for
 native access and for non-static fields the corresponding getter and setter in
 the interface of the enclosing class are marked for native invocation. If the
-'get'/'set' parameter is used, only the getter/setter is marked. For static
-fields, the implicit getter is always marked. The third form does not make sense
-for static fields because they do not belong to an interface.
-
-Note that no form of entry-point annotation allows invoking a field.
+"get" or "set" parameter is used, only the getter or setter is marked. For
+static fields, the implicit getter is always marked if the field is marked
+for native access.
+
+A field containing a closure may only be invoked using Dart_Invoke if the
+getter is marked, in which case it is the same as retrieving the closure from
+the field using Dart_GetField and then invoking the closure using
+Dart_InvokeClosure.

runtime/docs/compiler/aot/entry_point_pragma.md

@@ -714,7 +714,7 @@ ObjectPtr IsolateSpawnState::ResolveFunction() {
   // Check whether the root library defines a main function.
   const Library& lib =
       Library::Handle(zone, IG->object_store()->root_library());
-  const String& main = String::Handle(zone, String::New("main"));
+  const String& main = Symbols::main();
   Function& func = Function::Handle(zone, lib.LookupFunctionAllowPrivate(main));
   if (func.IsNull()) {
     // Check whether main is reexported from the root library.

runtime/lib/isolate.cc

@@ -1221,6 +1221,8 @@ DEFINE_NATIVE_ENTRY(TypeVariableMirror_upper_bound, 0, 1) {
   return param.bound();
 }
 
+static constexpr bool kNoStrictEntryPointChecks = false;
+
 DEFINE_NATIVE_ENTRY(InstanceMirror_invoke, 0, 5) {
   // Argument 0 is the mirror, which is unused by the native. It exists
   // because this native is an instance method in order to be polymorphic
@@ -1230,7 +1232,8 @@ DEFINE_NATIVE_ENTRY(InstanceMirror_invoke, 0, 5) {
                                arguments->NativeArgAt(2));
   GET_NON_NULL_NATIVE_ARGUMENT(Array, args, arguments->NativeArgAt(3));
   GET_NON_NULL_NATIVE_ARGUMENT(Array, arg_names, arguments->NativeArgAt(4));
-  RETURN_OR_PROPAGATE(reflectee.Invoke(function_name, args, arg_names));
+  RETURN_OR_PROPAGATE(reflectee.Invoke(function_name, args, arg_names,
+                                       kNoStrictEntryPointChecks));
 }
 
 DEFINE_NATIVE_ENTRY(InstanceMirror_invokeGetter, 0, 3) {
@@ -1239,7 +1242,8 @@ DEFINE_NATIVE_ENTRY(InstanceMirror_invokeGetter, 0, 3) {
   // with its cousins.
   GET_NATIVE_ARGUMENT(Instance, reflectee, arguments->NativeArgAt(1));
   GET_NON_NULL_NATIVE_ARGUMENT(String, getter_name, arguments->NativeArgAt(2));
-  RETURN_OR_PROPAGATE(reflectee.InvokeGetter(getter_name));
+  RETURN_OR_PROPAGATE(
+      reflectee.InvokeGetter(getter_name, kNoStrictEntryPointChecks));
 }
 
 DEFINE_NATIVE_ENTRY(InstanceMirror_invokeSetter, 0, 4) {
@@ -1249,7 +1253,8 @@ DEFINE_NATIVE_ENTRY(InstanceMirror_invokeSetter, 0, 4) {
   GET_NATIVE_ARGUMENT(Instance, reflectee, arguments->NativeArgAt(1));
   GET_NON_NULL_NATIVE_ARGUMENT(String, setter_name, arguments->NativeArgAt(2));
   GET_NATIVE_ARGUMENT(Instance, value, arguments->NativeArgAt(3));
-  RETURN_OR_PROPAGATE(reflectee.InvokeSetter(setter_name, value));
+  RETURN_OR_PROPAGATE(
+      reflectee.InvokeSetter(setter_name, value, kNoStrictEntryPointChecks));
 }
 
 DEFINE_NATIVE_ENTRY(InstanceMirror_computeType, 0, 1) {
@@ -1309,7 +1314,8 @@ DEFINE_NATIVE_ENTRY(ClassMirror_invoke, 0, 5) {
                                arguments->NativeArgAt(2));
   GET_NON_NULL_NATIVE_ARGUMENT(Array, args, arguments->NativeArgAt(3));
   GET_NON_NULL_NATIVE_ARGUMENT(Array, arg_names, arguments->NativeArgAt(4));
-  RETURN_OR_PROPAGATE(klass.Invoke(function_name, args, arg_names));
+  RETURN_OR_PROPAGATE(
+      klass.Invoke(function_name, args, arg_names, kNoStrictEntryPointChecks));
 }
 
 DEFINE_NATIVE_ENTRY(ClassMirror_invokeGetter, 0, 3) {
@@ -1324,7 +1330,8 @@ DEFINE_NATIVE_ENTRY(ClassMirror_invokeGetter, 0, 3) {
     UNREACHABLE();
   }
   GET_NON_NULL_NATIVE_ARGUMENT(String, getter_name, arguments->NativeArgAt(2));
-  RETURN_OR_PROPAGATE(klass.InvokeGetter(getter_name, true));
+  RETURN_OR_PROPAGATE(
+      klass.InvokeGetter(getter_name, kNoStrictEntryPointChecks));
 }
 
 DEFINE_NATIVE_ENTRY(ClassMirror_invokeSetter, 0, 4) {
@@ -1335,7 +1342,8 @@ DEFINE_NATIVE_ENTRY(ClassMirror_invokeSetter, 0, 4) {
   const Class& klass = Class::Handle(ref.GetClassReferent());
   GET_NON_NULL_NATIVE_ARGUMENT(String, setter_name, arguments->NativeArgAt(2));
   GET_NATIVE_ARGUMENT(Instance, value, arguments->NativeArgAt(3));
-  RETURN_OR_PROPAGATE(klass.InvokeSetter(setter_name, value));
+  RETURN_OR_PROPAGATE(
+      klass.InvokeSetter(setter_name, value, kNoStrictEntryPointChecks));
 }
 
 DEFINE_NATIVE_ENTRY(ClassMirror_invokeConstructor, 0, 5) {
@@ -1488,7 +1496,8 @@ DEFINE_NATIVE_ENTRY(LibraryMirror_invoke, 0, 5) {
                                arguments->NativeArgAt(2));
   GET_NON_NULL_NATIVE_ARGUMENT(Array, args, arguments->NativeArgAt(3));
   GET_NON_NULL_NATIVE_ARGUMENT(Array, arg_names, arguments->NativeArgAt(4));
-  RETURN_OR_PROPAGATE(library.Invoke(function_name, args, arg_names));
+  RETURN_OR_PROPAGATE(library.Invoke(function_name, args, arg_names,
+                                     kNoStrictEntryPointChecks));
 }
 
 DEFINE_NATIVE_ENTRY(LibraryMirror_invokeGetter, 0, 3) {
@@ -1498,7 +1507,8 @@ DEFINE_NATIVE_ENTRY(LibraryMirror_invokeGetter, 0, 3) {
   GET_NON_NULL_NATIVE_ARGUMENT(MirrorReference, ref, arguments->NativeArgAt(1));
   const Library& library = Library::Handle(ref.GetLibraryReferent());
   GET_NON_NULL_NATIVE_ARGUMENT(String, getter_name, arguments->NativeArgAt(2));
-  RETURN_OR_PROPAGATE(library.InvokeGetter(getter_name, true));
+  RETURN_OR_PROPAGATE(
+      library.InvokeGetter(getter_name, kNoStrictEntryPointChecks));
 }
 
 DEFINE_NATIVE_ENTRY(LibraryMirror_invokeSetter, 0, 4) {
@@ -1509,7 +1519,8 @@ DEFINE_NATIVE_ENTRY(LibraryMirror_invokeSetter, 0, 4) {
   const Library& library = Library::Handle(ref.GetLibraryReferent());
   GET_NON_NULL_NATIVE_ARGUMENT(String, setter_name, arguments->NativeArgAt(2));
   GET_NATIVE_ARGUMENT(Instance, value, arguments->NativeArgAt(3));
-  RETURN_OR_PROPAGATE(library.InvokeSetter(setter_name, value));
+  RETURN_OR_PROPAGATE(
+      library.InvokeSetter(setter_name, value, kNoStrictEntryPointChecks));
 }
 
 DEFINE_NATIVE_ENTRY(MethodMirror_owner, 0, 2) {

runtime/lib/mirrors.cc

@@ -2,22 +2,20 @@
 // 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.
 //
-// VMOptions=--verify-entry-points=true
 // SharedObjects=entrypoints_verification_test
 
 import 'dart:ffi';
 import './dylib_utils.dart';
 
-main() {
+main(List<String> args) {
   final helper = dlopenPlatformSpecific('entrypoints_verification_test');
   final runTest =
       helper.lookupFunction<Void Function(), void Function()>('RunTests');
   runTest();
-
-  new C();
-  new D();
 }
 
+final void Function() noop = () {};
+
 class C {}
 
 @pragma("vm:entry-point")
@@ -52,16 +50,16 @@ class D {
   @pragma("vm:entry-point", "get")
   static void fn3_get() {}
 
-  void Function()? fld0;
+  void Function()? fld0 = noop;
 
   @pragma("vm:entry-point")
-  void Function()? fld1;
+  void Function()? fld1 = noop;
 
   @pragma("vm:entry-point", "get")
-  void Function()? fld2;
+  void Function()? fld2 = noop;
 
   @pragma("vm:entry-point", "set")
-  void Function()? fld3;
+  void Function()? fld3 = noop;
 }
 
 void fn0() {}
@@ -81,25 +79,25 @@ class E extends D {
 
 @pragma("vm:entry-point")
 class F {
-  static void Function()? fld0;
+  static void Function()? fld0 = noop;
 
   @pragma("vm:entry-point")
-  static void Function()? fld1;
+  static void Function()? fld1 = noop;
 
   @pragma("vm:entry-point", "get")
-  static void Function()? fld2;
+  static void Function()? fld2 = noop;
 
   @pragma("vm:entry-point", "set")
-  static void Function()? fld3;
+  static void Function()? fld3 = noop;
 }
 
-void Function()? fld0;
+void Function()? fld0 = noop;
 
 @pragma("vm:entry-point")
-void Function()? fld1;
+void Function()? fld1 = noop;
 
 @pragma("vm:entry-point", "get")
-void Function()? fld2;
+void Function()? fld2 = noop;
 
 @pragma("vm:entry-point", "set")
-void Function()? fld3;
+void Function()? fld3 = noop;

runtime/tests/vm/dart/entrypoints_verification_test.dart

@@ -176,6 +176,7 @@ base class Class extends NativeFieldWrapperClass1 {
   external int method(int param1, int param2);
 }
 
+@pragma('vm:entry-point', 'call')
 void benchmark(int count) {
   Class c = Class();
   c.init();
@@ -336,7 +337,9 @@ BENCHMARK(FrameLookup) {
           return StackFrame.accessFrame();
         }
       }
+      @pragma('vm:entry-point')
       class StackFrameTest {
+        @pragma('vm:entry-point', 'call')
         static int testMain() {
           First obj = new First();
           return obj.method1(1);
@@ -430,6 +433,7 @@ BENCHMARK(CreateMirrorSystem) {
   const char* kScriptChars =
       "import 'dart:mirrors';\n"
       "\n"
+      "@pragma('vm:entry-point', 'call')\n"
       "void benchmark() {\n"
       "  currentMirrorSystem();\n"
       "}\n";
@@ -535,6 +539,7 @@ BENCHMARK(SimpleMessage) {
 
 BENCHMARK(LargeMap) {
   const char* kScript =
+      "@pragma('vm:entry-point', 'call')\n"
       "makeMap() {\n"
       "  Map m = {};\n"
       "  for (int i = 0; i < 100000; ++i) m[i*13+i*(i>>7)] = i;\n"

runtime/vm/benchmark_test.cc

@@ -738,7 +738,7 @@ void Precompiler::AddRoots() {
     UNREACHABLE();
   }
 
-  const String& name = String::Handle(String::New("main"));
+  const String& name = Symbols::main();
   Function& main = Function::Handle(lib.LookupFunctionAllowPrivate(name));
   if (main.IsNull()) {
     const Object& obj = Object::Handle(lib.LookupReExport(name));
@@ -1381,20 +1381,10 @@ const char* Precompiler::MustRetainFunction(const Function& function) {
   // * Native functions (for LinkNativeCall)
   // * Selector matches a symbol used in Resolver::ResolveDynamic calls
   //   in dart_entry.cc or dart_api_impl.cc.
-  // * _Closure.call (used in async stack handling)
   if (function.is_old_native()) {
     return "native function";
   }
 
-  // Use the same check for _Closure.call as in stack_trace.{h|cc}.
-  const auto& selector = String::Handle(Z, function.name());
-  if (selector.ptr() == Symbols::call().ptr()) {
-    const auto& name = String::Handle(Z, function.QualifiedScrubbedName());
-    if (name.Equals(Symbols::_ClosureCall())) {
-      return "_Closure.call";
-    }
-  }
-
   // We have to retain functions which can be a target of a SwitchableCall
   // at AOT runtime, since the AOT runtime needs to be able to find the
   // function object in the class.

runtime/vm/compiler/aot/precompiler.cc

@@ -81,12 +81,15 @@ TypeParameterPtr GetFunctionTypeParameter(const Function& fun, intptr_t index) {
   return param.ptr();
 }
 
-ObjectPtr Invoke(const Library& lib, const char* name) {
+ObjectPtr Invoke(const Library& lib,
+                 const char* name,
+                 bool check_is_entrypoint) {
   Thread* thread = Thread::Current();
   Dart_Handle api_lib = Api::NewHandle(thread, lib.ptr());
   Dart_Handle result;
   {
     TransitionVMToNative transition(thread);
+    SetFlagScope<bool> sfs(&FLAG_verify_entry_points, check_is_entrypoint);
     result =
         Dart_Invoke(api_lib, NewString(name), /*argc=*/0, /*argv=*/nullptr);
     EXPECT_VALID(result);

runtime/vm/compiler/backend/il_test_helper.cc

@@ -65,7 +65,9 @@ ClassPtr GetClass(const Library& lib, const char* name);
 TypeParameterPtr GetClassTypeParameter(const Class& klass, intptr_t index);
 TypeParameterPtr GetFunctionTypeParameter(const Function& fun, intptr_t index);
 
-ObjectPtr Invoke(const Library& lib, const char* name);
+ObjectPtr Invoke(const Library& lib,
+                 const char* name,
+                 bool check_is_entrypoint = false);
 
 InstructionsPtr BuildInstructions(
     std::function<void(compiler::Assembler* assembler)> fun);

runtime/vm/compiler/backend/il_test_helper.h

@@ -580,7 +580,9 @@ external InspectStack();
 @pragma('vm:never-inline')
 void nop() {}
 
+@pragma('vm:entry-point', 'get')
 int prologueCount = 0;
+@pragma('vm:entry-point', 'get')
 int epilogueCount = 0;
 
 @pragma('vm:never-inline')