diff --git a/pkg/expect/lib/expect.dart b/pkg/expect/lib/expect.dart index 3b7a2e59da6..47ada711c36 100644 --- a/pkg/expect/lib/expect.dart +++ b/pkg/expect/lib/expect.dart @@ -440,6 +440,17 @@ class Expect { _fail("$defaultMessage$diff"); } + /// Checks that [haystack] contains a given substring [needle]. + /// + /// For example, this succeeds: + /// + /// Expect.contains("a", "abcdefg"); + static void contains(String needle, String haystack) { + if (!haystack.contains(needle)) { + _fail("String '$needle' not found within '$haystack'"); + } + } + /// Checks that [actual] contains a given list of [substrings] in order. /// /// For example, this succeeds: diff --git a/runtime/tests/vm/dart/causal_stacks/async_throws_stack_lazy_non_symbolic_test.dart b/runtime/tests/vm/dart/causal_stacks/async_throws_stack_lazy_non_symbolic_test.dart index 11c1a3301ff..2ec438e2cab 100644 --- a/runtime/tests/vm/dart/causal_stacks/async_throws_stack_lazy_non_symbolic_test.dart +++ b/runtime/tests/vm/dart/causal_stacks/async_throws_stack_lazy_non_symbolic_test.dart @@ -2,7 +2,7 @@ // 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=--dwarf-stack-traces --save-debugging-info=async_lazy_debug.so --lazy-async-stacks --no-causal-async-stacks +// VMOptions=--dwarf-stack-traces --save-debugging-info=async_lazy_debug.so --lazy-async-stacks --no-causal-async-stacks --no-use-bare-instructions import 'dart:async'; import 'dart:io'; diff --git a/runtime/tests/vm/dart/causal_stacks/utils.dart b/runtime/tests/vm/dart/causal_stacks/utils.dart index ed1549c6ebc..a0fbb822498 100644 --- a/runtime/tests/vm/dart/causal_stacks/utils.dart +++ b/runtime/tests/vm/dart/causal_stacks/utils.dart @@ -1355,7 +1355,7 @@ Future doTestsLazy([String? debugInfoFilename]) async { final awaitTimeoutExpected = const [ r'^#0 throwAsync \(.*/utils.dart:21(:3)?\)$', r'^$', - r'^#1 Future.timeout. \(dart:async/future_impl.dart\)$', + r'^#1 Future.timeout. \(dart:async/future_impl.dart', r'^$', r'^#2 awaitTimeout ', r'^$', @@ -1386,7 +1386,7 @@ Future doTestsLazy([String? debugInfoFilename]) async { final awaitWaitExpected = const [ r'^#0 throwAsync \(.*/utils.dart:21(:3)?\)$', r'^$', - r'^#1 Future.wait. \(dart:async/future.dart\)$', + r'^#1 Future.wait. \(dart:async/future.dart', r'^$', r'^#2 awaitWait ', r'^$', diff --git a/runtime/tests/vm/dart/entrypoints/tearoff_prologue_test.dart b/runtime/tests/vm/dart/entrypoints/tearoff_prologue_test.dart deleted file mode 100644 index 2cf7278d0ba..00000000000 --- a/runtime/tests/vm/dart/entrypoints/tearoff_prologue_test.dart +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file -// 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. -// -// No type checks are removed here, but we can skip the argument count check. -// VMOptions=--enable-testing-pragmas --no-background-compilation --optimization-counter-threshold=10 -// VMOptions=--enable-testing-pragmas --no-background-compilation --optimization-counter-threshold=10 -Denable_inlining=true -// VMOptions=--enable-testing-pragmas --no-background-compilation --optimization-counter-threshold=-1 - -import "package:expect/expect.dart"; -import "common.dart"; - -class C { - @NeverInline - @pragma("vm:testing.unsafe.trace-entrypoints-fn", validateTearoff) - @pragma("vm:entry-point") - void samir1(T x) { - if (x == -1) { - throw "oh no"; - } - } -} - -void run(void Function(int) test, int i) { - test(i); -} - -main(List args) { - var c = new C(); - var f = c.samir1; - - const int iterations = benchmarkMode ? 100000000 : 100; - for (int i = 0; i < iterations; ++i) { - run(f, i); - } - - entryPoint.expectChecked(iterations); - tearoffEntryPoint.expectUnchecked(iterations); -} diff --git a/runtime/tests/vm/dart/entrypoints/tearoff_test.dart b/runtime/tests/vm/dart/entrypoints/tearoff_test.dart deleted file mode 100644 index d0bd88ab90a..00000000000 --- a/runtime/tests/vm/dart/entrypoints/tearoff_test.dart +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright (c) 2018, the Dart project authors. Please see the AUTHORS file -// 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=--enable-testing-pragmas --no-background-compilation --optimization-counter-threshold=10 -// VMOptions=--enable-testing-pragmas --no-background-compilation --optimization-counter-threshold=10 -Denable_inlining=true -// VMOptions=--enable-testing-pragmas --no-background-compilation --optimization-counter-threshold=-1 - -// Test that typed calls against tearoffs go into the unchecked entrypoint. - -import "package:expect/expect.dart"; -import "common.dart"; - -class C { - @NeverInline - @pragma("vm:testing.unsafe.trace-entrypoints-fn", validateTearoff) - @pragma("vm:entry-point") - void target1(T x, String y) { - Expect.notEquals(x, -1); - Expect.equals(y, "foo"); - } -} - -void run(void Function(int, String) fn, int i) { - fn(i, "foo"); -} - -main(List args) { - var f = (new C()).target1; - - const int iterations = benchmarkMode ? 100000000 : 100; - for (int i = 0; i < iterations; ++i) { - run(f, i); - } - - entryPoint.expectChecked(iterations); - tearoffEntryPoint.expectUnchecked(iterations); -} diff --git a/runtime/tests/vm/dart_2/causal_stacks/async_throws_stack_lazy_non_symbolic_test.dart b/runtime/tests/vm/dart_2/causal_stacks/async_throws_stack_lazy_non_symbolic_test.dart index 11c1a3301ff..2ec438e2cab 100644 --- a/runtime/tests/vm/dart_2/causal_stacks/async_throws_stack_lazy_non_symbolic_test.dart +++ b/runtime/tests/vm/dart_2/causal_stacks/async_throws_stack_lazy_non_symbolic_test.dart @@ -2,7 +2,7 @@ // 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=--dwarf-stack-traces --save-debugging-info=async_lazy_debug.so --lazy-async-stacks --no-causal-async-stacks +// VMOptions=--dwarf-stack-traces --save-debugging-info=async_lazy_debug.so --lazy-async-stacks --no-causal-async-stacks --no-use-bare-instructions import 'dart:async'; import 'dart:io'; diff --git a/runtime/tests/vm/dart_2/causal_stacks/utils.dart b/runtime/tests/vm/dart_2/causal_stacks/utils.dart index a67a475b41a..3b6facbeea5 100644 --- a/runtime/tests/vm/dart_2/causal_stacks/utils.dart +++ b/runtime/tests/vm/dart_2/causal_stacks/utils.dart @@ -1355,7 +1355,7 @@ Future doTestsLazy([String debugInfoFilename]) async { final awaitTimeoutExpected = const [ r'^#0 throwAsync \(.*/utils.dart:21(:3)?\)$', r'^$', - r'^#1 Future.timeout. \(dart:async/future_impl.dart\)$', + r'^#1 Future.timeout. \(dart:async/future_impl.dart', r'^$', r'^#2 awaitTimeout ', r'^$', @@ -1386,7 +1386,7 @@ Future doTestsLazy([String debugInfoFilename]) async { final awaitWaitExpected = const [ r'^#0 throwAsync \(.*/utils.dart:21(:3)?\)$', r'^$', - r'^#1 Future.wait. \(dart:async/future.dart\)$', + r'^#1 Future.wait. \(dart:async/future.dart', r'^$', r'^#2 awaitWait ', r'^$', diff --git a/runtime/tests/vm/dart_2/entrypoints/tearoff_prologue_test.dart b/runtime/tests/vm/dart_2/entrypoints/tearoff_prologue_test.dart deleted file mode 100644 index 2cf7278d0ba..00000000000 --- a/runtime/tests/vm/dart_2/entrypoints/tearoff_prologue_test.dart +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file -// 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. -// -// No type checks are removed here, but we can skip the argument count check. -// VMOptions=--enable-testing-pragmas --no-background-compilation --optimization-counter-threshold=10 -// VMOptions=--enable-testing-pragmas --no-background-compilation --optimization-counter-threshold=10 -Denable_inlining=true -// VMOptions=--enable-testing-pragmas --no-background-compilation --optimization-counter-threshold=-1 - -import "package:expect/expect.dart"; -import "common.dart"; - -class C { - @NeverInline - @pragma("vm:testing.unsafe.trace-entrypoints-fn", validateTearoff) - @pragma("vm:entry-point") - void samir1(T x) { - if (x == -1) { - throw "oh no"; - } - } -} - -void run(void Function(int) test, int i) { - test(i); -} - -main(List args) { - var c = new C(); - var f = c.samir1; - - const int iterations = benchmarkMode ? 100000000 : 100; - for (int i = 0; i < iterations; ++i) { - run(f, i); - } - - entryPoint.expectChecked(iterations); - tearoffEntryPoint.expectUnchecked(iterations); -} diff --git a/runtime/tests/vm/dart_2/entrypoints/tearoff_test.dart b/runtime/tests/vm/dart_2/entrypoints/tearoff_test.dart deleted file mode 100644 index d0bd88ab90a..00000000000 --- a/runtime/tests/vm/dart_2/entrypoints/tearoff_test.dart +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright (c) 2018, the Dart project authors. Please see the AUTHORS file -// 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=--enable-testing-pragmas --no-background-compilation --optimization-counter-threshold=10 -// VMOptions=--enable-testing-pragmas --no-background-compilation --optimization-counter-threshold=10 -Denable_inlining=true -// VMOptions=--enable-testing-pragmas --no-background-compilation --optimization-counter-threshold=-1 - -// Test that typed calls against tearoffs go into the unchecked entrypoint. - -import "package:expect/expect.dart"; -import "common.dart"; - -class C { - @NeverInline - @pragma("vm:testing.unsafe.trace-entrypoints-fn", validateTearoff) - @pragma("vm:entry-point") - void target1(T x, String y) { - Expect.notEquals(x, -1); - Expect.equals(y, "foo"); - } -} - -void run(void Function(int, String) fn, int i) { - fn(i, "foo"); -} - -main(List args) { - var f = (new C()).target1; - - const int iterations = benchmarkMode ? 100000000 : 100; - for (int i = 0; i < iterations; ++i) { - run(f, i); - } - - entryPoint.expectChecked(iterations); - tearoffEntryPoint.expectUnchecked(iterations); -} diff --git a/runtime/vm/compiler/assembler/assembler_arm.h b/runtime/vm/compiler/assembler/assembler_arm.h index daeb5f71280..f853da7ce3a 100644 --- a/runtime/vm/compiler/assembler/assembler_arm.h +++ b/runtime/vm/compiler/assembler/assembler_arm.h @@ -1023,6 +1023,12 @@ class Assembler : public AssemblerBase { JumpDistance distance = kFarJump) { b(label, condition); } + void BranchIfZero(Register rn, + Label* label, + JumpDistance distance = kFarJump) { + cmp(rn, Operand(0)); + b(label, ZERO); + } void MoveRegister(Register rd, Register rm, Condition cond = AL); diff --git a/runtime/vm/compiler/assembler/assembler_arm64.h b/runtime/vm/compiler/assembler/assembler_arm64.h index 2c85181e22a..3290499b5fd 100644 --- a/runtime/vm/compiler/assembler/assembler_arm64.h +++ b/runtime/vm/compiler/assembler/assembler_arm64.h @@ -1109,6 +1109,11 @@ class Assembler : public AssemblerBase { JumpDistance distance = kFarJump) { b(label, condition); } + void BranchIfZero(Register rn, + Label* label, + JumpDistance distance = kFarJump) { + cbz(label, rn); + } void cbz(Label* label, Register rt, OperandSize sz = kEightBytes) { EmitCompareAndBranch(CBZ, rt, label, sz); diff --git a/runtime/vm/compiler/assembler/assembler_ia32.cc b/runtime/vm/compiler/assembler/assembler_ia32.cc index 521f53ff818..d07c40f2387 100644 --- a/runtime/vm/compiler/assembler/assembler_ia32.cc +++ b/runtime/vm/compiler/assembler/assembler_ia32.cc @@ -1784,10 +1784,12 @@ void Assembler::LoadFromOffset(Register reg, } void Assembler::LoadFromStack(Register dst, intptr_t depth) { + ASSERT(depth >= 0); movl(dst, Address(ESP, depth * target::kWordSize)); } void Assembler::StoreToStack(Register src, intptr_t depth) { + ASSERT(depth >= 0); movl(Address(ESP, depth * target::kWordSize), src); } diff --git a/runtime/vm/compiler/assembler/assembler_ia32.h b/runtime/vm/compiler/assembler/assembler_ia32.h index 0c6ed99496f..dfa7f35b0d0 100644 --- a/runtime/vm/compiler/assembler/assembler_ia32.h +++ b/runtime/vm/compiler/assembler/assembler_ia32.h @@ -577,6 +577,12 @@ class Assembler : public AssemblerBase { JumpDistance distance = kFarJump) { j(condition, label, distance); } + void BranchIfZero(Register src, + Label* label, + JumpDistance distance = kFarJump) { + cmpl(src, Immediate(0)); + j(ZERO, label, distance); + } void LoadFromOffset(Register reg, Register base, @@ -716,6 +722,11 @@ class Assembler : public AssemblerBase { cmpxchgl(address, reg); } + void CompareTypeNullabilityWith(Register type, int8_t value) { + cmpb(FieldAddress(type, compiler::target::Type::nullability_offset()), + Immediate(value)); + } + void EnterFrame(intptr_t frame_space); void LeaveFrame(); void ReserveAlignedFrameSpace(intptr_t frame_space); diff --git a/runtime/vm/compiler/assembler/assembler_x64.h b/runtime/vm/compiler/assembler/assembler_x64.h index 01ed17b5196..3b5fa5c3c90 100644 --- a/runtime/vm/compiler/assembler/assembler_x64.h +++ b/runtime/vm/compiler/assembler/assembler_x64.h @@ -683,6 +683,12 @@ class Assembler : public AssemblerBase { JumpDistance distance = kFarJump) { j(condition, label, distance); } + void BranchIfZero(Register src, + Label* label, + JumpDistance distance = kFarJump) { + cmpq(src, Immediate(0)); + j(ZERO, label, distance); + } // Issues a move instruction if 'to' is not the same as 'from'. void MoveRegister(Register to, Register from); diff --git a/runtime/vm/compiler/backend/flow_graph_compiler.cc b/runtime/vm/compiler/backend/flow_graph_compiler.cc index 49021365891..cf202015ae8 100644 --- a/runtime/vm/compiler/backend/flow_graph_compiler.cc +++ b/runtime/vm/compiler/backend/flow_graph_compiler.cc @@ -2286,15 +2286,22 @@ void FlowGraphCompiler::GenerateCidRangesCheck( bool FlowGraphCompiler::CheckAssertAssignableTypeTestingABILocations( const LocationSummary& locs) { - ASSERT(locs.in(0).IsRegister() && - locs.in(0).reg() == TypeTestABI::kInstanceReg); - ASSERT((locs.in(1).IsConstant() && locs.in(1).constant().IsAbstractType()) || - (locs.in(1).IsRegister() && - locs.in(1).reg() == TypeTestABI::kDstTypeReg)); - ASSERT(locs.in(2).IsRegister() && - locs.in(2).reg() == TypeTestABI::kInstantiatorTypeArgumentsReg); - ASSERT(locs.in(3).IsRegister() && - locs.in(3).reg() == TypeTestABI::kFunctionTypeArgumentsReg); + ASSERT(locs.in(AssertAssignableInstr::kInstancePos).IsRegister() && + locs.in(AssertAssignableInstr::kInstancePos).reg() == + TypeTestABI::kInstanceReg); + ASSERT((locs.in(AssertAssignableInstr::kDstTypePos).IsConstant() && + locs.in(AssertAssignableInstr::kDstTypePos) + .constant() + .IsAbstractType()) || + (locs.in(AssertAssignableInstr::kDstTypePos).IsRegister() && + locs.in(AssertAssignableInstr::kDstTypePos).reg() == + TypeTestABI::kDstTypeReg)); + ASSERT(locs.in(AssertAssignableInstr::kInstantiatorTAVPos).IsRegister() && + locs.in(AssertAssignableInstr::kInstantiatorTAVPos).reg() == + TypeTestABI::kInstantiatorTypeArgumentsReg); + ASSERT(locs.in(AssertAssignableInstr::kFunctionTAVPos).IsRegister() && + locs.in(AssertAssignableInstr::kFunctionTAVPos).reg() == + TypeTestABI::kFunctionTypeArgumentsReg); ASSERT(locs.out(0).IsRegister() && locs.out(0).reg() == TypeTestABI::kInstanceReg); return true; @@ -2760,23 +2767,49 @@ void FlowGraphCompiler::GenerateAssertAssignable(CompileType* receiver_type, ASSERT(!token_pos.IsClassifying()); ASSERT(CheckAssertAssignableTypeTestingABILocations(*locs)); - if (!locs->in(1).IsConstant()) { - // TODO(dartbug.com/40813): Handle setting up the non-constant case. - UNREACHABLE(); - } - const auto& dst_type = AbstractType::Cast(locs->in(1).constant()); - ASSERT(dst_type.IsFinalized()); + // Non-null if we have a constant destination type. + const auto& dst_type = + locs->in(AssertAssignableInstr::kDstTypePos).IsConstant() + ? AbstractType::Cast( + locs->in(AssertAssignableInstr::kDstTypePos).constant()) + : Object::null_abstract_type(); - if (dst_type.IsTopTypeForSubtyping()) return; // No code needed. + if (!dst_type.IsNull()) { + ASSERT(dst_type.IsFinalized()); + if (dst_type.IsTopTypeForSubtyping()) return; // No code needed. + } compiler::Label done; + Register type_reg = TypeTestABI::kDstTypeReg; + // Generate caller-side checks to perform prior to calling the TTS. + if (dst_type.IsNull()) { + __ Comment("AssertAssignable for runtime type"); + // kDstTypeReg should already contain the destination type. + const bool null_safety = isolate()->null_safety(); + GenerateStubCall(token_pos, + StubCode::GetTypeIsTopTypeForSubtyping(null_safety), + PcDescriptorsLayout::kOther, locs, deopt_id); + // TypeTestABI::kSubtypeTestCacheReg is 0 if the type is a top type. + __ BranchIfZero(TypeTestABI::kSubtypeTestCacheReg, &done, + compiler::Assembler::kNearJump); - GenerateCallerChecksForAssertAssignable(receiver_type, dst_type, &done); + GenerateStubCall(token_pos, + StubCode::GetNullIsAssignableToType(null_safety), + PcDescriptorsLayout::kOther, locs, deopt_id); + // TypeTestABI::kSubtypeTestCacheReg is 0 if the object is null and is + // assignable. + __ BranchIfZero(TypeTestABI::kSubtypeTestCacheReg, &done, + compiler::Assembler::kNearJump); + } else { + __ Comment("AssertAssignable for compile-time type"); + GenerateCallerChecksForAssertAssignable(receiver_type, dst_type, &done); + if (dst_type.IsTypeParameter()) { + // The resolved type parameter is in the scratch register. + type_reg = TypeTestABI::kScratchReg; + } + } - GenerateTTSCall(token_pos, deopt_id, - dst_type.IsTypeParameter() ? TypeTestABI::kScratchReg - : TypeTestABI::kDstTypeReg, - dst_type, dst_name, locs); + GenerateTTSCall(token_pos, deopt_id, type_reg, dst_type, dst_name, locs); __ Bind(&done); } @@ -2789,8 +2822,7 @@ void FlowGraphCompiler::GenerateTTSCall(TokenPosition token_pos, const AbstractType& dst_type, const String& dst_name, LocationSummary* locs) { - // For now, we don't allow dynamic (non-compile-time) dst_type/dst_name. - ASSERT(!dst_type.IsNull() && !dst_name.IsNull()); + ASSERT(!dst_name.IsNull()); // We use 2 consecutive entries in the pool for the subtype cache and the // destination name. The second entry, namely [dst_name] seems to be unused, // but it will be used by the code throwing a TypeError if the type test fails @@ -2804,9 +2836,11 @@ void FlowGraphCompiler::GenerateTTSCall(TokenPosition token_pos, ASSERT((sub_type_cache_index + 1) == dst_name_index); ASSERT(__ constant_pool_allowed()); + __ Comment("TTSCall"); // If the dst_type is known at compile time and instantiated, we know the // target TTS stub and so can use a PC-relative call when available. - if (dst_type.IsInstantiated() && CanPcRelativeCall(dst_type)) { + if (!dst_type.IsNull() && dst_type.IsInstantiated() && + CanPcRelativeCall(dst_type)) { __ LoadWordFromPoolIndex(TypeTestABI::kSubtypeTestCacheReg, sub_type_cache_index); __ GenerateUnRelocatedPcRelativeCall(); diff --git a/runtime/vm/compiler/backend/flow_graph_compiler_ia32.cc b/runtime/vm/compiler/backend/flow_graph_compiler_ia32.cc index c03a43ad300..6b690991a0a 100644 --- a/runtime/vm/compiler/backend/flow_graph_compiler_ia32.cc +++ b/runtime/vm/compiler/backend/flow_graph_compiler_ia32.cc @@ -342,38 +342,62 @@ void FlowGraphCompiler::GenerateAssertAssignable(CompileType* receiver_type, ASSERT(!token_pos.IsClassifying()); ASSERT(CheckAssertAssignableTypeTestingABILocations(*locs)); - if (!locs->in(1).IsConstant()) { - // TODO(dartbug.com/40813): Handle setting up the non-constant case. - UNREACHABLE(); + const auto& dst_type = + locs->in(AssertAssignableInstr::kDstTypePos).IsConstant() + ? AbstractType::Cast( + locs->in(AssertAssignableInstr::kDstTypePos).constant()) + : Object::null_abstract_type(); + + if (!dst_type.IsNull()) { + ASSERT(dst_type.IsFinalized()); + if (dst_type.IsTopTypeForSubtyping()) return; // No code needed. } - ASSERT(locs->in(1).constant().IsAbstractType()); - const auto& dst_type = AbstractType::Cast(locs->in(1).constant()); - ASSERT(dst_type.IsFinalized()); - - if (dst_type.IsTopTypeForSubtyping()) return; // No code needed. - compiler::Label is_assignable, runtime_call; - if (Instance::NullIsAssignableTo(dst_type)) { - const compiler::Immediate& raw_null = - compiler::Immediate(static_cast(Object::null())); - __ cmpl(TypeTestABI::kInstanceReg, raw_null); - __ j(EQUAL, &is_assignable); - } + auto& test_cache = SubtypeTestCache::ZoneHandle(zone()); + if (dst_type.IsNull()) { + __ Comment("AssertAssignable for runtime type"); + // kDstTypeReg should already contain the destination type. + const bool null_safety = Isolate::Current()->null_safety(); + GenerateStubCall(token_pos, + StubCode::GetTypeIsTopTypeForSubtyping(null_safety), + PcDescriptorsLayout::kOther, locs, deopt_id); + // TypeTestABI::kSubtypeTestCacheReg is 0 if the type is a top type. + __ BranchIfZero(TypeTestABI::kSubtypeTestCacheReg, &is_assignable, + compiler::Assembler::kNearJump); - // Generate inline type check, linking to runtime call if not assignable. - SubtypeTestCache& test_cache = SubtypeTestCache::ZoneHandle(zone()); - test_cache = GenerateInlineInstanceof(token_pos, dst_type, &is_assignable, - &runtime_call); + GenerateStubCall(token_pos, + StubCode::GetNullIsAssignableToType(null_safety), + PcDescriptorsLayout::kOther, locs, deopt_id); + // TypeTestABI::kSubtypeTestCacheReg is 0 if the object is null and is + // assignable. + __ BranchIfZero(TypeTestABI::kSubtypeTestCacheReg, &is_assignable, + compiler::Assembler::kNearJump); + + // Use the full-arg version of the cache. + test_cache = GenerateCallSubtypeTestStub(kTestTypeSevenArgs, &is_assignable, + &runtime_call); + } else { + __ Comment("AssertAssignable for compile-time type"); + + if (Instance::NullIsAssignableTo(dst_type)) { + __ CompareObject(TypeTestABI::kInstanceReg, Object::null_object()); + __ BranchIf(EQUAL, &is_assignable); + } + + // Generate inline type check, linking to runtime call if not assignable. + test_cache = GenerateInlineInstanceof(token_pos, dst_type, &is_assignable, + &runtime_call); + } __ Bind(&runtime_call); __ PushObject(Object::null_object()); // Make room for the result. __ pushl(TypeTestABI::kInstanceReg); // Push the source object. - if (locs->in(1).IsConstant()) { - __ PushObject(locs->in(1).constant()); // Push the type of the destination. + // Push the type of the destination. + if (!dst_type.IsNull()) { + __ PushObject(dst_type); } else { - // TODO(dartbug.com/40813): Handle setting up the non-constant case. - UNREACHABLE(); + __ pushl(TypeTestABI::kDstTypeReg); } __ pushl(TypeTestABI::kInstantiatorTypeArgumentsReg); __ pushl(TypeTestABI::kFunctionTypeArgumentsReg); diff --git a/runtime/vm/compiler/backend/il.cc b/runtime/vm/compiler/backend/il.cc index 8d8abf95a9b..4042a0499c0 100644 --- a/runtime/vm/compiler/backend/il.cc +++ b/runtime/vm/compiler/backend/il.cc @@ -1070,8 +1070,8 @@ bool GuardFieldTypeInstr::AttributesEqual(Instruction* other) const { } Instruction* AssertSubtypeInstr::Canonicalize(FlowGraph* flow_graph) { - // If all inputs are constant, we can instantiate the sub and super type and - // remove this instruction if the subtype test succeeds. + // If all inputs needed to check instantation are constant, instantiate the + // sub and super type and remove the instruction if the subtype test succeeds. if (super_type()->BindsToConstant() && sub_type()->BindsToConstant() && instantiator_type_arguments()->BindsToConstant() && function_type_arguments()->BindsToConstant()) { @@ -5286,7 +5286,7 @@ intptr_t AssertAssignableInstr::statistics_tag() const { void AssertAssignableInstr::EmitNativeCode(FlowGraphCompiler* compiler) { compiler->GenerateAssertAssignable(value()->Type(), token_pos(), deopt_id(), dst_name(), locs()); - ASSERT(locs()->in(0).reg() == locs()->out(0).reg()); + ASSERT(locs()->in(kInstancePos).reg() == locs()->out(0).reg()); } LocationSummary* AssertSubtypeInstr::MakeLocationSummary(Zone* zone, diff --git a/runtime/vm/compiler/backend/il.h b/runtime/vm/compiler/backend/il.h index 6e4fe721faf..c1a04461959 100644 --- a/runtime/vm/compiler/backend/il.h +++ b/runtime/vm/compiler/backend/il.h @@ -3620,6 +3620,13 @@ class AssertAssignableInstr : public TemplateDefinition<4, Throws, Pure> { static const char* KindToCString(Kind kind); static bool ParseKind(const char* str, Kind* out); + enum { + kInstancePos = 0, + kDstTypePos = 1, + kInstantiatorTAVPos = 2, + kFunctionTAVPos = 3, + }; + AssertAssignableInstr(TokenPosition token_pos, Value* value, Value* dst_type, @@ -3633,10 +3640,10 @@ class AssertAssignableInstr : public TemplateDefinition<4, Throws, Pure> { dst_name_(dst_name), kind_(kind) { ASSERT(!dst_name.IsNull()); - SetInputAt(0, value); - SetInputAt(1, dst_type); - SetInputAt(2, instantiator_type_arguments); - SetInputAt(3, function_type_arguments); + SetInputAt(kInstancePos, value); + SetInputAt(kDstTypePos, dst_type); + SetInputAt(kInstantiatorTAVPos, instantiator_type_arguments); + SetInputAt(kFunctionTAVPos, function_type_arguments); } virtual intptr_t statistics_tag() const; @@ -3645,10 +3652,12 @@ class AssertAssignableInstr : public TemplateDefinition<4, Throws, Pure> { virtual CompileType ComputeType() const; virtual bool RecomputeType(); - Value* value() const { return inputs_[0]; } - Value* dst_type() const { return inputs_[1]; } - Value* instantiator_type_arguments() const { return inputs_[2]; } - Value* function_type_arguments() const { return inputs_[3]; } + Value* value() const { return inputs_[kInstancePos]; } + Value* dst_type() const { return inputs_[kDstTypePos]; } + Value* instantiator_type_arguments() const { + return inputs_[kInstantiatorTAVPos]; + } + Value* function_type_arguments() const { return inputs_[kFunctionTAVPos]; } virtual TokenPosition token_pos() const { return token_pos_; } const String& dst_name() const { return dst_name_; } diff --git a/runtime/vm/compiler/backend/il_arm.cc b/runtime/vm/compiler/backend/il_arm.cc index b7fd68e65c4..04b1857fad5 100644 --- a/runtime/vm/compiler/backend/il_arm.cc +++ b/runtime/vm/compiler/backend/il_arm.cc @@ -790,12 +790,14 @@ LocationSummary* AssertAssignableInstr::MakeLocationSummary(Zone* zone, LocationSummary* summary = new (zone) LocationSummary( zone, kNumInputs, kNumTemps, LocationSummary::kCallCalleeSafe); - summary->set_in(0, Location::RegisterLocation(TypeTestABI::kInstanceReg)); - summary->set_in(1, dst_type_loc); - summary->set_in(2, Location::RegisterLocation( - TypeTestABI::kInstantiatorTypeArgumentsReg)); + summary->set_in(kInstancePos, + Location::RegisterLocation(TypeTestABI::kInstanceReg)); + summary->set_in(kDstTypePos, dst_type_loc); summary->set_in( - 3, Location::RegisterLocation(TypeTestABI::kFunctionTypeArgumentsReg)); + kInstantiatorTAVPos, + Location::RegisterLocation(TypeTestABI::kInstantiatorTypeArgumentsReg)); + summary->set_in(kFunctionTAVPos, Location::RegisterLocation( + TypeTestABI::kFunctionTypeArgumentsReg)); summary->set_out(0, Location::SameAsFirstInput()); // Let's reserve all registers except for the input ones. diff --git a/runtime/vm/compiler/backend/il_arm64.cc b/runtime/vm/compiler/backend/il_arm64.cc index eeebf97201b..50100ead94c 100644 --- a/runtime/vm/compiler/backend/il_arm64.cc +++ b/runtime/vm/compiler/backend/il_arm64.cc @@ -688,12 +688,14 @@ LocationSummary* AssertAssignableInstr::MakeLocationSummary(Zone* zone, LocationSummary* summary = new (zone) LocationSummary( zone, kNumInputs, kNumTemps, LocationSummary::kCallCalleeSafe); - summary->set_in(0, Location::RegisterLocation(TypeTestABI::kInstanceReg)); - summary->set_in(1, dst_type_loc); - summary->set_in(2, Location::RegisterLocation( - TypeTestABI::kInstantiatorTypeArgumentsReg)); + summary->set_in(kInstancePos, + Location::RegisterLocation(TypeTestABI::kInstanceReg)); + summary->set_in(kDstTypePos, dst_type_loc); summary->set_in( - 3, Location::RegisterLocation(TypeTestABI::kFunctionTypeArgumentsReg)); + kInstantiatorTAVPos, + Location::RegisterLocation(TypeTestABI::kInstantiatorTypeArgumentsReg)); + summary->set_in(kFunctionTAVPos, Location::RegisterLocation( + TypeTestABI::kFunctionTypeArgumentsReg)); summary->set_out(0, Location::SameAsFirstInput()); // Let's reserve all registers except for the input ones. diff --git a/runtime/vm/compiler/backend/il_ia32.cc b/runtime/vm/compiler/backend/il_ia32.cc index 54356ba42d6..985299ef772 100644 --- a/runtime/vm/compiler/backend/il_ia32.cc +++ b/runtime/vm/compiler/backend/il_ia32.cc @@ -496,13 +496,15 @@ LocationSummary* AssertAssignableInstr::MakeLocationSummary(Zone* zone, const intptr_t kNumTemps = 0; LocationSummary* summary = new (zone) LocationSummary(zone, kNumInputs, kNumTemps, LocationSummary::kCall); - summary->set_in(0, Location::RegisterLocation(TypeTestABI::kInstanceReg)); + summary->set_in(kInstancePos, + Location::RegisterLocation(TypeTestABI::kInstanceReg)); + summary->set_in(kDstTypePos, LocationFixedRegisterOrConstant( + dst_type(), TypeTestABI::kDstTypeReg)); summary->set_in( - 1, LocationFixedRegisterOrConstant(dst_type(), TypeTestABI::kDstTypeReg)); - summary->set_in(2, Location::RegisterLocation( - TypeTestABI::kInstantiatorTypeArgumentsReg)); - summary->set_in( - 3, Location::RegisterLocation(TypeTestABI::kFunctionTypeArgumentsReg)); + kInstantiatorTAVPos, + Location::RegisterLocation(TypeTestABI::kInstantiatorTypeArgumentsReg)); + summary->set_in(kFunctionTAVPos, Location::RegisterLocation( + TypeTestABI::kFunctionTypeArgumentsReg)); summary->set_out(0, Location::SameAsFirstInput()); return summary; } diff --git a/runtime/vm/compiler/backend/il_x64.cc b/runtime/vm/compiler/backend/il_x64.cc index 92d820507d0..34af406e547 100644 --- a/runtime/vm/compiler/backend/il_x64.cc +++ b/runtime/vm/compiler/backend/il_x64.cc @@ -627,12 +627,14 @@ LocationSummary* AssertAssignableInstr::MakeLocationSummary(Zone* zone, LocationSummary* summary = new (zone) LocationSummary( zone, kNumInputs, kNumTemps, LocationSummary::kCallCalleeSafe); - summary->set_in(0, Location::RegisterLocation(TypeTestABI::kInstanceReg)); - summary->set_in(1, dst_type_loc); - summary->set_in(2, Location::RegisterLocation( - TypeTestABI::kInstantiatorTypeArgumentsReg)); + summary->set_in(kInstancePos, + Location::RegisterLocation(TypeTestABI::kInstanceReg)); + summary->set_in(kDstTypePos, dst_type_loc); summary->set_in( - 3, Location::RegisterLocation(TypeTestABI::kFunctionTypeArgumentsReg)); + kInstantiatorTAVPos, + Location::RegisterLocation(TypeTestABI::kInstantiatorTypeArgumentsReg)); + summary->set_in(kFunctionTAVPos, Location::RegisterLocation( + TypeTestABI::kFunctionTypeArgumentsReg)); summary->set_out(0, Location::SameAsFirstInput()); // Let's reserve all registers except for the input ones. diff --git a/runtime/vm/compiler/backend/range_analysis.cc b/runtime/vm/compiler/backend/range_analysis.cc index 4018099e7f3..bc8f0ca861c 100644 --- a/runtime/vm/compiler/backend/range_analysis.cc +++ b/runtime/vm/compiler/backend/range_analysis.cc @@ -2766,10 +2766,7 @@ void LoadFieldInstr::InferRange(RangeAnalysis* analysis, Range* range) { break; case Slot::Kind::kClosureData_default_type_arguments_info: - *range = Range( - RangeBoundary::FromConstant(0), - RangeBoundary::FromConstant( - (1 << Function::NumParentTypeParametersField::kNextBit) - 1)); + *range = Range(RangeBoundary::FromConstant(0), RangeBoundary::MaxSmi()); } } diff --git a/runtime/vm/compiler/backend/typed_data_aot_test.cc b/runtime/vm/compiler/backend/typed_data_aot_test.cc index 30d58db548b..72f41e393ad 100644 --- a/runtime/vm/compiler/backend/typed_data_aot_test.cc +++ b/runtime/vm/compiler/backend/typed_data_aot_test.cc @@ -491,10 +491,6 @@ ISOLATE_UNIT_TEST_CASE(IRTest_TypedDataAOT_Regress43534) { ILMatcher cursor(flow_graph, entry, /*trace=*/true); RELEASE_ASSERT(cursor.TryMatch( { - kMatchAndMoveGoto, - kMatchAndMoveBranchFalse, - kMatchAndMoveAssertAssignable, - kMatchAndMoveGoto, kMatchAndMoveLoadField, kMatchAndMoveGenericCheckBound, kMatchAndMoveLoadUntagged, diff --git a/runtime/vm/compiler/compiler_sources.gni b/runtime/vm/compiler/compiler_sources.gni index ad10b9cd4de..1496c09e62e 100644 --- a/runtime/vm/compiler/compiler_sources.gni +++ b/runtime/vm/compiler/compiler_sources.gni @@ -181,7 +181,6 @@ compiler_sources_tests = [ "cha_test.cc", "ffi/native_type_vm_test.cc", "frontend/kernel_binary_flowgraph_test.cc", - "frontend/multiple_entrypoints_test.cc", "write_barrier_elimination_test.cc", ] diff --git a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc index c5a05cd8e99..397daafc8ff 100644 --- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc +++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc @@ -722,31 +722,6 @@ Fragment StreamingFlowGraphBuilder::SetupCapturedParameters( return body; } -// If we run in checked mode or strong mode, we have to check the type of the -// passed arguments. -// -// TODO(#34162): If we're building an extra entry-point to skip -// type checks, we should substitute Redefinition nodes for the AssertAssignable -// instructions to ensure that the argument types are known. -void StreamingFlowGraphBuilder::CheckArgumentTypesAsNecessary( - const Function& dart_function, - intptr_t type_parameters_offset, - Fragment* explicit_checks, - Fragment* implicit_checks, - Fragment* implicit_redefinitions) { - if (dart_function.NeedsTypeArgumentTypeChecks()) { - B->BuildTypeArgumentTypeChecks( - MethodCanSkipTypeChecksForNonCovariantTypeArguments(dart_function) - ? TypeChecksToBuild::kCheckCovariantTypeParameterBounds - : TypeChecksToBuild::kCheckAllTypeParameterBounds, - implicit_checks); - } - if (dart_function.NeedsArgumentTypeChecks()) { - B->BuildArgumentTypeChecks(explicit_checks, implicit_checks, - implicit_redefinitions); - } -} - Fragment StreamingFlowGraphBuilder::ShortcutForUserDefinedEquals( const Function& dart_function, LocalVariable* first_parameter) { @@ -948,12 +923,21 @@ FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfFunction( // regular methods. const Fragment type_args_handling = TypeArgumentsHandling(dart_function); - Fragment explicit_type_checks; Fragment implicit_type_checks; + if (dart_function.NeedsTypeArgumentTypeChecks()) { + B->BuildTypeArgumentTypeChecks( + dart_function.CanReceiveDynamicInvocation() + ? TypeChecksToBuild::kCheckAllTypeParameterBounds + : TypeChecksToBuild::kCheckCovariantTypeParameterBounds, + &implicit_type_checks); + } + + Fragment explicit_type_checks; Fragment implicit_redefinitions; - CheckArgumentTypesAsNecessary(dart_function, type_parameters_offset, - &explicit_type_checks, &implicit_type_checks, - &implicit_redefinitions); + if (dart_function.NeedsArgumentTypeChecks()) { + B->BuildArgumentTypeChecks(&explicit_type_checks, &implicit_type_checks, + &implicit_redefinitions); + } // The RawParameter variables should be set to null to avoid retaining more // objects than necessary during GC. diff --git a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h index ef94bf30b28..175cc4ec25c 100644 --- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h +++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h @@ -94,11 +94,6 @@ class StreamingFlowGraphBuilder : public KernelReaderHelper { Fragment ShortcutForUserDefinedEquals(const Function& dart_function, LocalVariable* first_parameter); Fragment TypeArgumentsHandling(const Function& dart_function); - void CheckArgumentTypesAsNecessary(const Function& dart_function, - intptr_t type_parameters_offset, - Fragment* explicit_checks, - Fragment* implicit_checks, - Fragment* implicit_redefinitions); Fragment CompleteBodyWithYieldContinuations(Fragment body); static UncheckedEntryPointStyle ChooseEntryPointStyle( diff --git a/runtime/vm/compiler/frontend/kernel_to_il.cc b/runtime/vm/compiler/frontend/kernel_to_il.cc index af86e1ef6be..a8bde81ab94 100644 --- a/runtime/vm/compiler/frontend/kernel_to_il.cc +++ b/runtime/vm/compiler/frontend/kernel_to_il.cc @@ -1998,11 +1998,11 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfNoSuchMethodDispatcher( } String& name = String::Handle(Z); for (intptr_t i = 0; i < descriptor.NamedCount(); ++i) { - intptr_t parameter_index = descriptor.PositionalCount() + i; + const intptr_t parameter_index = descriptor.PositionAt(i); name = descriptor.NameAt(i); name = Symbols::New(H.thread(), name); body += LoadLocal(array); - body += IntConstant(receiver_index + descriptor.PositionAt(i)); + body += IntConstant(receiver_index + parameter_index); body += LoadLocal(parsed_function_->ParameterVariable(parameter_index)); body += StoreIndexed(kArrayCid); } @@ -2069,6 +2069,7 @@ struct FlowGraphBuilder::ClosureCallInfo { LocalVariable* num_max_params = nullptr; LocalVariable* has_named_params = nullptr; LocalVariable* parameter_names = nullptr; + LocalVariable* parameter_types = nullptr; LocalVariable* type_parameters = nullptr; LocalVariable* closure_data = nullptr; LocalVariable* default_tav_info = nullptr; @@ -2349,10 +2350,13 @@ Fragment FlowGraphBuilder::BuildClosureCallNamedArgumentsCheck( loop_body += BranchIfEqual(&match, &mismatch); loop_body.current = mismatch; - // We have a match, so go to the next name after incrementing the number - // of matched arguments. (No need to check for the required bit, as this - // parameter was provided.) + // We have a match, so go to the next name after storing the corresponding + // parameter index on the stack and incrementing the number of matched + // arguments. (No need to check the required bit for provided parameters.) Fragment matched(match); + matched += LoadLocal(info.vars->current_param_index); + matched += StoreLocal(info.vars->named_argument_parameter_indices.At(i)); + matched += Drop(); matched += LoadLocal(info.vars->current_num_processed); matched += IntConstant(1); matched += SmiBinaryOp(Token::kADD, /*is_truncating=*/true); @@ -2566,6 +2570,60 @@ Fragment FlowGraphBuilder::BuildClosureCallTypeArgumentsTypeCheck( return Fragment(loop_init.entry, done); } +Fragment FlowGraphBuilder::BuildClosureCallArgumentTypeCheck( + const ClosureCallInfo& info, + LocalVariable* param_index, + intptr_t arg_index, + const String& arg_name) { + Fragment instructions; + + // Load value. + instructions += LoadLocal(parsed_function_->ParameterVariable(arg_index)); + // Load destination type. + instructions += LoadLocal(info.parameter_types); + instructions += LoadLocal(param_index); + instructions += LoadIndexed(kArrayCid); + // Load instantiator type arguments. + instructions += LoadLocal(info.instantiator_type_args); + // Load the full set of function type arguments. + instructions += LoadLocal(info.vars->function_type_args); + // Check that the value has the right type. + instructions += AssertAssignable(TokenPosition::kNoSource, arg_name, + AssertAssignableInstr::kParameterCheck); + // Make sure to store the result to keep data dependencies accurate. + instructions += StoreLocal(parsed_function_->ParameterVariable(arg_index)); + instructions += Drop(); + + return instructions; +} + +Fragment FlowGraphBuilder::BuildClosureCallArgumentTypeChecks( + const ClosureCallInfo& info) { + Fragment instructions; + + // Only check explicit arguments (i.e., skip the receiver), as the receiver + // is always assignable to its type (stored as dynamic). + for (intptr_t i = 1; i < info.descriptor.PositionalCount(); i++) { + instructions += IntConstant(i); + LocalVariable* param_index = MakeTemporary("param_index"); + // We don't have a compile-time name, so this symbol signals the runtime + // that it should recreate the type check using info from the stack. + instructions += BuildClosureCallArgumentTypeCheck( + info, param_index, i, Symbols::dynamic_assert_assignable_stc_check()); + instructions += DropTemporary(¶m_index); + } + + for (intptr_t i = 0; i < info.descriptor.NamedCount(); i++) { + const intptr_t arg_index = info.descriptor.PositionAt(i); + const auto& arg_name = String::ZoneHandle(Z, info.descriptor.NameAt(i)); + auto const param_index = info.vars->named_argument_parameter_indices.At(i); + instructions += BuildClosureCallArgumentTypeCheck(info, param_index, + arg_index, arg_name); + } + + return instructions; +} + Fragment FlowGraphBuilder::BuildDynamicClosureCallChecks( LocalVariable* closure) { ClosureCallInfo info(closure, BuildThrowNoSuchMethod(), @@ -2601,11 +2659,13 @@ Fragment FlowGraphBuilder::BuildDynamicClosureCallChecks( body += StrictCompare(Token::kNE_STRICT); info.has_named_params = MakeTemporary("has_named_params"); - if (I->null_safety() || info.descriptor.NamedCount() > 0) { - body += LoadLocal(info.function); - body += LoadNativeField(Slot::Function_parameter_names()); - info.parameter_names = MakeTemporary("parameter_names"); - } + body += LoadLocal(info.function); + body += LoadNativeField(Slot::Function_parameter_names()); + info.parameter_names = MakeTemporary("parameter_names"); + + body += LoadLocal(info.function); + body += LoadNativeField(Slot::Function_parameter_types()); + info.parameter_types = MakeTemporary("parameter_types"); body += LoadLocal(info.function); body += LoadNativeField(Slot::Function_type_parameters()); @@ -2688,18 +2748,16 @@ Fragment FlowGraphBuilder::BuildDynamicClosureCallChecks( // and performing any needed type argument checking. body += TestClosureFunctionGeneric(info, generic, not_generic); - // TODO(dartbug.com/40813): Move checks that are currently compiled - // in the closure body to here, using the dynamic versions of - // AssertAssignable to typecheck the parameters using the runtime types - // available in the closure object. + // Check that the values provided as arguments are assignable to the types + // of the corresponding closure function parameters. + body += BuildClosureCallArgumentTypeChecks(info); // Drop all the read-only temporaries at the end of the fragment. body += DropTemporary(&info.parent_function_type_args); body += DropTemporary(&info.instantiator_type_args); body += DropTemporary(&info.type_parameters); - if (info.parameter_names != nullptr) { - body += DropTemporary(&info.parameter_names); - } + body += DropTemporary(&info.parameter_types); + body += DropTemporary(&info.parameter_names); body += DropTemporary(&info.has_named_params); body += DropTemporary(&info.num_max_params); body += DropTemporary(&info.num_opt_params); @@ -2793,8 +2851,10 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfInvokeFieldDispatcher( Array::ZoneHandle(Z, Array::New(descriptor.NamedCount(), Heap::kNew)); String& string_handle = String::Handle(Z); for (intptr_t i = 0; i < descriptor.NamedCount(); ++i) { + const intptr_t named_arg_index = + descriptor.PositionAt(i) - descriptor.PositionalCount(); string_handle = descriptor.NameAt(i); - array_handle.SetAt(i, string_handle); + array_handle.SetAt(named_arg_index, string_handle); } argument_names = &array_handle; } @@ -3216,49 +3276,38 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfImplicitClosureFunction( BlockEntryInstr* instruction_cursor = BuildPrologue(normal_entry, &prologue_info); - const Fragment prologue = CheckStackOverflowInPrologue(function.token_pos()); + Fragment closure(instruction_cursor); + closure += CheckStackOverflowInPrologue(function.token_pos()); + closure += BuildDefaultTypeHandling(function); - const Fragment default_type_handling = BuildDefaultTypeHandling(function); - - Fragment implicit_checks; - if (function.NeedsTypeArgumentTypeChecks() && - (target.is_static() || - MethodCanSkipTypeChecksForNonCovariantTypeArguments(parent))) { - BuildTypeArgumentTypeChecks( - target.is_static() - ? TypeChecksToBuild::kCheckAllTypeParameterBounds - : TypeChecksToBuild::kCheckNonCovariantTypeParameterBounds, - &implicit_checks); - } - if (function.NeedsArgumentTypeChecks() && - (target.is_static() || - MethodCanSkipTypeChecksForNonCovariantArguments(parent))) { - // We're going to throw away the explicit checks because the target will - // always check them. - Fragment explicit_checks_unused; - BuildArgumentTypeChecks(&explicit_checks_unused, &implicit_checks, nullptr); - } - - Fragment body; + // For implicit closure functions, any non-covariant checks are either + // performed by the type system or a dynamic invocation layer (dynamic closure + // call dispatcher, mirror, etc.). Static targets never have covariant + // arguments, and for non-static targets, they already perform the covariant + // checks internally. Thus, no checks are needed and we just need to invoke + // the target with the right receiver (unless static). + // + // TODO(dartbug.com/44195): Consider replacing the argument pushes + static + // call with stack manipulation and a tail call instead. intptr_t type_args_len = 0; if (function.IsGeneric()) { type_args_len = function.NumTypeParameters(); ASSERT(parsed_function_->function_type_arguments() != NULL); - body += LoadLocal(parsed_function_->function_type_arguments()); + closure += LoadLocal(parsed_function_->function_type_arguments()); } // Push receiver. if (!target.is_static()) { // The context has a fixed shape: a single variable which is the // closed-over receiver. - body += LoadLocal(parsed_function_->ParameterVariable(0)); - body += LoadNativeField(Slot::Closure_context()); - body += LoadNativeField(Slot::GetContextVariableSlotFor( + closure += LoadLocal(parsed_function_->ParameterVariable(0)); + closure += LoadNativeField(Slot::Closure_context()); + closure += LoadNativeField(Slot::GetContextVariableSlotFor( thread_, *parsed_function_->receiver_var())); } - body += PushExplicitParameters(function); + closure += PushExplicitParameters(function); // Forward parameters to the target. intptr_t argument_count = function.NumParameters() - @@ -3269,48 +3318,12 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfImplicitClosureFunction( Array& argument_names = Array::ZoneHandle(Z, GetOptionalParameterNames(function)); - body += StaticCall(TokenPosition::kNoSource, target, argument_count, - argument_names, ICData::kNoRebind, - /* result_type = */ NULL, type_args_len); + closure += StaticCall(TokenPosition::kNoSource, target, argument_count, + argument_names, ICData::kNoRebind, + /* result_type = */ NULL, type_args_len); // Return the result. - body += Return(function.end_token_pos()); - - // Setup multiple entrypoints if useful. - FunctionEntryInstr* extra_entry = nullptr; - if (function.MayHaveUncheckedEntryPoint()) { - // The prologue for a closure will always have context handling (e.g. - // setting up the receiver variable), but we don't need it on the unchecked - // entry because the only time we reference this is for loading the - // receiver, which we fetch directly from the context. - if (PrologueBuilder::PrologueSkippableOnUncheckedEntry(function)) { - // Use separate entry points since we can skip almost everything on the - // static entry. - extra_entry = BuildSeparateUncheckedEntryPoint( - /*normal_entry=*/instruction_cursor, - /*normal_prologue=*/prologue + default_type_handling + - implicit_checks, - /*extra_prologue=*/ - CheckStackOverflowInPrologue(function.token_pos()), - /*shared_prologue=*/Fragment(), - /*body=*/body); - } else { - Fragment shared_prologue(normal_entry, instruction_cursor); - shared_prologue += prologue; - extra_entry = BuildSharedUncheckedEntryPoint( - /*shared_prologue_linked_in=*/shared_prologue, - /*skippable_checks=*/default_type_handling + implicit_checks, - /*redefinitions_if_skipped=*/Fragment(), - /*body=*/body); - } - RecordUncheckedEntryPoint(graph_entry_, extra_entry); - } else { - Fragment function(instruction_cursor); - function += prologue; - function += default_type_handling; - function += implicit_checks; - function += body; - } + closure += Return(function.end_token_pos()); return new (Z) FlowGraph(*parsed_function_, graph_entry_, last_used_block_id_, prologue_info); diff --git a/runtime/vm/compiler/frontend/kernel_to_il.h b/runtime/vm/compiler/frontend/kernel_to_il.h index cd2374480be..6d4eaa8fc68 100644 --- a/runtime/vm/compiler/frontend/kernel_to_il.h +++ b/runtime/vm/compiler/frontend/kernel_to_il.h @@ -120,6 +120,20 @@ class FlowGraphBuilder : public BaseFlowGraphBuilder { // function is generic. Fragment BuildClosureCallTypeArgumentsTypeCheck(const ClosureCallInfo& info); + // Builds checks for type checking a given argument of the closure call using + // parameter information from the closure function retrieved at runtime. + // + // For named arguments, arg_name is a compile-time constant retrieved from + // the saved arguments descriptor. For positional arguments, null is passed. + Fragment BuildClosureCallArgumentTypeCheck(const ClosureCallInfo& info, + LocalVariable* param_index, + intptr_t arg_index, + const String& arg_name); + + // Builds checks for type checking the arguments of a call using parameter + // information for the function retrieved at runtime from the closure. + Fragment BuildClosureCallArgumentTypeChecks(const ClosureCallInfo& info); + // Main entry point for building checks. Fragment BuildDynamicClosureCallChecks(LocalVariable* closure); diff --git a/runtime/vm/compiler/frontend/multiple_entrypoints_test.cc b/runtime/vm/compiler/frontend/multiple_entrypoints_test.cc deleted file mode 100644 index 9ae1679e2ca..00000000000 --- a/runtime/vm/compiler/frontend/multiple_entrypoints_test.cc +++ /dev/null @@ -1,106 +0,0 @@ -// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file -// 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. - -#include "vm/compiler/backend/il_test_helper.h" -#include "vm/compiler/compiler_pass.h" -#include "vm/object.h" -#include "vm/unit_test.h" - -namespace dart { - -#if defined(DART_PRECOMPILER) - -ISOLATE_UNIT_TEST_CASE(IRTest_MultilpeEntryPoints_Regress43534) { - const char* kScript = - R"( - import 'dart:typed_data'; - - @pragma('vm:never-inline') - void callWith(void Function(T arg) fun, T arg) { - fun(arg); - } - - @pragma('vm:never-inline') - void use(dynamic arg) {} - - void test() { - callWith((Uint8List list) { - use(list); - }, Uint8List(10)); - } - )"; - - const auto& root_library = Library::Handle(LoadTestScript(kScript)); - Invoke(root_library, "test"); - const auto& test_function = - Function::Handle(GetFunction(root_library, "test")); - const auto& closures = GrowableObjectArray::Handle( - Isolate::Current()->object_store()->closure_functions()); - auto& function = Function::Handle(); - for (intptr_t i = closures.Length() - 1; 0 <= i; ++i) { - function ^= closures.At(i); - if (function.parent_function() == test_function.raw()) { - break; - } - function = Function::null(); - } - RELEASE_ASSERT(!function.IsNull()); - TestPipeline pipeline(function, CompilerPass::kAOT); - FlowGraph* flow_graph = pipeline.RunPasses({CompilerPass::kComputeSSA}); - - auto entry = flow_graph->graph_entry()->normal_entry(); - EXPECT(entry != nullptr); - - auto unchecked_entry = flow_graph->graph_entry()->unchecked_entry(); - EXPECT(unchecked_entry != nullptr); - - AssertAssignableInstr* assert_assignable = nullptr; - StaticCallInstr* static_call = nullptr; - - // Normal entry - ILMatcher cursor(flow_graph, entry, /*trace=*/true); - RELEASE_ASSERT(cursor.TryMatch( - { - kMatchAndMoveGoto, - kMatchAndMoveBranchFalse, - {kMatchAndMoveAssertAssignable, &assert_assignable}, - kMatchAndMoveGoto, - {kMatchAndMoveStaticCall, &static_call}, - kMatchReturn, - }, - kMoveGlob)); - - RedefinitionInstr* redefinition = nullptr; - StaticCallInstr* static_call2 = nullptr; - - // Unchecked entry - ILMatcher cursor2(flow_graph, entry, /*trace=*/true); - RELEASE_ASSERT(cursor2.TryMatch( - { - kMatchAndMoveGoto, - kMatchAndMoveBranchTrue, - {kMatchAndMoveRedefinition, &redefinition}, - kMatchAndMoveGoto, - {kMatchAndMoveStaticCall, &static_call2}, - kMatchReturn, - }, - kMoveGlob)); - - // Ensure the value the static call uses is a Phi node with 2 inputs: - // a) Normal entry: AssertAssignable - // b) Unchecked entry: Redefinition - RELEASE_ASSERT(static_call->ArgumentAt(0)->IsPhi()); - auto phi = static_call->ArgumentAt(0)->AsPhi(); - auto input_a = phi->InputAt(0)->definition(); - auto input_b = phi->InputAt(1)->definition(); - RELEASE_ASSERT(input_a->IsRedefinition()); - RELEASE_ASSERT(input_b->IsAssertAssignable()); - RELEASE_ASSERT(input_a == redefinition); - RELEASE_ASSERT(input_b == assert_assignable); - RELEASE_ASSERT(static_call == static_call2); -} - -#endif // defined(DART_PRECOMPILER) - -} // namespace dart diff --git a/runtime/vm/compiler/frontend/scope_builder.cc b/runtime/vm/compiler/frontend/scope_builder.cc index 4470cee1c1c..ab4762ad0a0 100644 --- a/runtime/vm/compiler/frontend/scope_builder.cc +++ b/runtime/vm/compiler/frontend/scope_builder.cc @@ -15,44 +15,6 @@ namespace kernel { #define T (type_translator_) #define I Isolate::Current() -// Returns true if the given method can skip type checks for all type arguments -// that are not covariant or generic covariant in its implementation. -bool MethodCanSkipTypeChecksForNonCovariantTypeArguments( - const Function& method) { - // Dart 2 type system at non-dynamic call sites statically guarantees that - // argument values match declared parameter types for all non-covariant - // and non-generic-covariant parameters. The same applies to type parameters - // bounds for type parameters of generic functions. - // - // Normally dynamic call sites will call dyn:* forwarders which perform type - // checks. - // - // Though for some kinds of methods (e.g. ffi trampolines called from native - // code) we do have to perform type checks for all parameters. - return !method.CanReceiveDynamicInvocation(); -} - -// Returns true if the given method can skip type checks for all arguments -// that are not covariant or generic covariant in its implementation. -bool MethodCanSkipTypeChecksForNonCovariantArguments(const Function& method) { - // Dart 2 type system at non-dynamic call sites statically guarantees that - // argument values match declarated parameter types for all non-covariant - // and non-generic-covariant parameters. The same applies to type parameters - // bounds for type parameters of generic functions. - // - // Normally dynamic call sites will call dyn:* forwarders which perform type - // checks. - // - // Though for some kinds of methods (e.g. ffi trampolines called from native - // code) we do have to perform type checks for all parameters. - // - // TODO(dartbug.com/40813): Remove the closure case when argument checks have - // been fully moved out of closures. - return !method.CanReceiveDynamicInvocation() && - !(method.IsClosureFunction() && - Function::ClosureBodiesContainNonCovariantArgumentChecks()); -} - ScopeBuilder::ScopeBuilder(ParsedFunction* parsed_function) : result_(NULL), parsed_function_(parsed_function), @@ -134,7 +96,6 @@ ScopeBuildingResult* ScopeBuilder::BuildScopes() { } if (parsed_function_->has_arg_desc_var()) { - needs_expr_temp_ = true; scope_->AddVariable(parsed_function_->arg_desc_var()); } @@ -230,47 +191,34 @@ ScopeBuildingResult* ScopeBuilder::BuildScopes() { result_->type_arguments_variable = variable; } - ParameterTypeCheckMode type_check_mode = kTypeCheckAllParameters; + ParameterTypeCheckMode type_check_mode = + kTypeCheckForNonDynamicallyInvokedMethod; if (function.IsSyncGenClosure()) { // Don't type check the parameter of sync-yielding since these calls are // all synthetic and types should always match. - ASSERT((function.NumParameters() - function.NumImplicitParameters()) == - 3); + ASSERT_EQUAL( + function.NumParameters() - function.NumImplicitParameters(), 3); ASSERT( Class::Handle( AbstractType::Handle(function.ParameterTypeAt(1)).type_class()) .ScrubbedName() == Symbols::_SyncIterator().raw()); type_check_mode = kTypeCheckForStaticFunction; - } else if (function.IsNonImplicitClosureFunction()) { - type_check_mode = kTypeCheckAllParameters; + } else if (function.is_static()) { + // In static functions we don't check anything. + type_check_mode = kTypeCheckForStaticFunction; } else if (function.IsImplicitClosureFunction()) { - if (MethodCanSkipTypeChecksForNonCovariantTypeArguments( - Function::Handle(Z, function.parent_function())) && - MethodCanSkipTypeChecksForNonCovariantArguments( - Function::Handle(Z, function.parent_function()))) { - // This is a tear-off of an instance method that can not be reached - // from any dynamic invocation. The method would not check any - // parameters except covariant ones and those annotated with - // generic-covariant-impl. Which means that we have to check - // the rest in the tear-off itself. - type_check_mode = - kTypeCheckEverythingNotCheckedInNonDynamicallyInvokedMethod; - } - } else { - if (function.is_static()) { - // In static functions we don't check anything. - type_check_mode = kTypeCheckForStaticFunction; - } else if (MethodCanSkipTypeChecksForNonCovariantTypeArguments( - function) && - MethodCanSkipTypeChecksForNonCovariantArguments(function)) { - // If the current function is never a target of a dynamic invocation - // and this parameter is not marked with generic-covariant-impl - // (which means that among all super-interfaces no type parameters - // ever occur at the position of this parameter) then we don't need - // to check this parameter on the callee side, because strong mode - // guarantees that it was checked at the caller side. - type_check_mode = kTypeCheckForNonDynamicallyInvokedMethod; - } + // All non-covariant checks are either performed by the type system, + // or by a dynamic closure call dispatcher/mirror if dynamically + // invoked. For covariant checks, static targets never have covariant + // arguments and dynamic targets do their own covariant checking. + // Thus, implicit closure functions perform no checking internally. + type_check_mode = kTypeCheckForImplicitClosureFunction; + } else if (function.CanReceiveDynamicInvocation()) { + // If the current function can be the direct target of a dynamic + // invocation, that is, dynamic calls do not go through a dynamic + // invocation forwarder or dynamic closure call dispatcher, then we must + // check non-covariant parameters as well as covariant ones. + type_check_mode = kTypeCheckAllParameters; } // Continue reading FunctionNode: @@ -336,9 +284,7 @@ ScopeBuildingResult* ScopeBuilder::BuildScopes() { } scope_->InsertParameterAt(pos++, result_->setter_value); - if (is_method && - MethodCanSkipTypeChecksForNonCovariantTypeArguments(function) && - MethodCanSkipTypeChecksForNonCovariantArguments(function)) { + if (is_method && !function.CanReceiveDynamicInvocation()) { if (field.is_covariant()) { result_->setter_value->set_is_explicit_covariant_parameter(); } else if (!field.is_generic_covariant_impl() || @@ -460,6 +406,9 @@ ScopeBuildingResult* ScopeBuilder::BuildScopes() { #define ADD_VAR(Name, _, __) scope_->AddVariable(vars->Name); FOR_EACH_DYNAMIC_CLOSURE_CALL_VARIABLE(ADD_VAR); #undef ADD_VAR + for (auto const& v : vars->named_argument_parameter_indices) { + scope_->AddVariable(v); + } } } FALL_THROUGH; @@ -480,7 +429,10 @@ ScopeBuildingResult* ScopeBuilder::BuildScopes() { UNREACHABLE(); } if (needs_expr_temp_) { - scope_->AddVariable(parsed_function_->EnsureExpressionTemp()); + parsed_function_->EnsureExpressionTemp(); + } + if (parsed_function_->has_expression_temp_var()) { + scope_->AddVariable(parsed_function_->expression_temp_var()); } if (parsed_function_->function().MayHaveUncheckedEntryPoint()) { scope_->AddVariable(parsed_function_->EnsureEntryPointsTemp()); @@ -1539,7 +1491,8 @@ void ScopeBuilder::HandleLocalFunction(intptr_t parent_kernel_offset) { FunctionNodeHelper::kPositionalParameters); ProcedureAttributesMetadata default_attrs; - AddPositionalAndNamedParameters(0, kTypeCheckAllParameters, default_attrs); + AddPositionalAndNamedParameters(0, kTypeCheckForNonDynamicallyInvokedMethod, + default_attrs); // "Peek" is now done. helper_.SetOffset(offset); @@ -1639,6 +1592,17 @@ void ScopeBuilder::AddVariableDeclarationParameter( variable->set_type_check_mode(LocalVariable::kTypeCheckedByCaller); } break; + case kTypeCheckForImplicitClosureFunction: + if (needs_covariant_check_in_method) { + // Don't type check covariant parameters - they will be checked by + // a function we forward to. Their types however are not known. + variable->set_type_check_mode(LocalVariable::kSkipTypeCheck); + } else { + // All non-covariant checks are either checked by the type system or + // by a dynamic closure call dispatcher. + variable->set_type_check_mode(LocalVariable::kTypeCheckedByCaller); + } + break; case kTypeCheckForStaticFunction: variable->set_type_check_mode(LocalVariable::kTypeCheckedByCaller); break; diff --git a/runtime/vm/compiler/frontend/scope_builder.h b/runtime/vm/compiler/frontend/scope_builder.h index 33e24b3aa2a..c8680c34974 100644 --- a/runtime/vm/compiler/frontend/scope_builder.h +++ b/runtime/vm/compiler/frontend/scope_builder.h @@ -73,6 +73,10 @@ class ScopeBuilder { // No parameters will be checked. kTypeCheckForStaticFunction, + + // No non-covariant checks are performed, and any covariant checks are + // performed by the target. + kTypeCheckForImplicitClosureFunction, }; // This assumes that the reader is at a FunctionNode, @@ -230,15 +234,6 @@ class ScopeBuildingResult : public ZoneAllocated { DISALLOW_COPY_AND_ASSIGN(ScopeBuildingResult); }; -// Returns true if the given method can skip type checks for all type arguments -// that are not covariant or generic covariant in its implementation. -bool MethodCanSkipTypeChecksForNonCovariantTypeArguments( - const Function& method); - -// Returns true if the given method can skip type checks for all arguments -// that are not covariant or generic covariant in its implementation. -bool MethodCanSkipTypeChecksForNonCovariantArguments(const Function& method); - } // namespace kernel } // namespace dart diff --git a/runtime/vm/compiler/runtime_api.h b/runtime/vm/compiler/runtime_api.h index 90f656641af..25908f15612 100644 --- a/runtime/vm/compiler/runtime_api.h +++ b/runtime/vm/compiler/runtime_api.h @@ -893,6 +893,7 @@ class TypeParameter : public AllStatic { static word NextFieldOffset(); static word parameterized_class_id_offset(); static word index_offset(); + static word nullability_offset(); }; class LibraryPrefix : public AllStatic { diff --git a/runtime/vm/compiler/runtime_offsets_extracted.h b/runtime/vm/compiler/runtime_offsets_extracted.h index 9cd38b3b618..2f9397273f8 100644 --- a/runtime/vm/compiler/runtime_offsets_extracted.h +++ b/runtime/vm/compiler/runtime_offsets_extracted.h @@ -395,6 +395,8 @@ static constexpr dart::compiler::target::word Type_nullability_offset = 33; static constexpr dart::compiler::target::word TypeParameter_parameterized_class_id_offset = 32; static constexpr dart::compiler::target::word TypeParameter_index_offset = 40; +static constexpr dart::compiler::target::word TypeParameter_nullability_offset = + 43; static constexpr dart::compiler::target::word TypeArguments_instantiations_offset = 4; static constexpr dart::compiler::target::word TypeArguments_length_offset = 8; @@ -915,6 +917,8 @@ static constexpr dart::compiler::target::word Type_nullability_offset = 61; static constexpr dart::compiler::target::word TypeParameter_parameterized_class_id_offset = 64; static constexpr dart::compiler::target::word TypeParameter_index_offset = 72; +static constexpr dart::compiler::target::word TypeParameter_nullability_offset = + 75; static constexpr dart::compiler::target::word TypeArguments_instantiations_offset = 8; static constexpr dart::compiler::target::word TypeArguments_length_offset = 16; @@ -1431,6 +1435,8 @@ static constexpr dart::compiler::target::word Type_nullability_offset = 33; static constexpr dart::compiler::target::word TypeParameter_parameterized_class_id_offset = 32; static constexpr dart::compiler::target::word TypeParameter_index_offset = 40; +static constexpr dart::compiler::target::word TypeParameter_nullability_offset = + 43; static constexpr dart::compiler::target::word TypeArguments_instantiations_offset = 4; static constexpr dart::compiler::target::word TypeArguments_length_offset = 8; @@ -1948,6 +1954,8 @@ static constexpr dart::compiler::target::word Type_nullability_offset = 61; static constexpr dart::compiler::target::word TypeParameter_parameterized_class_id_offset = 64; static constexpr dart::compiler::target::word TypeParameter_index_offset = 72; +static constexpr dart::compiler::target::word TypeParameter_nullability_offset = + 75; static constexpr dart::compiler::target::word TypeArguments_instantiations_offset = 8; static constexpr dart::compiler::target::word TypeArguments_length_offset = 16; @@ -2464,6 +2472,8 @@ static constexpr dart::compiler::target::word Type_nullability_offset = 33; static constexpr dart::compiler::target::word TypeParameter_parameterized_class_id_offset = 32; static constexpr dart::compiler::target::word TypeParameter_index_offset = 40; +static constexpr dart::compiler::target::word TypeParameter_nullability_offset = + 43; static constexpr dart::compiler::target::word TypeArguments_instantiations_offset = 4; static constexpr dart::compiler::target::word TypeArguments_length_offset = 8; @@ -2978,6 +2988,8 @@ static constexpr dart::compiler::target::word Type_nullability_offset = 61; static constexpr dart::compiler::target::word TypeParameter_parameterized_class_id_offset = 64; static constexpr dart::compiler::target::word TypeParameter_index_offset = 72; +static constexpr dart::compiler::target::word TypeParameter_nullability_offset = + 75; static constexpr dart::compiler::target::word TypeArguments_instantiations_offset = 8; static constexpr dart::compiler::target::word TypeArguments_length_offset = 16; @@ -3488,6 +3500,8 @@ static constexpr dart::compiler::target::word Type_nullability_offset = 33; static constexpr dart::compiler::target::word TypeParameter_parameterized_class_id_offset = 32; static constexpr dart::compiler::target::word TypeParameter_index_offset = 40; +static constexpr dart::compiler::target::word TypeParameter_nullability_offset = + 43; static constexpr dart::compiler::target::word TypeArguments_instantiations_offset = 4; static constexpr dart::compiler::target::word TypeArguments_length_offset = 8; @@ -3999,6 +4013,8 @@ static constexpr dart::compiler::target::word Type_nullability_offset = 61; static constexpr dart::compiler::target::word TypeParameter_parameterized_class_id_offset = 64; static constexpr dart::compiler::target::word TypeParameter_index_offset = 72; +static constexpr dart::compiler::target::word TypeParameter_nullability_offset = + 75; static constexpr dart::compiler::target::word TypeArguments_instantiations_offset = 8; static constexpr dart::compiler::target::word TypeArguments_length_offset = 16; @@ -4548,6 +4564,8 @@ static constexpr dart::compiler::target::word AOT_TypeParameter_parameterized_class_id_offset = 32; static constexpr dart::compiler::target::word AOT_TypeParameter_index_offset = 40; +static constexpr dart::compiler::target::word + AOT_TypeParameter_nullability_offset = 43; static constexpr dart::compiler::target::word AOT_TypeArguments_instantiations_offset = 4; static constexpr dart::compiler::target::word AOT_TypeArguments_length_offset = @@ -5121,6 +5139,8 @@ static constexpr dart::compiler::target::word AOT_TypeParameter_parameterized_class_id_offset = 64; static constexpr dart::compiler::target::word AOT_TypeParameter_index_offset = 72; +static constexpr dart::compiler::target::word + AOT_TypeParameter_nullability_offset = 75; static constexpr dart::compiler::target::word AOT_TypeArguments_instantiations_offset = 8; static constexpr dart::compiler::target::word AOT_TypeArguments_length_offset = @@ -5698,6 +5718,8 @@ static constexpr dart::compiler::target::word AOT_TypeParameter_parameterized_class_id_offset = 64; static constexpr dart::compiler::target::word AOT_TypeParameter_index_offset = 72; +static constexpr dart::compiler::target::word + AOT_TypeParameter_nullability_offset = 75; static constexpr dart::compiler::target::word AOT_TypeArguments_instantiations_offset = 8; static constexpr dart::compiler::target::word AOT_TypeArguments_length_offset = @@ -6269,6 +6291,8 @@ static constexpr dart::compiler::target::word AOT_TypeParameter_parameterized_class_id_offset = 32; static constexpr dart::compiler::target::word AOT_TypeParameter_index_offset = 40; +static constexpr dart::compiler::target::word + AOT_TypeParameter_nullability_offset = 43; static constexpr dart::compiler::target::word AOT_TypeArguments_instantiations_offset = 4; static constexpr dart::compiler::target::word AOT_TypeArguments_length_offset = @@ -6835,6 +6859,8 @@ static constexpr dart::compiler::target::word AOT_TypeParameter_parameterized_class_id_offset = 64; static constexpr dart::compiler::target::word AOT_TypeParameter_index_offset = 72; +static constexpr dart::compiler::target::word + AOT_TypeParameter_nullability_offset = 75; static constexpr dart::compiler::target::word AOT_TypeArguments_instantiations_offset = 8; static constexpr dart::compiler::target::word AOT_TypeArguments_length_offset = @@ -7405,6 +7431,8 @@ static constexpr dart::compiler::target::word AOT_TypeParameter_parameterized_class_id_offset = 64; static constexpr dart::compiler::target::word AOT_TypeParameter_index_offset = 72; +static constexpr dart::compiler::target::word + AOT_TypeParameter_nullability_offset = 75; static constexpr dart::compiler::target::word AOT_TypeArguments_instantiations_offset = 8; static constexpr dart::compiler::target::word AOT_TypeArguments_length_offset = diff --git a/runtime/vm/compiler/runtime_offsets_list.h b/runtime/vm/compiler/runtime_offsets_list.h index 98ab25f6bed..8b5aa0ae6c5 100644 --- a/runtime/vm/compiler/runtime_offsets_list.h +++ b/runtime/vm/compiler/runtime_offsets_list.h @@ -267,6 +267,7 @@ FIELD(Type, nullability_offset) \ FIELD(TypeParameter, parameterized_class_id_offset) \ FIELD(TypeParameter, index_offset) \ + FIELD(TypeParameter, nullability_offset) \ FIELD(TypeArguments, instantiations_offset) \ FIELD(TypeArguments, length_offset) \ FIELD(TypeArguments, nullability_offset) \ diff --git a/runtime/vm/compiler/stub_code_compiler.cc b/runtime/vm/compiler/stub_code_compiler.cc index 7536df580fd..5456c292536 100644 --- a/runtime/vm/compiler/stub_code_compiler.cc +++ b/runtime/vm/compiler/stub_code_compiler.cc @@ -189,6 +189,251 @@ void StubCodeCompiler::GenerateInstanceOfStub(Assembler* assembler) { __ Ret(); } +// For use in GenerateTypeIsTopTypeForSubtyping and +// GenerateNullIsAssignableToType. +static void EnsureIsTypeOrTypeParameter(Assembler* assembler, + Register type_reg, + Register scratch_reg) { +#if defined(DEBUG) + compiler::Label is_type_param_or_type; + __ LoadClassIdMayBeSmi(scratch_reg, type_reg); + __ CompareImmediate(scratch_reg, kTypeParameterCid); + __ BranchIf(EQUAL, &is_type_param_or_type, compiler::Assembler::kNearJump); + __ CompareImmediate(scratch_reg, kTypeCid); + __ BranchIf(EQUAL, &is_type_param_or_type, compiler::Assembler::kNearJump); + // Type references show up in F-bounded polymorphism, which is limited + // to classes. Thus, TypeRefs only appear in places like class type + // arguments or the bounds of uninstantiated class type parameters. + // + // Since this stub is currently used only by the dynamic versions of + // AssertSubtype and AssertAssignable, where kDstType is either the bound of + // a function type parameter or the type of a function parameter + // (respectively), we should never see a TypeRef here. This check is here + // in case this changes and we need to update this stub. + __ Stop("not a type or type parameter"); + __ Bind(&is_type_param_or_type); +#endif +} + +// Version of AbstractType::IsTopTypeForSubtyping() used when the type is not +// known at compile time. Must be kept in sync. +// +// Inputs: +// - TypeTestABI::kDstTypeReg: Destination type. +// +// Non-preserved scratch registers: +// - TypeTestABI::kScratchReg (only on non-IA32 architectures) +// +// Outputs: +// - TypeTestABI::kSubtypeTestCacheReg: 0 if the value is guaranteed assignable, +// non-zero otherwise. +// +// All registers other than outputs and non-preserved scratches are preserved. +static void GenerateTypeIsTopTypeForSubtyping(Assembler* assembler, + bool null_safety) { + // The only case where the original value of kSubtypeTestCacheReg is needed + // after the stub call is on IA32, where it's currently preserved on the stack + // before calling the stub (as it's also CODE_REG on that architecture), so we + // both use it as a scratch and clobber it for the return value. + const Register scratch1_reg = TypeTestABI::kSubtypeTestCacheReg; + // We reuse the first scratch register as the output register because we're + // always guaranteed to have a type in it (starting with kDstType), and all + // non-Smi ObjectPtrs are non-zero values. + const Register output_reg = scratch1_reg; +#if defined(TARGET_ARCH_IA32) + // The remaining scratch registers are preserved and restored before exit on + // IA32. Because we have few registers to choose from (which are all used in + // TypeTestABI), use specific TestTypeABI registers. + const Register scratch2_reg = TypeTestABI::kFunctionTypeArgumentsReg; + // Preserve non-output scratch registers. + __ PushRegister(scratch2_reg); +#else + const Register scratch2_reg = TypeTestABI::kScratchReg; +#endif + static_assert(scratch1_reg != scratch2_reg, + "both scratch registers are the same"); + + compiler::Label check_top_type, is_top_type, done; + // Initialize scratch1_reg with the type to check (which also sets the + // output register to a non-zero value). scratch1_reg (and thus the output + // register) will always have a type in it from here on out. + __ MoveRegister(scratch1_reg, TypeTestABI::kDstTypeReg); + __ Bind(&check_top_type); + // scratch1_reg: Current type to check. + EnsureIsTypeOrTypeParameter(assembler, scratch1_reg, scratch2_reg); + compiler::Label is_type_ref; + __ CompareClassId(scratch1_reg, kTypeParameterCid, scratch2_reg); + // Type parameters can't be top types themselves, though a particular + // instantiation may result in a top type. + __ BranchIf(EQUAL, &done); + __ LoadField( + scratch2_reg, + compiler::FieldAddress(scratch1_reg, + compiler::target::Type::type_class_id_offset())); + __ SmiUntag(scratch2_reg); + __ CompareImmediate(scratch2_reg, kDynamicCid); + __ BranchIf(EQUAL, &is_top_type, compiler::Assembler::kNearJump); + __ CompareImmediate(scratch2_reg, kVoidCid); + __ BranchIf(EQUAL, &is_top_type, compiler::Assembler::kNearJump); + compiler::Label unwrap_future_or; + __ CompareImmediate(scratch2_reg, kFutureOrCid); + __ BranchIf(EQUAL, &unwrap_future_or, compiler::Assembler::kNearJump); + __ CompareImmediate(scratch2_reg, kInstanceCid); + __ BranchIf(NOT_EQUAL, &done, compiler::Assembler::kNearJump); + if (null_safety) { + // Instance type isn't a top type if non-nullable in null safe mode. + __ CompareTypeNullabilityWith( + scratch1_reg, static_cast(Nullability::kNonNullable)); + __ BranchIf(EQUAL, &done, compiler::Assembler::kNearJump); + } + __ Bind(&is_top_type); + __ LoadImmediate(output_reg, 0); + __ Bind(&done); +#if defined(TARGET_ARCH_IA32) + // Restore preserved scratch registers. + __ PopRegister(scratch2_reg); +#endif + __ Ret(); + // An uncommon case, so off the main trunk of the function. + __ Bind(&unwrap_future_or); + __ LoadField(scratch2_reg, + compiler::FieldAddress( + scratch1_reg, compiler::target::Type::arguments_offset())); + __ CompareObject(scratch2_reg, Object::null_object()); + // If the arguments are null, then unwrapping gives dynamic, a top type. + __ BranchIf(EQUAL, &is_top_type, compiler::Assembler::kNearJump); + __ LoadField( + scratch1_reg, + compiler::FieldAddress( + scratch2_reg, compiler::target::TypeArguments::type_at_offset(0))); + __ Jump(&check_top_type, compiler::Assembler::kNearJump); +} + +void StubCodeCompiler::GenerateTypeIsTopTypeForSubtypingStub( + Assembler* assembler) { + GenerateTypeIsTopTypeForSubtyping(assembler, + /*null_safety=*/false); +} + +void StubCodeCompiler::GenerateTypeIsTopTypeForSubtypingNullSafeStub( + Assembler* assembler) { + GenerateTypeIsTopTypeForSubtyping(assembler, + /*null_safety=*/true); +} + +// Version of Instance::NullIsAssignableTo() used when the destination type is +// not known at compile time. Must be kept in sync. +// +// Inputs: +// - TypeTestABI::kInstanceReg: Object to check for assignability. +// - TypeTestABI::kDstTypeReg: Destination type. +// +// Non-preserved non-output scratch registers: +// - TypeTestABI::kScratchReg (only on non-IA32 architectures) +// +// Outputs: +// - TypeTestABI::kSubtypeTestCacheReg: 0 if the value is guaranteed assignable, +// non-zero otherwise. +// +// All registers other than outputs and non-preserved scratches are preserved. +static void GenerateNullIsAssignableToType(Assembler* assembler, + bool null_safety) { + // The only case where the original value of kSubtypeTestCacheReg is needed + // after the stub call is on IA32, where it's currently preserved on the stack + // before calling the stub (as it's also CODE_REG on that architecture), so we + // both use it as a scratch and clobber it for the return value. + const Register scratch1_reg = TypeTestABI::kSubtypeTestCacheReg; + // We reuse the first scratch register as the output register because we're + // always guaranteed to have a type in it (starting with kDstType), and all + // non-Smi ObjectPtrs are non-zero values. + const Register output_reg = scratch1_reg; +#if defined(TARGET_ARCH_IA32) + // The remaining scratch registers are preserved and restored before exit on + // IA32. Because we have few registers to choose from (which are all used in + // TypeTestABI), use specific TestTypeABI registers. + const Register scratch2_reg = TypeTestABI::kFunctionTypeArgumentsReg; + // Preserve non-output scratch registers. + __ PushRegister(scratch2_reg); +#else + const Register scratch2_reg = TypeTestABI::kScratchReg; +#endif + static_assert(scratch1_reg != scratch2_reg, + "code assumes distinct scratch registers"); + + compiler::Label is_assignable, done; + // Initialize the first scratch register (and thus the output register) with + // the destination type. We do this before the check to ensure the output + // register has a non-zero value if !null_safety and kInstanceReg is not null. + __ MoveRegister(scratch1_reg, TypeTestABI::kDstTypeReg); + __ CompareObject(TypeTestABI::kInstanceReg, Object::null_object()); + if (null_safety) { + compiler::Label check_null_assignable; + // Skip checking the type if not null. + __ BranchIf(NOT_EQUAL, &done, compiler::Assembler::kNearJump); + __ Bind(&check_null_assignable); + // scratch1_reg: Current type to check. + EnsureIsTypeOrTypeParameter(assembler, scratch1_reg, scratch2_reg); + compiler::Label is_not_type; + __ CompareClassId(scratch1_reg, kTypeCid, scratch2_reg); + __ BranchIf(NOT_EQUAL, &is_not_type, compiler::Assembler::kNearJump); + __ CompareTypeNullabilityWith( + scratch1_reg, static_cast(Nullability::kNonNullable)); + __ BranchIf(NOT_EQUAL, &is_assignable, compiler::Assembler::kNearJump); + // FutureOr is a special case because it may have the non-nullable bit set, + // but FutureOr functions as the union of T and Future, so it must be + // unwrapped to see if T is nullable. + __ LoadField( + scratch2_reg, + compiler::FieldAddress(scratch1_reg, + compiler::target::Type::type_class_id_offset())); + __ SmiUntag(scratch2_reg); + __ CompareImmediate(scratch2_reg, kFutureOrCid); + __ BranchIf(NOT_EQUAL, &done, compiler::Assembler::kNearJump); + __ LoadField(scratch2_reg, + compiler::FieldAddress( + scratch1_reg, compiler::target::Type::arguments_offset())); + __ CompareObject(scratch2_reg, Object::null_object()); + // If the arguments are null, then unwrapping gives the dynamic type, + // which can take null. + __ BranchIf(EQUAL, &is_assignable, compiler::Assembler::kNearJump); + __ LoadField( + scratch1_reg, + compiler::FieldAddress( + scratch2_reg, compiler::target::TypeArguments::type_at_offset(0))); + __ Jump(&check_null_assignable, compiler::Assembler::kNearJump); + __ Bind(&is_not_type); + // Null is assignable to a type parameter only if it is nullable. + __ LoadFieldFromOffset( + scratch2_reg, scratch1_reg, + compiler::target::TypeParameter::nullability_offset(), kByte); + __ CompareImmediate(scratch2_reg, + static_cast(Nullability::kNonNullable)); + __ BranchIf(EQUAL, &done, compiler::Assembler::kNearJump); + } else { + // Null in non-null-safe mode is always assignable. + __ BranchIf(NOT_EQUAL, &done, compiler::Assembler::kNearJump); + } + __ Bind(&is_assignable); + __ LoadImmediate(output_reg, 0); + __ Bind(&done); +#if defined(TARGET_ARCH_IA32) + // Restore preserved scratch registers. + __ PopRegister(scratch2_reg); +#endif + __ Ret(); +} + +void StubCodeCompiler::GenerateNullIsAssignableToTypeStub( + Assembler* assembler) { + GenerateNullIsAssignableToType(assembler, + /*null_safety=*/false); +} + +void StubCodeCompiler::GenerateNullIsAssignableToTypeNullSafeStub( + Assembler* assembler) { + GenerateNullIsAssignableToType(assembler, + /*null_safety=*/true); +} #if !defined(TARGET_ARCH_IA32) // The TypeTestStubs are used to test whether a given value is of a given // type. All variants have the same calling convention: diff --git a/runtime/vm/dart_entry.cc b/runtime/vm/dart_entry.cc index 63db37e039e..d98eaa1ba07 100644 --- a/runtime/vm/dart_entry.cc +++ b/runtime/vm/dart_entry.cc @@ -438,7 +438,7 @@ void ArgumentsDescriptor::PrintTo(BaseTextBuffer* buffer) const { buffer->AddString(", "); } str = NameAt(i); - buffer->Printf("'%s'", str.ToCString()); + buffer->Printf("'%s' (%" Pd ")", str.ToCString(), PositionAt(i)); } buffer->Printf("]"); } diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index eb9c6479687..c1a8c93ae6a 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -3513,8 +3513,7 @@ FunctionPtr Class::CreateInvocationDispatcher(const String& target_name, invocation.SetParameterTypeAt(0, Object::dynamic_type()); invocation.SetParameterNameAt(0, Symbols::This()); // Remaining positional parameters. - intptr_t i = 1; - for (; i < desc.PositionalCount(); i++) { + for (intptr_t i = 1; i < desc.PositionalCount(); i++) { invocation.SetParameterTypeAt(i, Object::dynamic_type()); char name[64]; Utils::SNPrint(name, 64, ":p%" Pd, i); @@ -3523,10 +3522,11 @@ FunctionPtr Class::CreateInvocationDispatcher(const String& target_name, } // Named parameters. - for (; i < desc.Count(); i++) { - invocation.SetParameterTypeAt(i, Object::dynamic_type()); - intptr_t index = i - desc.PositionalCount(); - invocation.SetParameterNameAt(i, String::Handle(zone, desc.NameAt(index))); + for (intptr_t i = 0; i < desc.NamedCount(); i++) { + const intptr_t param_index = desc.PositionAt(i); + const auto& param_name = String::Handle(zone, desc.NameAt(i)); + invocation.SetParameterTypeAt(param_index, Object::dynamic_type()); + invocation.SetParameterNameAt(param_index, param_name); } invocation.TruncateUnusedParameterFlags(); invocation.set_result_type(Object::dynamic_type()); @@ -7007,13 +7007,13 @@ void Function::set_default_type_arguments(const TypeArguments& value) const { const auto& closure_data = ClosureData::Handle(ClosureData::RawCast(raw_ptr()->data_)); ASSERT(!closure_data.IsNull()); + intptr_t updated_info = closure_data.default_type_arguments_info(); auto kind = DefaultTypeArgumentsKindFor(value); ASSERT(kind != DefaultTypeArgumentsKind::kInvalid); - const intptr_t num_parent_type_params = NumParentTypeParameters(); - const intptr_t default_type_args_info = - DefaultTypeArgumentsKindField::encode(kind) | - NumParentTypeParametersField::encode(num_parent_type_params); - closure_data.set_default_type_arguments_info(default_type_args_info); + updated_info = DefaultTypeArgumentsKindField::update(kind, updated_info); + updated_info = NumParentTypeParametersField::update(NumParentTypeParameters(), + updated_info); + closure_data.set_default_type_arguments_info(updated_info); // We could just store null for the ksharesFunction/kSharesInstantiator cases, // assuming all clients retrieve the DefaultTypeArgumentsKind to distinguish. closure_data.set_default_type_arguments(value); @@ -9644,15 +9644,20 @@ bool Function::HasDynamicCallers(Zone* zone) const { } bool Function::PrologueNeedsArgumentsDescriptor() const { + // These functions have a saved compile-time arguments descriptor that is + // used in lieu of the runtime arguments descriptor in generated IL. + if (IsInvokeFieldDispatcher() || IsNoSuchMethodDispatcher()) { + return false; + } // The prologue of those functions need to examine the arg descriptor for // various purposes. - return IsGeneric() || HasOptionalParameters(); + return IsGeneric() || HasOptionalParameters() || + CanReceiveDynamicInvocation(); } bool Function::MayHaveUncheckedEntryPoint() const { return FLAG_enable_multiple_entrypoints && - (NeedsTypeArgumentTypeChecks() || NeedsArgumentTypeChecks() || - IsImplicitClosureFunction()); + (NeedsTypeArgumentTypeChecks() || NeedsArgumentTypeChecks()); } const char* Function::ToCString() const { @@ -9751,6 +9756,13 @@ void ClosureData::set_default_type_arguments(const TypeArguments& value) const { } intptr_t ClosureData::default_type_arguments_info() const { + const SmiPtr value = raw_ptr()->default_type_arguments_info_; + if (value == Smi::null()) { + static_assert(Function::DefaultTypeArgumentsKindField::decode(0) == + Function::DefaultTypeArgumentsKind::kInvalid, + "Returning valid value for null Smi"); + return 0; + } return Smi::Value(raw_ptr()->default_type_arguments_info_); } @@ -18297,6 +18309,8 @@ bool Instance::NullIsInstanceOf( return other.IsLegacy() && (other.IsObjectType() || other.IsNeverType()); } +// Must be kept in sync with GenerateNullIsAssignableToType in +// stub_code_compiler.cc if any changes are made. bool Instance::NullIsAssignableTo(const AbstractType& other) { Thread* thread = Thread::Current(); Isolate* isolate = thread->isolate(); @@ -19115,6 +19129,8 @@ bool AbstractType::IsTopTypeForInstanceOf() const { return false; } +// Must be kept in sync with GenerateTypeIsTopTypeForSubtyping in +// stub_code_compiler.cc if any changes are made. bool AbstractType::IsTopTypeForSubtyping() const { const classid_t cid = type_class_id(); if (cid == kDynamicCid || cid == kVoidCid) { diff --git a/runtime/vm/object.h b/runtime/vm/object.h index eb290ea5c47..6538625ebd6 100644 --- a/runtime/vm/object.h +++ b/runtime/vm/object.h @@ -2694,23 +2694,20 @@ class Function : public Object { (1 << kDefaultTypeArgumentsKindFieldSize), "Wrong bit size chosen for default TAV kind field"); - // Fields encoded in an integer stored alongside a default TAV. + // Fields encoded in an integer stored alongside a default TAV. The size of + // the integer should be <= the size of a target Smi. using DefaultTypeArgumentsKindField = BitField; - // If more space is needed, we can almost certainly reduce the size of this - // field. + // Just use the rest of the space for the number of parent type parameters. using NumParentTypeParametersField = BitField; - - static_assert(NumParentTypeParametersField::kNextBit <= - compiler::target::kSmiBits, - "Default TAV info does not fit in a target Smi"); + compiler::target::kSmiBits - + DefaultTypeArgumentsKindField::kNextBit>; // Returns a canonicalized vector of the type parameters instantiated // to bounds. If non-generic, the empty type arguments vector is returned. @@ -2839,10 +2836,6 @@ class Function : public Object { return (kind() == FunctionLayout::kConstructor) && is_static(); } - static bool ClosureBodiesContainNonCovariantArgumentChecks() { - return FLAG_precompiled_mode || FLAG_lazy_dispatchers; - } - // Whether this function can receive an invocation where the number and names // of arguments have not been checked. bool CanReceiveDynamicInvocation() const { return IsFfiTrampoline(); } @@ -2910,13 +2903,11 @@ class Function : public Object { } bool IsInFactoryScope() const; - bool NeedsArgumentTypeChecks() const { - return (IsClosureFunction() && - ClosureBodiesContainNonCovariantArgumentChecks()) || - !(is_static() || (kind() == FunctionLayout::kConstructor)); + bool NeedsTypeArgumentTypeChecks() const { + return !(is_static() || (kind() == FunctionLayout::kConstructor)); } - bool NeedsTypeArgumentTypeChecks() const { + bool NeedsArgumentTypeChecks() const { return !(is_static() || (kind() == FunctionLayout::kConstructor)); } @@ -6889,12 +6880,10 @@ class SubtypeTestCache : public Object { ArrayPtr cache() const; private: - // A VM heap allocated preinitialized empty subtype entry array. - static ArrayPtr cached_array_; - void set_cache(const Array& value) const; - intptr_t TestEntryLength() const; + // A VM heap allocated preinitialized empty subtype entry array. + static ArrayPtr cached_array_; FINAL_HEAP_OBJECT_IMPLEMENTATION(SubtypeTestCache, Object); friend class Class; @@ -8163,6 +8152,9 @@ class TypeParameter : public AbstractType { return TypeParameterLayout::DeclarationBit::decode(raw_ptr()->flags_); } void SetDeclaration(bool value) const; + static intptr_t nullability_offset() { + return OFFSET_OF(TypeParameterLayout, nullability_); + } virtual Nullability nullability() const { return static_cast(raw_ptr()->nullability_); } diff --git a/runtime/vm/parser.cc b/runtime/vm/parser.cc index f4bf1341469..c9cf9234c90 100644 --- a/runtime/vm/parser.cc +++ b/runtime/vm/parser.cc @@ -74,20 +74,17 @@ ParsedFunction::ParsedFunction(Thread* thread, const Function& function) Symbols::CurrentContextVar(), Object::dynamic_type()); current_context_var_ = temp; - const bool reify_generic_argument = function.IsGeneric(); - - const bool load_optional_arguments = function.HasOptionalParameters(); - - const bool check_arguments = function.CanReceiveDynamicInvocation(); - - const bool need_argument_descriptor = - load_optional_arguments || check_arguments || reify_generic_argument; - - if (need_argument_descriptor) { + if (function.PrologueNeedsArgumentsDescriptor()) { arg_desc_var_ = new (zone()) LocalVariable(TokenPosition::kNoSource, TokenPosition::kNoSource, Symbols::ArgDescVar(), Object::dynamic_type()); } + + // The code generated by the prologue builder for loading optional arguments + // requires the expression temporary variable. + if (function.HasOptionalParameters()) { + EnsureExpressionTemp(); + } } void ParsedFunction::AddToGuardedFields(const Field* field) const { @@ -339,19 +336,33 @@ ParsedFunction::DynamicClosureCallVars* ParsedFunction::EnsureDynamicClosureCallVars() { ASSERT(function().IsDynamicClosureCallDispatcher(thread())); if (dynamic_closure_call_vars_ != nullptr) return dynamic_closure_call_vars_; - dynamic_closure_call_vars_ = new (zone()) DynamicClosureCallVars(); + const auto& saved_args_desc = + Array::Handle(zone(), function().saved_args_desc()); + const ArgumentsDescriptor descriptor(saved_args_desc); + dynamic_closure_call_vars_ = + new (zone()) DynamicClosureCallVars(zone(), descriptor.NamedCount()); + + auto const pos = function().token_pos(); const auto& type_Dynamic = Object::dynamic_type(); const auto& type_Function = Type::ZoneHandle(zone(), Type::DartFunctionType()); const auto& type_Smi = Type::ZoneHandle(zone(), Type::SmiType()); #define INIT_FIELD(Name, TypeName, Symbol) \ - dynamic_closure_call_vars_->Name = new (zone()) \ - LocalVariable(function().token_pos(), function().token_pos(), \ - Symbols::DynamicCall##Symbol##Var(), type_##TypeName); + dynamic_closure_call_vars_->Name = new (zone()) LocalVariable( \ + pos, pos, Symbols::DynamicCall##Symbol##Var(), type_##TypeName); FOR_EACH_DYNAMIC_CLOSURE_CALL_VARIABLE(INIT_FIELD); #undef INIT_FIELD + for (intptr_t i = 0; i < descriptor.NamedCount(); i++) { + auto const name = OS::SCreate( + zone(), ":dyn_call_named_argument_%" Pd "_parameter_index", i); + auto const var = new (zone()) LocalVariable( + pos, pos, String::ZoneHandle(zone(), Symbols::New(thread(), name)), + type_Smi); + dynamic_closure_call_vars_->named_argument_parameter_indices.Add(var); + } + return dynamic_closure_call_vars_; } diff --git a/runtime/vm/parser.h b/runtime/vm/parser.h index 76b6249ff9b..2aa73d810e4 100644 --- a/runtime/vm/parser.h +++ b/runtime/vm/parser.h @@ -237,6 +237,9 @@ class ParsedFunction : public ZoneAllocated { // Variables needed for the InvokeFieldDispatcher for dynamic closure calls, // because they are both read and written to by the builders. struct DynamicClosureCallVars : ZoneAllocated { + DynamicClosureCallVars(Zone* zone, intptr_t num_named) + : named_argument_parameter_indices(zone, num_named) {} + #define FOR_EACH_DYNAMIC_CLOSURE_CALL_VARIABLE(V) \ V(current_function, Function, CurrentFunction) \ V(current_num_processed, Smi, CurrentNumProcessed) \ @@ -246,6 +249,10 @@ class ParsedFunction : public ZoneAllocated { #define DEFINE_FIELD(Name, _, __) LocalVariable* Name = nullptr; FOR_EACH_DYNAMIC_CLOSURE_CALL_VARIABLE(DEFINE_FIELD) #undef DEFINE_FIELD + + // An array of local variables, one for each named parameter in the + // saved arguments descriptor. + ZoneGrowableArray named_argument_parameter_indices; }; DynamicClosureCallVars* dynamic_closure_call_vars() const { diff --git a/runtime/vm/runtime_entry.cc b/runtime/vm/runtime_entry.cc index 93c3f4f373e..db0d99fa511 100644 --- a/runtime/vm/runtime_entry.cc +++ b/runtime/vm/runtime_entry.cc @@ -806,10 +806,10 @@ DEFINE_RUNTIME_ENTRY(TypeCheck, 7) { ASSERT(mode == kTypeCheckFromInline); #endif - ASSERT(!dst_type.IsDynamicType()); // No need to check assignment. - // A null instance is already detected and allowed in inlined code, unless - // strong checking is enabled. + // These are guaranteed on the calling side. + ASSERT(!dst_type.IsDynamicType()); ASSERT(!src_instance.IsNull() || isolate->null_safety()); + const bool is_instance_of = src_instance.IsAssignableTo( dst_type, instantiator_type_arguments, function_type_arguments); @@ -819,16 +819,6 @@ DEFINE_RUNTIME_ENTRY(TypeCheck, 7) { Bool::Get(is_instance_of)); } if (!is_instance_of) { - // Throw a dynamic type error. - const TokenPosition location = GetCallerLocation(); - const AbstractType& src_type = - AbstractType::Handle(zone, src_instance.GetType(Heap::kNew)); - if (!dst_type.IsInstantiated()) { - // Instantiate dst_type before reporting the error. - dst_type = dst_type.InstantiateFrom(instantiator_type_arguments, - function_type_arguments, kAllFree, - Heap::kNew); - } if (dst_name.IsNull()) { #if !defined(TARGET_ARCH_IA32) // Can only come here from type testing stub. @@ -852,6 +842,56 @@ DEFINE_RUNTIME_ENTRY(TypeCheck, 7) { #endif } + if (dst_name.raw() == + Symbols::dynamic_assert_assignable_stc_check().raw()) { +#if !defined(TARGET_ARCH_IA32) + // Can only come here from type testing stub via dynamic AssertAssignable. + ASSERT(mode != kTypeCheckFromInline); +#endif + // This was a dynamic closure call where the destination name was not + // known at compile-time. Thus, fetch the original arguments and arguments + // descriptor and re-do the type check in the runtime, which causes the + // error with the proper destination name to be thrown. + DartFrameIterator iterator(thread, + StackFrameIterator::kNoCrossThreadIteration); + StackFrame* caller_frame = iterator.NextFrame(); + const auto& dispatcher = + Function::Handle(zone, caller_frame->LookupDartFunction()); + ASSERT(dispatcher.IsInvokeFieldDispatcher()); + const auto& orig_arguments_desc = + Array::Handle(zone, dispatcher.saved_args_desc()); + const ArgumentsDescriptor args_desc(orig_arguments_desc); + const intptr_t arg_count = args_desc.CountWithTypeArgs(); + const auto& orig_arguments = Array::Handle(zone, Array::New(arg_count)); + auto& obj = Object::Handle(zone); + for (intptr_t i = 0; i < arg_count; i++) { + obj = *reinterpret_cast( + ParamAddress(caller_frame->fp(), arg_count - i)); + orig_arguments.SetAt(i, obj); + } + const auto& receiver = Closure::CheckedHandle( + zone, orig_arguments.At(args_desc.FirstArgIndex())); + const auto& function = Function::Handle(zone, receiver.function()); + const auto& result = Object::Handle( + zone, function.DoArgumentTypesMatch(orig_arguments, args_desc)); + if (result.IsError()) { + Exceptions::PropagateError(Error::Cast(result)); + } + // IsAssignableTo returned false, so we should have thrown a type + // error in DoArgumentsTypesMatch. + UNREACHABLE(); + } + + // Throw a dynamic type error. + const TokenPosition location = GetCallerLocation(); + const AbstractType& src_type = + AbstractType::Handle(zone, src_instance.GetType(Heap::kNew)); + if (!dst_type.IsInstantiated()) { + // Instantiate dst_type before reporting the error. + dst_type = dst_type.InstantiateFrom(instantiator_type_arguments, + function_type_arguments, kAllFree, + Heap::kNew); + } Exceptions::CreateAndThrowTypeError(location, src_type, dst_type, dst_name); UNREACHABLE(); } diff --git a/runtime/vm/stub_code.cc b/runtime/vm/stub_code.cc index 7f8f3a36ec7..3dea626df3c 100644 --- a/runtime/vm/stub_code.cc +++ b/runtime/vm/stub_code.cc @@ -277,6 +277,16 @@ CodePtr StubCode::GetAllocationStubForTypedData(classid_t class_id) { return Code::null(); } +const Code& StubCode::GetTypeIsTopTypeForSubtyping(bool null_safety) { + return null_safety ? StubCode::TypeIsTopTypeForSubtypingNullSafe() + : StubCode::TypeIsTopTypeForSubtyping(); +} + +const Code& StubCode::GetNullIsAssignableToType(bool null_safety) { + return null_safety ? StubCode::NullIsAssignableToTypeNullSafe() + : StubCode::NullIsAssignableToType(); +} + #if !defined(TARGET_ARCH_IA32) CodePtr StubCode::GetBuildMethodExtractorStub( compiler::ObjectPoolBuilder* pool) { diff --git a/runtime/vm/stub_code.h b/runtime/vm/stub_code.h index a946cd5881c..d54ee02c836 100644 --- a/runtime/vm/stub_code.h +++ b/runtime/vm/stub_code.h @@ -68,6 +68,10 @@ class StubCode : public AllStatic { static CodePtr GetBuildMethodExtractorStub(compiler::ObjectPoolBuilder* pool); #endif + static const Code& GetTypeIsTopTypeForSubtyping(bool null_safety); + + static const Code& GetNullIsAssignableToType(bool null_safety); + #if !defined(DART_PRECOMPILED_RUNTIME) // Generate the stub and finalize the generated code into the stub // code executable area. diff --git a/runtime/vm/stub_code_list.h b/runtime/vm/stub_code_list.h index 3d9a9b9cad7..c08bfd6052b 100644 --- a/runtime/vm/stub_code_list.h +++ b/runtime/vm/stub_code_list.h @@ -86,6 +86,10 @@ namespace dart { V(OneArgUnoptimizedStaticCall) \ V(TwoArgsUnoptimizedStaticCall) \ V(AssertSubtype) \ + V(TypeIsTopTypeForSubtyping) \ + V(TypeIsTopTypeForSubtypingNullSafe) \ + V(NullIsAssignableToType) \ + V(NullIsAssignableToTypeNullSafe) \ V(Subtype1TestCache) \ V(Subtype3TestCache) \ V(Subtype5TestCache) \ diff --git a/runtime/vm/symbols.h b/runtime/vm/symbols.h index a1db78b0fd6..bbee27e0a0d 100644 --- a/runtime/vm/symbols.h +++ b/runtime/vm/symbols.h @@ -463,6 +463,9 @@ class ObjectPointerVisitor; V(controller, "controller") \ V(current_character, ":current_character") \ V(current_position, ":current_position") \ + V(dynamic_assert_assignable_stc_check, \ + ":dynamic_assert_assignable_stc_check") \ + V(getID, "getID") \ V(hashCode, "get:hashCode") \ V(identityHashCode, "identityHashCode") \ V(index_temp, ":index_temp") \ diff --git a/tests/language/closure/dynamic_test.dart b/tests/language/closure/dynamic_test.dart new file mode 100644 index 00000000000..eb954ec4cba --- /dev/null +++ b/tests/language/closure/dynamic_test.dart @@ -0,0 +1,63 @@ +// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file +// 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. +// +// Test that dynamic invocation of closures works as expected, including +// appropriate type checks. +// +// VMOptions=--lazy-dispatchers +// VMOptions=--no-lazy-dispatchers + +import 'package:expect/expect.dart'; + +class A { + final int nonce_; + + const A(this.nonce_); +} + +class B { + final int nonce_; + + const B(this.nonce_); +} + +class C extends A { + const C(int nonce) : super(nonce); +} + +void main() { + dynamic f = (String a1, int a2, A a3, + {String n1 = "default_named", int n2 = -1, A n3 = const A(-1)}) {}; + + f("test_fixed", 1, A(1), n1: "test_named", n2: 2, n3: A(2)); + + // Test named argument permutations + f("test_fixed", 1, A(1), n1: "test_named", n3: A(2), n2: 2); + f("test_fixed", 1, A(1), n2: 2, n1: "test_named", n3: A(2)); + f("test_fixed", 1, A(1), n2: 2, n3: A(2), n1: "test_named"); + f("test_fixed", 1, A(1), n3: A(2), n1: "test_named", n2: 2); + f("test_fixed", 1, A(1), n3: A(2), n2: 2, n1: "test_named"); + + // Test subclasses match the type + f("test_fixed", 1, C(1), n1: "test_named", n2: 2, n3: A(2)); + f("test_fixed", 1, A(1), n1: "test_named", n2: 2, n3: C(2)); + + // Should fail with no such method errors + Expect.throwsNoSuchMethodError(() => f()); + Expect.throwsNoSuchMethodError(() => f("test_fixed", 1, A(1), n4: 4)); + + // Should fail with type errors + Expect.throwsTypeError( + () => f(100, 1, A(1), n1: "test_named", n2: 2, n3: A(2))); + Expect.throwsTypeError( + () => f("test_fixed", 1.1, A(1), n1: "test_named", n2: 2, n3: A(2))); + Expect.throwsTypeError( + () => f("test_fixed", 1, B(1), n1: "test_named", n2: 2, n3: A(2))); + Expect.throwsTypeError( + () => f("test_fixed", 1, A(1), n1: 100, n2: 2, n3: A(2))); + Expect.throwsTypeError( + () => f("test_fixed", 1, A(1), n1: "test_named", n2: 2.2, n3: A(2))); + Expect.throwsTypeError( + () => f("test_fixed", 1, A(1), n1: "test_named", n2: 2, n3: B(2))); +} diff --git a/tests/language/stack_trace/stack_trace_test.dart b/tests/language/stack_trace/stack_trace_test.dart index 5e4d7c65fa8..b1e60a95d13 100644 --- a/tests/language/stack_trace/stack_trace_test.dart +++ b/tests/language/stack_trace/stack_trace_test.dart @@ -88,6 +88,7 @@ class RethrowStacktraceTest { var config = 0; @pragma("vm:entry-point") // Prevent obfuscation + @pragma("vm:never-inline") // Prevent inlining issue12940() { throw "Progy"; } @@ -121,7 +122,7 @@ class RethrowStacktraceTest { try { d(); } catch (e, s) { - Expect.isTrue(s.toString().contains("issue12940")); + Expect.contains("issue12940", s.toString()); } } } diff --git a/tests/language_2/closure/dynamic_test.dart b/tests/language_2/closure/dynamic_test.dart new file mode 100644 index 00000000000..b130a9b76e1 --- /dev/null +++ b/tests/language_2/closure/dynamic_test.dart @@ -0,0 +1,62 @@ +// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file +// 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. +// +// Test that dynamic invocation of closures works as expected, including +// appropriate type checks. +// +// VMOptions=--lazy-dispatchers +// VMOptions=--no-lazy-dispatchers + +import 'package:expect/expect.dart'; + +class A { + final int nonce_; + + A(this.nonce_); +} + +class B { + final int nonce_; + + B(this.nonce_); +} + +class C extends A { + C(int nonce) : super(nonce); +} + +void main() { + dynamic f = (String a1, int a2, A a3, {String n1, int n2, A n3}) {}; + + f("test_fixed", 1, A(1), n1: "test_named", n2: 2, n3: A(2)); + +// Test named argument permutations + f("test_fixed", 1, A(1), n1: "test_named", n3: A(2), n2: 2); + f("test_fixed", 1, A(1), n2: 2, n1: "test_named", n3: A(2)); + f("test_fixed", 1, A(1), n2: 2, n3: A(2), n1: "test_named"); + f("test_fixed", 1, A(1), n3: A(2), n1: "test_named", n2: 2); + f("test_fixed", 1, A(1), n3: A(2), n2: 2, n1: "test_named"); + +// Test subclasses match the type + f("test_fixed", 1, C(1), n1: "test_named", n2: 2, n3: A(2)); + f("test_fixed", 1, A(1), n1: "test_named", n2: 2, n3: C(2)); + +// Should fail with no such method errors + Expect.throwsNoSuchMethodError(() => f()); + Expect.throwsNoSuchMethodError(() => f("test_fixed", 1, A(1), n4: 4)); + +// Should fail with type errors + Expect.throwsTypeError( + () => f(100, 1, A(1), n1: "test_named", n2: 2, n3: A(2))); + Expect.throwsTypeError( + () => f("test_fixed", 1.1, A(1), n1: "test_named", n2: 2, n3: A(2))); + Expect.throwsTypeError( + () => f("test_fixed", 1, B(1), n1: "test_named", n2: 2, n3: A(2))); + Expect.throwsTypeError( + () => f("test_fixed", 1, A(1), n1: 100, n2: 2, n3: A(2))); + Expect.throwsTypeError( + () => f("test_fixed", 1, A(1), n1: "test_named", n2: 2.2, n3: A(2))); + Expect.throwsTypeError( + () => f("test_fixed", 1, A(1), n1: "test_named", n2: 2, n3: B(2))); +} diff --git a/tests/language_2/stack_trace/stack_trace_test.dart b/tests/language_2/stack_trace/stack_trace_test.dart index 5e4d7c65fa8..b1e60a95d13 100644 --- a/tests/language_2/stack_trace/stack_trace_test.dart +++ b/tests/language_2/stack_trace/stack_trace_test.dart @@ -88,6 +88,7 @@ class RethrowStacktraceTest { var config = 0; @pragma("vm:entry-point") // Prevent obfuscation + @pragma("vm:never-inline") // Prevent inlining issue12940() { throw "Progy"; } @@ -121,7 +122,7 @@ class RethrowStacktraceTest { try { d(); } catch (e, s) { - Expect.isTrue(s.toString().contains("issue12940")); + Expect.contains("issue12940", s.toString()); } } } diff --git a/tools/bots/test_matrix.json b/tools/bots/test_matrix.json index 582dd8223d5..6cca3d04f8c 100644 --- a/tools/bots/test_matrix.json +++ b/tools/bots/test_matrix.json @@ -748,7 +748,7 @@ "builder-tag": "vm_nnbd" } }, - "dartkp-weak-asserts-(linux|mac)-(debug|product|release)-simarm64": { + "dartkp-weak-asserts-(linux|mac)-(debug|product|release)-(simarm|simarm64)": { "options": { "enable-asserts": true, "use-elf": true, @@ -778,7 +778,7 @@ "builder-tag": "vm_nnbd" } }, - "dartkp-strong-(linux|mac)-(debug|product|release)-simarm64": { + "dartkp-strong-(linux|mac)-(debug|product|release)-(simarm|simarm64)": { "options": { "use-elf": true, "builder-tag": "vm_nnbd"