[dyn_modules] Check target of dcall from dynamic modules is valid.

Unlike other calls from Dynamic Modules, dynamic calls cannot be
validated entirely at compile time. While we check that the selector
used matches a selector that was allowed (either because a method with
that selector name was exposed as dynamically callable or because the
selector was allowlisted during bytecode compilation), the compiler
doesn't know statically whether the target of the call is exposed.

In prior changes we modified the annotator to add a pragma indicating
whether a member is dynamically-callable or
implicitly-dynamically-callable. Here we use that information to set a
bit on functions and their corresponding dynamic invocation forwarders,
which is verified by the interpreter to make sure the dynamic call is
still allowed.

TEST=none yet - will be added in subsequent CL (see CL chain)

Bug: b/448095881
Change-Id: I27acb4e690a68e08fe1f1ca94e0d77cc7dc4d11e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/498300
Reviewed-by: Slava Egorov <vegorov@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Auto-Submit: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Sigmund Cherem <sigmund@google.com>

runtime/vm/compiler/aot/precompiler.cc

@@ -835,7 +835,8 @@ void Precompiler::CollectCallbackFields() {
               dispatcher = subcls.GetInvocationDispatcher(
                   field_name, args_desc,
                   UntaggedFunction::kInvokeFieldDispatcher,
-                  /* create_if_absent = */ true);
+                  /* create_if_absent = */ true,
+                  field.is_dynamically_callable());
               if (FLAG_trace_precompiler) {
                 THR_Print("Added invoke-field-dispatcher for %s to %s\n",
                           field_name.ToCString(), subcls.ToCString());
@@ -1329,7 +1330,8 @@ void Precompiler::AddClosureCall(const String& call_selector,
       Function::Handle(Z, cache_class.GetInvocationDispatcher(
                               call_selector, arguments_descriptor,
                               UntaggedFunction::kInvokeFieldDispatcher,
-                              true /* create_if_absent */));
+                              /* create_if_absent = */ true,
+                              /* is_dynamically_callable = */ true));
   AddFunction(dispatcher, RetainReasons::kInvokeFieldDispatcher);
 }
 
@@ -1757,7 +1759,10 @@ void Precompiler::CheckForNewDynamicFunctions() {
             function.kind() == UntaggedFunction::kRegularFunction;
         if (is_getter || is_setter || is_regular) {
           selector2 = Function::CreateDynamicInvocationForwarderName(selector);
-          if (IsSent(selector2)) {
+          bool generate_dynamic_forwarder = false;
+          if (function.is_dynamically_callable()) {
+            generate_dynamic_forwarder = true;
+          } else if (IsSent(selector2)) {
             if (function.kind() == UntaggedFunction::kImplicitGetter ||
                 function.kind() == UntaggedFunction::kImplicitSetter) {
               field = function.accessor_field();
@@ -1765,23 +1770,17 @@ void Precompiler::CheckForNewDynamicFunctions() {
             } else if (!found_metadata) {
               metadata = kernel::ProcedureAttributesOf(function, Z);
             }
-
-            if (is_getter) {
-              if (metadata.getter_called_dynamically) {
+            generate_dynamic_forwarder =
+                is_getter ? metadata.getter_called_dynamically
+                          : metadata.method_or_setter_called_dynamically;
+          }
+          if (generate_dynamic_forwarder) {
             function2 = function.GetDynamicInvocationForwarder(selector2);
-                AddFunction(function2,
-                            RetainReasons::kDynamicInvocationForwarder);
+            ASSERT(function.is_dynamically_callable() ==
+                   function2.is_dynamically_callable());
+            AddFunction(function2, RetainReasons::kDynamicInvocationForwarder);
             functions_called_dynamically_.Insert(function2);
           }
-            } else {
-              if (metadata.method_or_setter_called_dynamically) {
-                function2 = function.GetDynamicInvocationForwarder(selector2);
-                AddFunction(function2,
-                            RetainReasons::kDynamicInvocationForwarder);
-                functions_called_dynamically_.Insert(function2);
-              }
-            }
-          }
         }
       }
     }

runtime/vm/interpreter.cc

@@ -873,7 +873,8 @@ DART_FORCE_INLINE bool Interpreter::InstanceCall(Thread* thread,
                                                  ObjectPtr* top,
                                                  const KBCInstr** pc,
                                                  ObjectPtr** FP,
-                                                 ObjectPtr** SP) {
+                                                 ObjectPtr** SP,
+                                                 bool check_dynamic_call) {
   ObjectPtr null_value = Object::null();
   const intptr_t type_args_len =
       InterpreterHelpers::ArgDescTypeArgsLen(argdesc_);
@@ -906,14 +907,32 @@ DART_FORCE_INLINE bool Interpreter::InstanceCall(Thread* thread,
 
   if (target != Function::null()) {
     lookup_cache_.Insert(receiver_cid, target_name, argdesc_, target);
+
+    if (check_dynamic_call) {
+      // Ensure the function can be called dynamically from a dynamic module.
+      // TODO(b/448095881): don't perform this check repeatedly, consider
+      // splitting the lookup-cache to separately track dynamic calls.
+      Zone* zone = thread->zone();
+      const Function& target_func = Function::Handle(zone, target);
+      if (!target_func.is_dynamically_callable() &&
+          !target_func.is_declared_in_bytecode()) {
+        target = Function::null();
+        top[4] = null_value;
+      }
+    }
+
+    if (target != Function::null()) {
       top[0] = target;
       return Invoke(thread, call_base, top, pc, FP, SP);
     }
+  }
 
-  // The miss handler should only fail to return a function in AOT mode,
-  // in which case we need to call DRT_InvokeNoSuchMethod, which
-  // walks the receiver appropriately in this case.
-#if defined(DART_PRECOMPILED_RUNTIME)
+  // Technically, the miss handler should only fail to return a function in AOT
+  // mode, in which case we need to call DRT_InvokeNoSuchMethod, which walks the
+  // receiver appropriately in this case.
+  //
+  // When a target is found, we may still reach this point in either AOT or JIT
+  // if the member is not dynamically-callable.
 
   // The receiver, name, and argument descriptor are already in the appropriate
   // places on the stack from the previous call.
@@ -955,9 +974,6 @@ DART_FORCE_INLINE bool Interpreter::InstanceCall(Thread* thread,
     **SP = result;
     pp_ = InterpreterHelpers::FrameBytecode(*FP)->untag()->object_pool();
   }
-#else
-  UNREACHABLE();
-#endif
 
   return true;
 }
@@ -2475,8 +2491,15 @@ SwitchDispatchNoSingleStep:
       InterpreterHelpers::IncrementUsageCounter(FrameFunction(FP));
       StringPtr target_name = String::RawCast(LOAD_CONSTANT(kidx));
       argdesc_ = Array::RawCast(LOAD_CONSTANT(kidx + 1));
-      if (!InstanceCall(thread, target_name, call_base, call_top, &pc, &FP,
-                        &SP)) {
+
+#if defined(DART_PRECOMPILED_RUNTIME)
+      bool caller_in_dynamic_module = true;
+#else
+      // TODO(sigmund): track when caller is declared in a dynamic module.
+      bool caller_in_dynamic_module = false;
+#endif
+      if (!InstanceCall(thread, target_name, call_base, call_top, &pc, &FP, &SP,
+                        /*check_dynamic_call=*/caller_in_dynamic_module)) {
         HANDLE_EXCEPTION;
       }
       CHECK_SINGLE_STEPPING;

runtime/vm/interpreter.h

@@ -180,7 +180,8 @@ class Interpreter {
                     ObjectPtr* call_top,
                     const KBCInstr** pc,
                     ObjectPtr** FP,
-                    ObjectPtr** SP);
+                    ObjectPtr** SP,
+                    bool check_dynamic_call = false);
 
   bool CopyParameters(Thread* thread,
                       const KBCInstr** pc,

runtime/vm/kernel_loader.cc

@@ -1442,6 +1442,8 @@ void KernelLoader::FinishClassLoading(const Class& klass,
           NoSanitizeThreadPragma::decode(pragma_bits));
       field.set_has_deeply_immutable_type(
           DeeplyImmutablePragma::decode(pragma_bits));
+      field.set_is_dynamically_callable(
+          DynModuleDynamicallyCallablePragma::decode(pragma_bits));
       ReadInferredType(field, field_offset + library_kernel_offset_);
       CheckForInitializer(field);
       // Static fields with initializers are implicitly late.
@@ -1775,6 +1777,13 @@ void KernelLoader::ReadVMAnnotations(const Library& library,
           *pragma_bits = DynModuleCanBeOverriddenImplicitlyPragma::update(
               true, *pragma_bits);
         }
+        if (constant_reader.IsStringConstant(
+                name_index, "dyn-module:dynamically-callable") ||
+            constant_reader.IsStringConstant(
+                name_index, "dyn-module:implicitly-dynamically-callable")) {
+          *pragma_bits =
+              DynModuleDynamicallyCallablePragma::update(true, *pragma_bits);
+        }
       }
     } else {
       helper_.SkipExpression();
@@ -1835,6 +1844,8 @@ void KernelLoader::LoadProcedure(const Library& library,
                        !native_name.IsNull() || is_ffi_native,  // is_native
                        script_class, procedure_helper.start_position_));
   function.set_has_pragma(HasPragma::decode(pragma_bits));
+  function.set_is_dynamically_callable(
+      DynModuleDynamicallyCallablePragma::decode(pragma_bits));
   function.set_end_token_pos(procedure_helper.end_position_);
   function.set_is_synthetic(procedure_helper.IsNoSuchMethodForwarder() ||
                             procedure_helper.IsMemberSignature() ||
@@ -2070,6 +2081,8 @@ void KernelLoader::GenerateFieldAccessors(const Class& klass,
     getter.SetIsDynamicallyOverridden(
         DynModuleCanBeOverriddenPragma::decode(pragma_bits) ||
         DynModuleCanBeOverriddenImplicitlyPragma::decode(pragma_bits));
+    getter.set_is_dynamically_callable(
+        DynModuleDynamicallyCallablePragma::decode(pragma_bits));
   }
 
   if (needs_setter) {
@@ -2102,6 +2115,8 @@ void KernelLoader::GenerateFieldAccessors(const Class& klass,
     setter.SetIsDynamicallyOverridden(
         DynModuleCanBeOverriddenPragma::decode(pragma_bits) ||
         DynModuleCanBeOverriddenImplicitlyPragma::decode(pragma_bits));
+    setter.set_is_dynamically_callable(
+        DynModuleDynamicallyCallablePragma::decode(pragma_bits));
   }
 }
 

runtime/vm/kernel_loader.h

@@ -243,6 +243,10 @@ class KernelLoader : public ValueObject {
       BitField<uint32_t, bool, DynModuleImplicitlyExtendablePragma::kNextBit>;
   using DynModuleCanBeOverriddenImplicitlyPragma =
       BitField<uint32_t, bool, DynModuleCanBeOverriddenPragma::kNextBit>;
+  using DynModuleDynamicallyCallablePragma =
+      BitField<uint32_t,
+               bool,
+               DynModuleCanBeOverriddenImplicitlyPragma::kNextBit>;
 
   void FinishTopLevelClassLoading(const Class& toplevel_class,
                                   const Library& library,

runtime/vm/object.cc

@@ -3920,7 +3920,8 @@ void Class::AddInvocationDispatcher(const String& target_name,
 FunctionPtr Class::GetInvocationDispatcher(const String& target_name,
                                            const Array& args_desc,
                                            UntaggedFunction::Kind kind,
-                                           bool create_if_absent) const {
+                                           bool create_if_absent,
+                                           bool is_dynamically_callable) const {
   ASSERT(kind == UntaggedFunction::kNoSuchMethodDispatcher ||
          kind == UntaggedFunction::kInvokeFieldDispatcher ||
          kind == UntaggedFunction::kDynamicInvocationForwarder);
@@ -3951,7 +3952,8 @@ FunctionPtr Class::GetInvocationDispatcher(const String& target_name,
   if (!function.IsNull()) return function.ptr();
 
   // Otherwise create it & add it.
-  function = CreateInvocationDispatcher(target_name, args_desc, kind);
+  function = CreateInvocationDispatcher(target_name, args_desc, kind,
+                                        is_dynamically_callable);
   AddInvocationDispatcher(target_name, args_desc, function);
   return function.ptr();
 }
@@ -3959,7 +3961,8 @@ FunctionPtr Class::GetInvocationDispatcher(const String& target_name,
 FunctionPtr Class::CreateInvocationDispatcher(
     const String& target_name,
     const Array& args_desc,
-    UntaggedFunction::Kind kind) const {
+    UntaggedFunction::Kind kind,
+    bool is_dynamically_callable) const {
   ASSERT(target_name.ptr() != Symbols::DynamicImplicitCall().ptr());
   Thread* thread = Thread::Current();
   Zone* zone = thread->zone();
@@ -4026,6 +4029,7 @@ FunctionPtr Class::CreateInvocationDispatcher(
   invocation.set_is_visible(false);
   invocation.set_is_reflectable(false);
   invocation.set_saved_args_desc(args_desc);
+  invocation.set_is_dynamically_callable(is_dynamically_callable);
 
   signature ^= ClassFinalizer::FinalizeType(signature);
   invocation.SetSignature(signature);
@@ -28004,7 +28008,10 @@ EntryPointPragma FindEntryPointPragma(IsolateGroup* IG,
     if ((pragma_name != Symbols::vm_entry_point().ptr()) &&
         (pragma_name != Symbols::dyn_module_callable().ptr()) &&
         (pragma_name != Symbols::dyn_module_implicitly_callable().ptr()) &&
-        (pragma_name != Symbols::dyn_module_extendable().ptr())) {
+        (pragma_name != Symbols::dyn_module_extendable().ptr()) &&
+        (pragma_name != Symbols::dyn_module_dynamically_callable().ptr()) &&
+        (pragma_name !=
+         Symbols::dyn_module_implicitly_dynamically_callable().ptr())) {
       continue;
     }
     *reusable_field_handle = IG->object_store()->pragma_options();

runtime/vm/object.h

@@ -1830,10 +1830,12 @@ class Class : public Object {
   intptr_t FindInvocationDispatcherFunctionIndex(const Function& needle) const;
   FunctionPtr InvocationDispatcherFunctionFromIndex(intptr_t idx) const;
 
-  FunctionPtr GetInvocationDispatcher(const String& target_name,
+  FunctionPtr GetInvocationDispatcher(
+      const String& target_name,
       const Array& args_desc,
       UntaggedFunction::Kind kind,
-                                      bool create_if_absent) const;
+      bool create_if_absent,
+      bool is_dynamically_callable = false) const;
 
   FunctionPtr GetRecordFieldGetter(const String& getter_name) const;
 
@@ -2094,7 +2096,8 @@ class Class : public Object {
 
   FunctionPtr CreateInvocationDispatcher(const String& target_name,
                                          const Array& args_desc,
-                                         UntaggedFunction::Kind kind) const;
+                                         UntaggedFunction::Kind kind,
+                                         bool is_dynamically_callable) const;
 
   FunctionPtr CreateRecordFieldGetter(const String& getter_name) const;
 
@@ -4178,6 +4181,8 @@ class Function : public Object {
   // polymorphic_target: A polymorphic method.
   // has_pragma: Has a @pragma decoration.
   // no_such_method_forwarder: A stub method that just calls noSuchMethod.
+  // dynamically_callable: host function or dynamic forwarder that can be
+  //                       invoked dynamically from a dynamic module.
 
 // Bits that are set when function is created, don't have to worry about
 // concurrent updates.
@@ -4195,7 +4200,8 @@ class Function : public Object {
   V(HasPragma, has_pragma)                                                     \
   V(IsSynthetic, is_synthetic)                                                 \
   V(IsExtensionMember, is_extension_member)                                    \
-  V(IsExtensionTypeMember, is_extension_type_member)
+  V(IsExtensionTypeMember, is_extension_type_member)                           \
+  V(IsDynamicallyCallable, is_dynamically_callable)
 // Bit that is updated after function is constructed, has to be updated in
 // concurrent-safe manner.
 #define FOR_EACH_FUNCTION_VOLATILE_KIND_BIT(V) V(Inlinable, is_inlinable)
@@ -4502,6 +4508,13 @@ class Field : public Object {
     return untag()->kind_bits_.Read<HasDeeplyImmutableTypeBit>();
   }
 
+  void set_is_dynamically_callable(bool value) const {
+    untag()->kind_bits_.UpdateBool<IsDynamicallyCallableBit>(value);
+  }
+  bool is_dynamically_callable() const {
+    return untag()->kind_bits_.Read<IsDynamicallyCallableBit>();
+  }
+
 #if defined(DART_DYNAMIC_MODULES)
   bool is_declared_in_bytecode() const;
 #else
@@ -4911,6 +4924,10 @@ class Field : public Object {
       BitField<decltype(UntaggedField::kind_bits_),
                bool,
                NoSanitizeThreadBit::kNextBit>;
+  using IsDynamicallyCallableBit =
+      BitField<decltype(UntaggedField::kind_bits_),
+               bool,
+               HasDeeplyImmutableTypeBit::kNextBit>;
 
   // Force this field's guard to be dynamic and deoptimize dependent code.
   void ForceDynamicGuardedCidAndLength() const;

runtime/vm/runtime_entry.cc

@@ -2293,7 +2293,8 @@ static bool ResolveCallThroughGetter(const Class& receiver_class,
   const Function& target_function =
       Function::Handle(receiver_class.GetInvocationDispatcher(
           dispatcher_name, arguments_descriptor,
-          UntaggedFunction::kInvokeFieldDispatcher, create_if_absent));
+          UntaggedFunction::kInvokeFieldDispatcher, create_if_absent,
+          getter.is_dynamically_callable()));
   ASSERT(!create_if_absent || !target_function.IsNull());
   if (FLAG_trace_ic) {
     OS::PrintErr(
@@ -2333,7 +2334,8 @@ FunctionPtr InlineCacheMissHelper(const Class& receiver_class,
     const Function& target_function =
         Function::Handle(receiver_class.GetInvocationDispatcher(
             *demangled, args_descriptor,
-            UntaggedFunction::kNoSuchMethodDispatcher, create_if_absent));
+            UntaggedFunction::kNoSuchMethodDispatcher, create_if_absent,
+            /* is_dynamically_callable = */ true));
     if (FLAG_trace_ic) {
       OS::PrintErr(
           "NoSuchMethod IC miss: adding <%s> id:%" Pd " -> <%s>\n",
@@ -3585,19 +3587,23 @@ static ObjectPtr InvokeCallThroughGetterOrNoSuchMethod(
     ArgumentsDescriptor args_desc(orig_arguments_desc);
     while (!cls.IsNull()) {
       // If there is a function with the target name but mismatched arguments
-      // we need to call `receiver.noSuchMethod()`.
+      // we need to call `receiver.noSuchMethod()`. Similarly, if there is a
+      // function that we aren't allowed to invoke because the target function
+      // was not dynamically-callable from a dynamic module.
       if (cls.EnsureIsFinalized(thread) == Error::null()) {
         function = Resolver::ResolveDynamicFunction(zone, cls, target_name);
       }
       if (!function.IsNull()) {
-        ASSERT(!function.AreValidArguments(args_desc, nullptr));
+        ASSERT(!function.is_dynamically_callable() ||
+               !function.AreValidArguments(args_desc, nullptr));
         break;  // mismatch, invoke noSuchMethod
       }
       if (is_dynamic_call) {
         function =
             Resolver::ResolveDynamicFunction(zone, cls, demangled_target_name);
         if (!function.IsNull()) {
-          ASSERT(!function.AreValidArguments(args_desc, nullptr));
+          ASSERT(!function.is_dynamically_callable() ||
+                 !function.AreValidArguments(args_desc, nullptr));
           break;  // mismatch, invoke noSuchMethod
         }
       }

runtime/vm/symbol_list.h

@@ -524,6 +524,9 @@ namespace dart {
   V(dyn_module_extendable, "dyn-module:extendable")                            \
   V(dyn_module_implicitly_callable, "dyn-module:implicitly-callable")          \
   V(dyn_module_can_be_used_as_type, "dyn-module:can-be-used-as-type")          \
+  V(dyn_module_dynamically_callable, "dyn-module:dynamically-callable")        \
+  V(dyn_module_implicitly_dynamically_callable,                                \
+    "dyn-module:implicitly-dynamically-callable")                              \
   V(executable, "executable")                                                  \
   V(external_effect, "external-effect")                                        \
   V(get, "get")                                                                \

utils/dynamic_module_runner/dynamic_interface.yaml

@@ -55,3 +55,22 @@ callable:
   - library: 'dart:vmservice_io'
   - library: 'dart:_internal'
     member: 'extractTypeArguments'
+
+dynamically-callable:
+  - library: 'dart:async'
+  - library: 'dart:cli'
+  - library: 'dart:collection'
+  - library: 'dart:concurrent'
+  - library: 'dart:convert'
+  - library: 'dart:core'
+  - library: 'dart:core'
+    class: '_Closure'
+    member: 'get:call'
+  - library: 'dart:developer'
+  - library: 'dart:ffi'
+  - library: 'dart:io'
+  - library: 'dart:isolate'
+  - library: 'dart:math'
+  - library: 'dart:nativewrappers'
+  - library: 'dart:typed_data'
+  - library: 'dart:vmservice_io'