From cca6298498969f501c4ae3dd1266c4218cc810e0 Mon Sep 17 00:00:00 2001 From: Sigmund Cherem Date: Wed, 13 May 2026 09:40:57 -0700 Subject: [PATCH] [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 Reviewed-by: Alexander Markov Auto-Submit: Sigmund Cherem Commit-Queue: Sigmund Cherem --- runtime/vm/compiler/aot/precompiler.cc | 37 +++++++-------- runtime/vm/interpreter.cc | 47 ++++++++++++++----- runtime/vm/interpreter.h | 3 +- runtime/vm/kernel_loader.cc | 15 ++++++ runtime/vm/kernel_loader.h | 4 ++ runtime/vm/object.cc | 15 ++++-- runtime/vm/object.h | 29 +++++++++--- runtime/vm/runtime_entry.cc | 16 +++++-- runtime/vm/symbol_list.h | 3 ++ .../dynamic_interface.yaml | 19 ++++++++ 10 files changed, 141 insertions(+), 47 deletions(-) diff --git a/runtime/vm/compiler/aot/precompiler.cc b/runtime/vm/compiler/aot/precompiler.cc index a1501f76cdb..25807e1cc8e 100644 --- a/runtime/vm/compiler/aot/precompiler.cc +++ b/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,22 +1770,16 @@ void Precompiler::CheckForNewDynamicFunctions() { } else if (!found_metadata) { metadata = kernel::ProcedureAttributesOf(function, Z); } - - if (is_getter) { - if (metadata.getter_called_dynamically) { - function2 = function.GetDynamicInvocationForwarder(selector2); - 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); - } - } + generate_dynamic_forwarder = + is_getter ? metadata.getter_called_dynamically + : metadata.method_or_setter_called_dynamically; + } + if (generate_dynamic_forwarder) { + function2 = function.GetDynamicInvocationForwarder(selector2); + ASSERT(function.is_dynamically_callable() == + function2.is_dynamically_callable()); + AddFunction(function2, RetainReasons::kDynamicInvocationForwarder); + functions_called_dynamically_.Insert(function2); } } } diff --git a/runtime/vm/interpreter.cc b/runtime/vm/interpreter.cc index 6895874a119..9e8391f8b3a 100644 --- a/runtime/vm/interpreter.cc +++ b/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); - top[0] = target; - return Invoke(thread, call_base, top, pc, FP, SP); + + 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; diff --git a/runtime/vm/interpreter.h b/runtime/vm/interpreter.h index b2d36c17e0b..dfbbde9305c 100644 --- a/runtime/vm/interpreter.h +++ b/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, diff --git a/runtime/vm/kernel_loader.cc b/runtime/vm/kernel_loader.cc index d73164d8697..a5a800e3f47 100644 --- a/runtime/vm/kernel_loader.cc +++ b/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)); } } diff --git a/runtime/vm/kernel_loader.h b/runtime/vm/kernel_loader.h index 9ab092d0560..c8f308398cf 100644 --- a/runtime/vm/kernel_loader.h +++ b/runtime/vm/kernel_loader.h @@ -243,6 +243,10 @@ class KernelLoader : public ValueObject { BitField; using DynModuleCanBeOverriddenImplicitlyPragma = BitField; + using DynModuleDynamicallyCallablePragma = + BitField; void FinishTopLevelClassLoading(const Class& toplevel_class, const Library& library, diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index 29c6933454b..80dcdb586f2 100644 --- a/runtime/vm/object.cc +++ b/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(); diff --git a/runtime/vm/object.h b/runtime/vm/object.h index ae3f0c35ec6..9c23957a928 100644 --- a/runtime/vm/object.h +++ b/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, - const Array& args_desc, - UntaggedFunction::Kind kind, - bool create_if_absent) const; + FunctionPtr GetInvocationDispatcher( + const String& target_name, + const Array& args_desc, + UntaggedFunction::Kind kind, + 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(); } + void set_is_dynamically_callable(bool value) const { + untag()->kind_bits_.UpdateBool(value); + } + bool is_dynamically_callable() const { + return untag()->kind_bits_.Read(); + } + #if defined(DART_DYNAMIC_MODULES) bool is_declared_in_bytecode() const; #else @@ -4911,6 +4924,10 @@ class Field : public Object { BitField; + using IsDynamicallyCallableBit = + BitField; // Force this field's guard to be dynamic and deoptimize dependent code. void ForceDynamicGuardedCidAndLength() const; diff --git a/runtime/vm/runtime_entry.cc b/runtime/vm/runtime_entry.cc index 8e2bb3dbfd6..14077523ad3 100644 --- a/runtime/vm/runtime_entry.cc +++ b/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 } } diff --git a/runtime/vm/symbol_list.h b/runtime/vm/symbol_list.h index 2391059bf74..ee21e6bab42 100644 --- a/runtime/vm/symbol_list.h +++ b/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") \ diff --git a/utils/dynamic_module_runner/dynamic_interface.yaml b/utils/dynamic_module_runner/dynamic_interface.yaml index 1283f5a1787..1974cedc556 100644 --- a/utils/dynamic_module_runner/dynamic_interface.yaml +++ b/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'