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

This is a reland of commit cb9ecbc

This reland only turns on the entry point verification flag by
default in AOT mode. After Flutter tests that use native access
in JIT mode have been appropriately updated, a followup CL will
turn this flag on by default in JIT mode as well.

Original change's description:
> [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]>

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

Change-Id: I24919c32ab4760c7c5435c378879791086256f02
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,flutter-linux-try,vm-linux-debug-x64-try,vm-linux-release-x64-try,vm-appjit-linux-product-x64-try
CoreLibraryReviewExempt: adding/editing vm-only pragma annotations
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/391620
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

@@ -710,7 +710,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

@@ -8,16 +8,15 @@
 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 +51,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 +80,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

@@ -740,7 +740,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));
@@ -1383,20 +1383,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,19 +81,6 @@ TypeParameterPtr GetFunctionTypeParameter(const Function& fun, intptr_t index) {
   return param.ptr();
 }
 
-ObjectPtr Invoke(const Library& lib, const char* name) {
-  Thread* thread = Thread::Current();
-  Dart_Handle api_lib = Api::NewHandle(thread, lib.ptr());
-  Dart_Handle result;
-  {
-    TransitionVMToNative transition(thread);
-    result =
-        Dart_Invoke(api_lib, NewString(name), /*argc=*/0, /*argv=*/nullptr);
-    EXPECT_VALID(result);
-  }
-  return Api::UnwrapHandle(result);
-}
-
 InstructionsPtr BuildInstructions(
     std::function<void(compiler::Assembler* assembler)> fun) {
   auto thread = Thread::Current();

runtime/vm/compiler/backend/il_test_helper.cc

@@ -65,8 +65,6 @@ 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);
-
 InstructionsPtr BuildInstructions(
     std::function<void(compiler::Assembler* assembler)> fun);
 

runtime/vm/compiler/backend/il_test_helper.h

@@ -581,7 +581,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')

runtime/vm/compiler/backend/inliner_test.cc

@@ -1937,6 +1937,7 @@ ISOLATE_UNIT_TEST_CASE(Ffi_StructSinking) {
         external int a;
       }
 
+      @pragma('vm:entry-point')
       int test(int addr) =>
         Pointer<S>.fromAddress(addr)[0].a;
       )";

runtime/vm/compiler/backend/redundancy_elimination_test.cc

@@ -176,13 +176,16 @@ ISOLATE_UNIT_TEST_CASE(RegenerateAllocStubs) {
 
 TEST_CASE(EvalExpression) {
   const char* kScriptChars =
-      "int ten = 2 * 5;              \n"
-      "get dot => '.';               \n"
-      "class A {                     \n"
-      "  var apa = 'Herr Nilsson';   \n"
-      "  calc(x) => '${x*ten}';      \n"
-      "}                             \n"
-      "makeObj() => new A();         \n";
+      R"(
+       int ten = 2 * 5;
+       get dot => '.';
+       class A {
+         var apa = 'Herr Nilsson';
+         calc(x) => '${x*ten}';
+       }
+       @pragma('vm:entry-point', 'call')
+       makeObj() => new A();
+      )";
 
   Dart_Handle lib = TestCase::LoadTestScript(kScriptChars, nullptr);
   Dart_Handle obj_handle =

runtime/vm/compiler_test.cc

@@ -29,6 +29,7 @@ static const char* kCustomIsolateScriptChars =
     import 'dart:isolate';
 
     final RawReceivePort mainPort = new RawReceivePort();
+    @pragma('vm:entry-point', 'get')
     final SendPort mainSendPort = mainPort.sendPort;
 
     @pragma('vm:external-name', 'native_echo')
@@ -55,6 +56,7 @@ static const char* kCustomIsolateScriptChars =
       SendPort spawn();
     }
 
+    @pragma('vm:entry-point', 'call')
     isolateMain() {
        echo('Running isolateMain');
        mainPort.handler = (message) {