From fbbc99e1e94830f5f5ab0c1926e8c9aec0b441ec Mon Sep 17 00:00:00 2001 From: Ben Konyi Date: Mon, 8 Feb 2021 23:30:05 +0000 Subject: [PATCH] [ VM ] Ensure Dart C APIs perform type checks when invoking functions and constructors Dart_New, Dart_InvokeConstructor, and Dart_InvokeClosure were not checking the types of arguments before performing invocations. Fixes https://github.com/dart-lang/sdk/issues/44205. TEST=DartAPI_*_Issue44205 Change-Id: I6c06e3e8a0a88b9caa7e2212c4a2c8e43d39b4f4 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/182962 Commit-Queue: Ben Konyi Reviewed-by: Siva Annamalai Reviewed-by: Alexander Markov --- runtime/vm/dart_api_impl.cc | 48 +++++++++--- runtime/vm/dart_api_impl_test.cc | 124 +++++++++++++++++++++++++++++++ runtime/vm/dart_entry.cc | 2 +- runtime/vm/object.cc | 6 +- 4 files changed, 168 insertions(+), 12 deletions(-) diff --git a/runtime/vm/dart_api_impl.cc b/runtime/vm/dart_api_impl.cc index e5860d4b697..50521d8d68b 100644 --- a/runtime/vm/dart_api_impl.cc +++ b/runtime/vm/dart_api_impl.cc @@ -4401,6 +4401,17 @@ DART_EXPORT Dart_Handle Dart_New(Dart_Handle type, args.SetAt(arg_index++, argument); } + const int kTypeArgsLen = 0; + Array& args_descriptor_array = Array::Handle( + Z, ArgumentsDescriptor::NewBoxed(kTypeArgsLen, args.Length())); + + ArgumentsDescriptor args_descriptor(args_descriptor_array); + ObjectPtr type_error = constructor.DoArgumentTypesMatch( + args, args_descriptor, type_arguments, Object::empty_type_arguments()); + if (type_error != Error::null()) { + return Api::NewHandle(T, type_error); + } + // Invoke the constructor and return the new object. result = DartEntry::InvokeFunction(constructor, args); if (result.IsError()) { @@ -4451,7 +4462,17 @@ DART_EXPORT Dart_Handle Dart_Allocate(Dart_Handle type) { if (type_obj.IsNull()) { RETURN_TYPE_ERROR(Z, type, Type); } + + if (!type_obj.IsFinalized()) { + return Api::NewError( + "%s expects argument 'type' to be a fully resolved type.", + CURRENT_FUNC); + } + const Class& cls = Class::Handle(Z, type_obj.type_class()); + const TypeArguments& type_arguments = + TypeArguments::Handle(Z, type_obj.arguments()); + CHECK_ERROR_HANDLE(cls.VerifyEntryPoint()); #if defined(DEBUG) if (!cls.is_allocated() && (Dart::vm_snapshot_kind() == Snapshot::kFullAOT)) { @@ -4459,7 +4480,11 @@ DART_EXPORT Dart_Handle Dart_Allocate(Dart_Handle type) { } #endif CHECK_ERROR_HANDLE(cls.EnsureIsAllocateFinalized(T)); - return Api::NewHandle(T, AllocateObject(T, cls)); + const Instance& new_obj = Instance::Handle(Z, AllocateObject(T, cls)); + if (!type_arguments.IsNull()) { + new_obj.SetTypeArguments(type_arguments); + } + return Api::NewHandle(T, new_obj.ptr()); } DART_EXPORT Dart_Handle @@ -4546,7 +4571,7 @@ DART_EXPORT Dart_Handle Dart_InvokeConstructor(Dart_Handle object, // Construct name of the constructor to invoke. const String& constructor_name = Api::UnwrapStringHandle(Z, name); - const AbstractType& type_obj = + AbstractType& type_obj = AbstractType::Handle(Z, instance.GetType(Heap::kNew)); const Class& cls = Class::Handle(Z, type_obj.type_class()); const String& class_name = String::Handle(Z, cls.Name()); @@ -4570,20 +4595,23 @@ DART_EXPORT Dart_Handle Dart_InvokeConstructor(Dart_Handle object, kTypeArgsLen, number_of_arguments + extra_args, 0, NULL)) { CHECK_ERROR_HANDLE(constructor.VerifyCallEntryPoint()); // Create the argument list. - // Constructors get the uninitialized object. - if (!type_arguments.IsNull()) { - // The type arguments will be null if the class has no type - // parameters, in which case the following call would fail - // because there is no slot reserved in the object for the - // type vector. - instance.SetTypeArguments(type_arguments); - } Dart_Handle result; Array& args = Array::Handle(Z); result = SetupArguments(T, number_of_arguments, arguments, extra_args, &args); if (!Api::IsError(result)) { args.SetAt(0, instance); + + const int kTypeArgsLen = 0; + const Array& args_descriptor_array = Array::Handle( + Z, ArgumentsDescriptor::NewBoxed(kTypeArgsLen, args.Length())); + ArgumentsDescriptor args_descriptor(args_descriptor_array); + ObjectPtr type_error = constructor.DoArgumentTypesMatch( + args, args_descriptor, type_arguments); + if (type_error != Error::null()) { + return Api::NewHandle(T, type_error); + } + const Object& retval = Object::Handle(Z, DartEntry::InvokeFunction(constructor, args)); if (retval.IsError()) { diff --git a/runtime/vm/dart_api_impl_test.cc b/runtime/vm/dart_api_impl_test.cc index a9fd5ba94dc..fd10aaa4742 100644 --- a/runtime/vm/dart_api_impl_test.cc +++ b/runtime/vm/dart_api_impl_test.cc @@ -5792,6 +5792,130 @@ TEST_CASE(DartAPI_New) { EXPECT(!instanceOf); } +TEST_CASE(DartAPI_New_Issue44205) { + const char* kScriptChars = + "class MyIntClass {\n" + " MyIntClass(this.value);\n" + " int value;\n" + "}\n" + "\n" + "class MyClass {\n" + " T value;\n" + " MyClass(this.value);\n" + " factory MyClass.foo(T x) => MyClass(x);\n" + "}\n" + "\n" + "Type getIntType() { return int; }\n" + "\n"; + + Dart_Handle lib = TestCase::LoadTestScript(kScriptChars, NULL); + Dart_Handle int_wrapper_type = + Dart_GetNonNullableType(lib, NewString("MyIntClass"), 0, NULL); + + Dart_Handle args[1]; + args[0] = Dart_EmptyString(); + + Dart_Handle result = Dart_New(int_wrapper_type, Dart_EmptyString(), 1, args); + EXPECT_ERROR(result, + "type 'String' is not a subtype of type 'int' of 'value'"); + + Dart_Handle int_type = Dart_Invoke(lib, NewString("getIntType"), 0, args); + EXPECT_VALID(int_type); + Dart_Handle type_args = Dart_NewList(1); + EXPECT_VALID(type_args); + EXPECT_VALID(Dart_ListSetAt(type_args, 0, int_type)); + Dart_Handle my_class_type = + Dart_GetNonNullableType(lib, NewString("MyClass"), 1, &type_args); + EXPECT_VALID(my_class_type); + + // Generic generative constructor + args[0] = Dart_EmptyString(); + result = Dart_New(my_class_type, Dart_EmptyString(), 1, args); + EXPECT_ERROR(result, + "type 'String' is not a subtype of type 'int' of 'value'"); + + // Generic factory constructor + result = Dart_New(my_class_type, NewString("foo"), 1, args); + EXPECT_ERROR(result, "type 'String' is not a subtype of type 'int' of 'x'"); +} + +TEST_CASE(DartAPI_InvokeConstructor_Issue44205) { + const char* kScriptChars = + "class MyIntClass {\n" + " MyIntClass(this.value);\n" + " int value;\n" + "}\n" + "\n" + "class MyClass {\n" + " T value;\n" + " MyClass(this.value);\n" + "}\n" + "\n" + "Type getIntType() { return int; }\n" + "\n"; + + Dart_Handle lib = TestCase::LoadTestScript(kScriptChars, NULL); + Dart_Handle int_wrapper_type = + Dart_GetNonNullableType(lib, NewString("MyIntClass"), 0, NULL); + + Dart_Handle args[1]; + args[0] = Dart_EmptyString(); + Dart_Handle result = Dart_Allocate(int_wrapper_type); + EXPECT_VALID(result); + + result = Dart_InvokeConstructor(result, Dart_EmptyString(), 1, args); + EXPECT_ERROR(result, + "type 'String' is not a subtype of type 'int' of 'value'"); + + Dart_Handle int_type = Dart_Invoke(lib, NewString("getIntType"), 0, args); + EXPECT_VALID(int_type); + Dart_Handle type_args = Dart_NewList(1); + EXPECT_VALID(type_args); + EXPECT_VALID(Dart_ListSetAt(type_args, 0, int_type)); + Dart_Handle my_class_type = + Dart_GetNonNullableType(lib, NewString("MyClass"), 1, &type_args); + EXPECT_VALID(my_class_type); + + result = Dart_Allocate(my_class_type); + EXPECT_VALID(result); + result = Dart_InvokeConstructor(result, Dart_EmptyString(), 1, args); + EXPECT_ERROR(result, + "type 'String' is not a subtype of type 'int' of 'value'"); +} + +TEST_CASE(DartAPI_InvokeClosure_Issue44205) { + const char* kScriptChars = + "class InvokeClosure {\n" + " InvokeClosure(int i, int j) : fld1 = i, fld2 = j {}\n" + " Function method1(int i) {\n" + " f(int j) => j + i + fld1 + fld2 + fld4; \n" + " return f;\n" + " }\n" + " int fld1;\n" + " final int fld2;\n" + " static const int fld4 = 10;\n" + "}\n" + "Function testMain1() {\n" + " InvokeClosure obj = new InvokeClosure(10, 20);\n" + " return obj.method1(10);\n" + "}\n"; + Dart_Handle result; + CHECK_API_SCOPE(thread); + + // Create a test library and Load up a test script in it. + Dart_Handle lib = TestCase::LoadTestScript(kScriptChars, NULL); + + // Invoke a function which returns a closure. + Dart_Handle retobj = Dart_Invoke(lib, NewString("testMain1"), 0, NULL); + EXPECT_VALID(retobj); + + // Now invoke the closure and check the result. + Dart_Handle dart_arguments[1]; + dart_arguments[0] = Dart_EmptyString(); + result = Dart_InvokeClosure(retobj, 1, dart_arguments); + EXPECT_ERROR(result, "type 'String' is not a subtype of type 'int' of 'j'"); +} + TEST_CASE(DartAPI_New_Issue2971) { // Issue 2971: We were unable to use Dart_New to construct an // instance of List, due to problems implementing interface diff --git a/runtime/vm/dart_entry.cc b/runtime/vm/dart_entry.cc index 4f201b3090a..691bd6c32d7 100644 --- a/runtime/vm/dart_entry.cc +++ b/runtime/vm/dart_entry.cc @@ -276,7 +276,7 @@ ObjectPtr DartEntry::InvokeCallable(Thread* thread, const auto& result = Object::Handle( zone, callable_function.DoArgumentTypesMatch(arguments, args_desc)); if (result.IsError()) { - Exceptions::PropagateError(Error::Cast(result)); + return result.ptr(); } return InvokeFunction(callable_function, arguments, arguments_descriptor); diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index 7ad31370594..ec68b243d9a 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -8138,10 +8138,14 @@ ObjectPtr Function::DoArgumentTypesMatch( // Adjust for type arguments when they're present. const intptr_t param_index = arg_index - arg_offset; type = ParameterTypeAt(param_index); - if (!check_argument(argument, type, instantiator_type_arguments, function_type_arguments)) { auto& name = String::Handle(zone, ParameterNameAt(param_index)); + if (!type.IsInstantiated()) { + type = + type.InstantiateFrom(instantiator_type_arguments, + function_type_arguments, kAllFree, Heap::kNew); + } return ThrowTypeError(token_pos(), argument, type, name); } }