[vm/nnbd] Fix supertype check in GenerateSubtype1TestCacheLookup Before calling Subtype1TestCache stub, there was a quick check if supertype's class id matches tested class id. Null extends Object, so supertype of Null matches Object. However, in NNBD strong mode Null is not assignable to Object. Fixes https://github.com/dart-lang/sdk/issues/41272 Change-Id: Icc3efaf90992b5083cbe086af81db7504724b855 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/142085 Reviewed-by: Régis Crelier <regis@google.com> Commit-Queue: Alexander Markov <alexmarkov@google.com>
diff --git a/runtime/vm/compiler/backend/flow_graph_compiler_arm.cc b/runtime/vm/compiler/backend/flow_graph_compiler_arm.cc index c32f5d1..d1d05c6 100644 --- a/runtime/vm/compiler/backend/flow_graph_compiler_arm.cc +++ b/runtime/vm/compiler/backend/flow_graph_compiler_arm.cc
@@ -428,13 +428,17 @@ __ LoadClassId(R2, TypeTestABI::kInstanceReg); __ LoadClassById(R1, R2); // R1: instance class. - // Check immediate superclass equality. - __ ldr(R2, compiler::FieldAddress( - R1, compiler::target::Class::super_type_offset())); - __ ldr(R2, compiler::FieldAddress( - R2, compiler::target::Type::type_class_id_offset())); - __ CompareImmediate(R2, Smi::RawValue(type_class.id())); - __ b(is_instance_lbl, EQ); + // Check immediate superclass equality. If type_class is Object, then testing + // supertype may yield a wrong result for Null in NNBD strong mode (because + // Null also extends Object). + if (!type_class.IsObjectClass() || !Isolate::Current()->null_safety()) { + __ ldr(R2, compiler::FieldAddress( + R1, compiler::target::Class::super_type_offset())); + __ ldr(R2, compiler::FieldAddress( + R2, compiler::target::Type::type_class_id_offset())); + __ CompareImmediate(R2, Smi::RawValue(type_class.id())); + __ b(is_instance_lbl, EQ); + } const Register kInstantiatorTypeArgumentsReg = kNoRegister; const Register kFunctionTypeArgumentsReg = kNoRegister;
diff --git a/runtime/vm/compiler/backend/flow_graph_compiler_arm64.cc b/runtime/vm/compiler/backend/flow_graph_compiler_arm64.cc index 20cc37d..fc9b93c 100644 --- a/runtime/vm/compiler/backend/flow_graph_compiler_arm64.cc +++ b/runtime/vm/compiler/backend/flow_graph_compiler_arm64.cc
@@ -414,11 +414,15 @@ __ LoadClassId(TMP, TypeTestABI::kInstanceReg); __ LoadClassById(R1, TMP); // R1: instance class. - // Check immediate superclass equality. - __ LoadFieldFromOffset(R2, R1, Class::super_type_offset()); - __ LoadFieldFromOffset(R2, R2, Type::type_class_id_offset()); - __ CompareImmediate(R2, Smi::RawValue(type_class.id())); - __ b(is_instance_lbl, EQ); + // Check immediate superclass equality. If type_class is Object, then testing + // supertype may yield a wrong result for Null in NNBD strong mode (because + // Null also extends Object). + if (!type_class.IsObjectClass() || !Isolate::Current()->null_safety()) { + __ LoadFieldFromOffset(R2, R1, Class::super_type_offset()); + __ LoadFieldFromOffset(R2, R2, Type::type_class_id_offset()); + __ CompareImmediate(R2, Smi::RawValue(type_class.id())); + __ b(is_instance_lbl, EQ); + } const Register kInstantiatorTypeArgumentsReg = kNoRegister; const Register kFunctionTypeArgumentsReg = kNoRegister;
diff --git a/runtime/vm/compiler/backend/flow_graph_compiler_ia32.cc b/runtime/vm/compiler/backend/flow_graph_compiler_ia32.cc index d362feb..579430c 100644 --- a/runtime/vm/compiler/backend/flow_graph_compiler_ia32.cc +++ b/runtime/vm/compiler/backend/flow_graph_compiler_ia32.cc
@@ -401,11 +401,15 @@ __ LoadClassId(EDI, TypeTestABI::kInstanceReg); __ LoadClassById(ECX, EDI); // ECX: instance class. - // Check immediate superclass equality. - __ movl(EDI, compiler::FieldAddress(ECX, Class::super_type_offset())); - __ movl(EDI, compiler::FieldAddress(EDI, Type::type_class_id_offset())); - __ cmpl(EDI, compiler::Immediate(Smi::RawValue(type_class.id()))); - __ j(EQUAL, is_instance_lbl); + // Check immediate superclass equality. If type_class is Object, then testing + // supertype may yield a wrong result for Null in NNBD strong mode (because + // Null also extends Object). + if (!type_class.IsObjectClass() || !Isolate::Current()->null_safety()) { + __ movl(EDI, compiler::FieldAddress(ECX, Class::super_type_offset())); + __ movl(EDI, compiler::FieldAddress(EDI, Type::type_class_id_offset())); + __ cmpl(EDI, compiler::Immediate(Smi::RawValue(type_class.id()))); + __ j(EQUAL, is_instance_lbl); + } const Register kInstantiatorTypeArgumentsReg = kNoRegister; const Register kFunctionTypeArgumentsReg = kNoRegister;
diff --git a/runtime/vm/compiler/backend/flow_graph_compiler_x64.cc b/runtime/vm/compiler/backend/flow_graph_compiler_x64.cc index f1f0c6b..151047f 100644 --- a/runtime/vm/compiler/backend/flow_graph_compiler_x64.cc +++ b/runtime/vm/compiler/backend/flow_graph_compiler_x64.cc
@@ -433,11 +433,16 @@ __ LoadClassId(TMP, TypeTestABI::kInstanceReg); __ LoadClassById(R10, TMP); // R10: instance class. - // Check immediate superclass equality. - __ movq(R13, compiler::FieldAddress(R10, Class::super_type_offset())); - __ movq(R13, compiler::FieldAddress(R13, Type::type_class_id_offset())); - __ CompareImmediate(R13, compiler::Immediate(Smi::RawValue(type_class.id()))); - __ j(EQUAL, is_instance_lbl); + // Check immediate superclass equality. If type_class is Object, then testing + // supertype may yield a wrong result for Null in NNBD strong mode (because + // Null also extends Object). + if (!type_class.IsObjectClass() || !Isolate::Current()->null_safety()) { + __ movq(R13, compiler::FieldAddress(R10, Class::super_type_offset())); + __ movq(R13, compiler::FieldAddress(R13, Type::type_class_id_offset())); + __ CompareImmediate(R13, + compiler::Immediate(Smi::RawValue(type_class.id()))); + __ j(EQUAL, is_instance_lbl); + } const Register kInstantiatorTypeArgumentsReg = kNoRegister; const Register kFunctionTypeArgumentsReg = kNoRegister;
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index 2afe44c..90d1230 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc
@@ -17827,7 +17827,8 @@ } const Class& cls = Class::Handle(zone, clazz()); if (cls.IsClosureClass()) { - if (other.IsDartFunctionType() || other.IsDartClosureType()) { + if (other.IsDartFunctionType() || other.IsDartClosureType() || + other.IsObjectType()) { return true; } AbstractType& instantiated_other = AbstractType::Handle(zone, other.raw());
diff --git a/tests/language/nnbd/subtyping/regress41272_test.dart b/tests/language/nnbd/subtyping/regress41272_test.dart new file mode 100644 index 0000000..2519404 --- /dev/null +++ b/tests/language/nnbd/subtyping/regress41272_test.dart
@@ -0,0 +1,24 @@ +// 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. + +// VMOptions=--optimization_counter_threshold=10 --deterministic +// Requirements=nnbd-strong + +// Verifies that null cannot be casted to Object. +// Regression test for https://github.com/dart-lang/sdk/issues/41272. + +import 'package:expect/expect.dart'; + +doTest() { + dynamic x; + Expect.throwsTypeError(() { + x as Object; + }); +} + +main() { + for (int i = 0; i < 20; ++i) { + doTest(); + } +}