[vm] Prevent late fields from being unboxed

When unboxed doubles are assigned to late fields, it conflicts with the
late field logic and causes a crash.

Bug: #38841
Bug: #39658
Change-Id: I641f597006114f02473a20f8c2526eeaf4fe813f
Fixes: #39658
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/127442
Commit-Queue: Liam Appelbe <liama@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>

Author: liamappelbe

runtime/vm/compiler/backend/slot_test.cc

@@ -54,8 +54,9 @@ TEST_CASE(SlotFromGuardedField) {
   const Field& field = Field::Handle(
       Field::New(String::Handle(Symbols::New(thread, "field")),
                  /*is_static=*/false, /*is_final=*/false, /*is_const=*/false,
-                 /*is_reflectable=*/true, dummy_class, Object::dynamic_type(),
-                 TokenPosition::kMinSource, TokenPosition::kMinSource));
+                 /*is_reflectable=*/true, /*is_late=*/false, dummy_class,
+                 Object::dynamic_type(), TokenPosition::kMinSource,
+                 TokenPosition::kMinSource));
 
   // Set non-trivial guarded state on the field.
   field.set_guarded_cid(kSmiCid);

runtime/vm/compiler/backend/type_propagator_test.cc

@@ -184,7 +184,8 @@ ISOLATE_UNIT_TEST_CASE(TypePropagator_Refinement) {
                  /*is_static=*/true,
                  /*is_final=*/false,
                  /*is_const=*/false,
-                 /*is_reflectable=*/true, object_class, Object::dynamic_type(),
+                 /*is_reflectable=*/true,
+                 /*is_late=*/false, object_class, Object::dynamic_type(),
                  TokenPosition::kNoSource, TokenPosition::kNoSource));
 
   FlowGraphBuilderHelper H;

runtime/vm/compiler/frontend/bytecode_reader.cc

@@ -2017,16 +2017,15 @@ void BytecodeReaderHelper::ReadFieldDeclarations(const Class& cls,
     }
 
     field = Field::New(name, is_static, is_final, is_const,
-                       (flags & kIsReflectableFlag) != 0, script_class, type,
-                       position, end_position);
+                       (flags & kIsReflectableFlag) != 0, is_late, script_class,
+                       type, position, end_position);
 
     field.set_is_declared_in_bytecode(true);
     field.set_has_pragma(has_pragma);
     field.set_is_covariant((flags & kIsCovariantFlag) != 0);
     field.set_is_generic_covariant_impl((flags & kIsGenericCovariantImplFlag) !=
                                         0);
     field.set_has_nontrivial_initializer(has_nontrivial_initializer);
-    field.set_is_late((flags & kIsLateFlag) != 0);
     field.set_is_extension_member(is_extension_member);
     field.set_has_initializer(has_initializer);
 
@@ -2134,13 +2133,13 @@ void BytecodeReaderHelper::ReadFieldDeclarations(const Class& cls,
 
   if (cls.is_enum_class()) {
     // Add static field 'const _deleted_enum_sentinel'.
-    field =
-        Field::New(Symbols::_DeletedEnumSentinel(),
-                   /* is_static = */ true,
-                   /* is_final = */ true,
-                   /* is_const = */ true,
-                   /* is_reflectable = */ false, cls, Object::dynamic_type(),
-                   TokenPosition::kNoSource, TokenPosition::kNoSource);
+    field = Field::New(Symbols::_DeletedEnumSentinel(),
+                       /* is_static = */ true,
+                       /* is_final = */ true,
+                       /* is_const = */ true,
+                       /* is_reflectable = */ false,
+                       /* is_late = */ false, cls, Object::dynamic_type(),
+                       TokenPosition::kNoSource, TokenPosition::kNoSource);
 
     fields.SetAt(num_fields, field);
   }

runtime/vm/kernel_loader.cc

@@ -1187,12 +1187,11 @@ void KernelLoader::FinishTopLevelClassLoading(
     const bool is_late = field_helper.IsLate();
     const bool is_extension_member = field_helper.IsExtensionMember();
     const Field& field = Field::Handle(
-        Z,
-        Field::NewTopLevel(name, is_final, field_helper.IsConst(), script_class,
-                           field_helper.position_, field_helper.end_position_));
+        Z, Field::NewTopLevel(name, is_final, field_helper.IsConst(), is_late,
+                              script_class, field_helper.position_,
+                              field_helper.end_position_));
     field.set_kernel_offset(field_offset);
     field.set_has_pragma(has_pragma_annotation);
-    field.set_is_late(is_late);
     field.set_is_extension_member(is_extension_member);
     const AbstractType& type = T.BuildType();  // read type.
     field.SetFieldType(type);
@@ -1541,16 +1540,15 @@ void KernelLoader::FinishClassLoading(const Class& klass,
       const bool is_late = field_helper.IsLate();
       const bool is_extension_member = field_helper.IsExtensionMember();
       Field& field = Field::Handle(
-          Z,
-          Field::New(name, field_helper.IsStatic(), is_final,
-                     field_helper.IsConst(), is_reflectable, script_class, type,
-                     field_helper.position_, field_helper.end_position_));
+          Z, Field::New(name, field_helper.IsStatic(), is_final,
+                        field_helper.IsConst(), is_reflectable, is_late,
+                        script_class, type, field_helper.position_,
+                        field_helper.end_position_));
       field.set_kernel_offset(field_offset);
       field.set_has_pragma(has_pragma_annotation);
       field.set_is_covariant(field_helper.IsCovariant());
       field.set_is_generic_covariant_impl(
           field_helper.IsGenericCovariantImpl());
-      field.set_is_late(is_late);
       field.set_is_extension_member(is_extension_member);
       ReadInferredType(field, field_offset + library_kernel_offset_);
       CheckForInitializer(field);
@@ -1575,13 +1573,14 @@ void KernelLoader::FinishClassLoading(const Class& klass,
       // Add static field 'const _deleted_enum_sentinel'.
       // This field does not need to be of type E.
       Field& deleted_enum_sentinel = Field::ZoneHandle(Z);
-      deleted_enum_sentinel = Field::New(
-          Symbols::_DeletedEnumSentinel(),
-          /* is_static = */ true,
-          /* is_final = */ true,
-          /* is_const = */ true,
-          /* is_reflectable = */ false, klass, Object::dynamic_type(),
-          TokenPosition::kNoSource, TokenPosition::kNoSource);
+      deleted_enum_sentinel =
+          Field::New(Symbols::_DeletedEnumSentinel(),
+                     /* is_static = */ true,
+                     /* is_final = */ true,
+                     /* is_const = */ true,
+                     /* is_reflectable = */ false,
+                     /* is_late = */ false, klass, Object::dynamic_type(),
+                     TokenPosition::kNoSource, TokenPosition::kNoSource);
       fields_.Add(&deleted_enum_sentinel);
     }
 

runtime/vm/object.cc

@@ -3834,7 +3834,11 @@ bool Class::InjectCIDFields() const {
   const AbstractType& field_type = Type::Handle(zone, Type::IntType());
   for (size_t i = 0; i < ARRAY_SIZE(cid_fields); i++) {
     field_name = Symbols::New(thread, cid_fields[i].field_name);
-    field = Field::New(field_name, true, false, true, false, *this, field_type,
+    field = Field::New(field_name, /* is_static = */ true,
+                       /* is_final = */ false,
+                       /* is_const = */ true,
+                       /* is_reflectable = */ false,
+                       /* is_late = */ false, *this, field_type,
                        TokenPosition::kMinSource, TokenPosition::kMinSource);
     value = Smi::New(cid_fields[i].cid);
     field.SetStaticValue(value, true);
@@ -8746,6 +8750,7 @@ void Field::InitializeNew(const Field& result,
                           bool is_final,
                           bool is_const,
                           bool is_reflectable,
+                          bool is_late,
                           const Object& owner,
                           TokenPosition token_pos,
                           TokenPosition end_token_pos) {
@@ -8758,13 +8763,14 @@ void Field::InitializeNew(const Field& result,
   result.set_is_final(is_final);
   result.set_is_const(is_const);
   result.set_is_reflectable(is_reflectable);
+  result.set_is_late(is_late);
   result.set_is_double_initialized(false);
   result.set_owner(owner);
   result.set_token_pos(token_pos);
   result.set_end_token_pos(end_token_pos);
   result.set_has_nontrivial_initializer(false);
   result.set_has_initializer(false);
-  result.set_is_unboxing_candidate(!is_final);
+  result.set_is_unboxing_candidate(!is_final && !is_late);
   result.set_initializer_changed_after_initialization(false);
   NOT_IN_PRECOMPILED(result.set_is_declared_in_bytecode(false));
   NOT_IN_PRECOMPILED(result.set_binary_declaration_offset(0));
@@ -8800,29 +8806,31 @@ RawField* Field::New(const String& name,
                      bool is_final,
                      bool is_const,
                      bool is_reflectable,
+                     bool is_late,
                      const Object& owner,
                      const AbstractType& type,
                      TokenPosition token_pos,
                      TokenPosition end_token_pos) {
   ASSERT(!owner.IsNull());
   const Field& result = Field::Handle(Field::New());
   InitializeNew(result, name, is_static, is_final, is_const, is_reflectable,
-                owner, token_pos, end_token_pos);
+                is_late, owner, token_pos, end_token_pos);
   result.SetFieldType(type);
   return result.raw();
 }
 
 RawField* Field::NewTopLevel(const String& name,
                              bool is_final,
                              bool is_const,
+                             bool is_late,
                              const Object& owner,
                              TokenPosition token_pos,
                              TokenPosition end_token_pos) {
   ASSERT(!owner.IsNull());
   const Field& result = Field::Handle(Field::New());
   InitializeNew(result, name, true,       /* is_static */
                 is_final, is_const, true, /* is_reflectable */
-                owner, token_pos, end_token_pos);
+                is_late, owner, token_pos, end_token_pos);
   return result.raw();
 }
 
@@ -10240,6 +10248,7 @@ void Library::AddMetadata(const Object& owner,
       Field::Handle(zone, Field::NewTopLevel(metaname,
                                              false,  // is_final
                                              false,  // is_const
+                                             false,  // is_late
                                              owner, token_pos, token_pos));
   field.SetFieldType(Object::dynamic_type());
   field.set_is_reflectable(false);
@@ -12002,6 +12011,7 @@ void Namespace::AddMetadata(const Object& owner,
   Field& field = Field::Handle(Field::NewTopLevel(Symbols::TopLevel(),
                                                   false,  // is_final
                                                   false,  // is_const
+                                                  false,  // is_late
                                                   owner, token_pos, token_pos));
   field.set_is_reflectable(false);
   field.SetFieldType(Object::dynamic_type());

runtime/vm/object.h

@@ -3603,6 +3603,7 @@ class Field : public Object {
                        bool is_final,
                        bool is_const,
                        bool is_reflectable,
+                       bool is_late,
                        const Object& owner,
                        const AbstractType& type,
                        TokenPosition token_pos,
@@ -3611,6 +3612,7 @@ class Field : public Object {
   static RawField* NewTopLevel(const String& name,
                                bool is_final,
                                bool is_const,
+                               bool is_late,
                                const Object& owner,
                                TokenPosition token_pos,
                                TokenPosition end_token_pos);
@@ -3854,6 +3856,7 @@ class Field : public Object {
                             bool is_final,
                             bool is_const,
                             bool is_reflectable,
+                            bool is_late,
                             const Object& owner,
                             TokenPosition token_pos,
                             TokenPosition end_token_pos);

runtime/vm/object_test.cc

@@ -234,7 +234,7 @@ ISOLATE_UNIT_TEST_CASE(InstanceClass) {
   const Array& one_fields = Array::Handle(Array::New(1));
   const String& field_name = String::Handle(Symbols::New(thread, "the_field"));
   const Field& field = Field::Handle(
-      Field::New(field_name, false, false, false, true, one_field_class,
+      Field::New(field_name, false, false, false, true, false, one_field_class,
                  Object::dynamic_type(), TokenPosition::kMinSource,
                  TokenPosition::kMinSource));
   one_fields.SetAt(0, field);
@@ -2852,7 +2852,7 @@ static RawField* CreateTestField(const char* name) {
   const String& field_name =
       String::Handle(Symbols::New(Thread::Current(), name));
   const Field& field = Field::Handle(Field::New(
-      field_name, true, false, false, true, cls, Object::dynamic_type(),
+      field_name, true, false, false, true, false, cls, Object::dynamic_type(),
       TokenPosition::kMinSource, TokenPosition::kMinSource));
   return field.raw();
 }

tests/language_2/nnbd/syntax/late_modifier_bug_39658.dart

@@ -0,0 +1,64 @@
+// Copyright (c) 2019, 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.
+
+// SharedOptions=--enable-experiment=non-nullable
+import 'package:expect/expect.dart';
+
+int initCalls = 0;
+double init() {
+  ++initCalls;
+  return 1.23;
+}
+
+class A {
+  late double? fieldWithInit = init();
+  late double fieldWithTrivialInit = 1.23;
+  late double? fieldWithNullInit = null;
+  late double fieldWithNoInit;
+}
+
+main() {
+  // Late, non-final, with init.
+  var a = A();
+  Expect.equals(0, initCalls);
+  Expect.equals(1.23, a.fieldWithInit);
+  Expect.equals(1.23, a.fieldWithTrivialInit);
+  Expect.equals(null, a.fieldWithNullInit);
+  Expect.throws(
+      () => a.fieldWithNoInit, (error) => error is LateInitializationError);
+  Expect.equals(1, initCalls);
+  Expect.equals(1.23, a.fieldWithInit);
+  Expect.equals(1.23, a.fieldWithTrivialInit);
+  Expect.equals(null, a.fieldWithNullInit);
+  Expect.throws(
+      () => a.fieldWithNoInit, (error) => error is LateInitializationError);
+  Expect.equals(1, initCalls);
+  a.fieldWithInit = 4.56;
+  a.fieldWithTrivialInit = 4.56;
+  a.fieldWithNullInit = 4.56;
+  a.fieldWithNoInit = 4.56;
+  Expect.equals(1, initCalls);
+  Expect.equals(4.56, a.fieldWithInit);
+  Expect.equals(4.56, a.fieldWithTrivialInit);
+  Expect.equals(4.56, a.fieldWithNullInit);
+  Expect.equals(4.56, a.fieldWithNoInit);
+  Expect.equals(1, initCalls);
+  initCalls = 0;
+
+  // Late, non-final, with init that's pre-empted by setter.
+  var b = A();
+  Expect.equals(0, initCalls);
+  b.fieldWithInit = 4.56;
+  Expect.equals(0, initCalls);
+  Expect.equals(4.56, b.fieldWithInit);
+  Expect.equals(0, initCalls);
+
+  // Late, non-final, with init that's pre-empted by null setter.
+  var c = A();
+  Expect.equals(0, initCalls);
+  c.fieldWithInit = null;
+  Expect.equals(0, initCalls);
+  Expect.equals(null, c.fieldWithInit);
+  Expect.equals(0, initCalls);
+}