[vm/bytecode] Record null-initialized fields in bytecode

If a field is initialized with null (either explicitly or implicitly),
field store can be omitted in bytecode. In such case, bytecode should
still convey the information about this initialization to VM for
field guards to work correctly.

Change-Id: I1fd45b858c3c521b97fa5dbffe0e15b1ea75d92f
Reviewed-on: https://dart-review.googlesource.com/76060
Reviewed-by: Zach Anderson <zra@google.com>
Reviewed-by: RĂ©gis Crelier <regis@google.com>
diff --git a/pkg/vm/lib/bytecode/gen_bytecode.dart b/pkg/vm/lib/bytecode/gen_bytecode.dart
index 7de7ecd..24b30f5 100644
--- a/pkg/vm/lib/bytecode/gen_bytecode.dart
+++ b/pkg/vm/lib/bytecode/gen_bytecode.dart
@@ -81,6 +81,8 @@
   List<Label> yieldPoints;
   Map<TreeNode, int> contextLevels;
   List<ClosureBytecode> closures;
+  Set<Field> initializedFields;
+  List<Reference> nullableFields;
   ConstantPool cp;
   ConstantEmitter constantEmitter;
   BytecodeAssembler asm;
@@ -263,20 +265,35 @@
       libraryIndex.getTopLevelMember('dart:_internal', 'unsafeCast');
 
   void _genConstructorInitializers(Constructor node) {
-    bool isRedirecting =
+    final bool isRedirecting =
         node.initializers.any((init) => init is RedirectingInitializer);
+
     if (!isRedirecting) {
+      initializedFields = new Set<Field>();
       for (var field in node.enclosingClass.fields) {
         if (!field.isStatic && field.initializer != null) {
           _genFieldInitializer(field, field.initializer);
         }
       }
     }
+
     visitList(node.initializers, this);
+
+    if (!isRedirecting) {
+      nullableFields = <Reference>[];
+      for (var field in node.enclosingClass.fields) {
+        if (!field.isStatic && !initializedFields.contains(field)) {
+          nullableFields.add(field.reference);
+        }
+      }
+      initializedFields = null; // No more initialized fields, please.
+    }
   }
 
   void _genFieldInitializer(Field field, Expression initializer) {
-    if (initializer is NullLiteral) {
+    assert(!field.isStatic);
+
+    if (initializer is NullLiteral && !initializedFields.contains(field)) {
       return;
     }
 
@@ -285,6 +302,8 @@
 
     final int cpIndex = cp.add(new ConstantInstanceField(field));
     asm.emitStoreFieldTOS(cpIndex);
+
+    initializedFields.add(field);
   }
 
   void _genArguments(Expression receiver, Arguments arguments) {
@@ -677,6 +696,8 @@
     yieldPoints = null; // Initialized when entering sync-yielding closure.
     contextLevels = <TreeNode, int>{};
     closures = <ClosureBytecode>[];
+    initializedFields = null; // Tracked for constructors only.
+    nullableFields = const <Reference>[];
     cp = new ConstantPool();
     constantEmitter = new ConstantEmitter(cp);
     asm = new BytecodeAssembler();
@@ -716,8 +737,8 @@
 
   void end(Member node) {
     if (!hasErrors) {
-      metadata.mapping[node] =
-          new BytecodeMetadata(cp, asm.bytecode, asm.exceptionsTable, closures);
+      metadata.mapping[node] = new BytecodeMetadata(
+          cp, asm.bytecode, asm.exceptionsTable, nullableFields, closures);
     }
 
     enclosingClass = null;
@@ -737,6 +758,8 @@
     yieldPoints = null;
     contextLevels = null;
     closures = null;
+    initializedFields = null;
+    nullableFields = null;
     cp = null;
     constantEmitter = null;
     asm = null;
diff --git a/pkg/vm/lib/metadata/bytecode.dart b/pkg/vm/lib/metadata/bytecode.dart
index aae9218..40c664c 100644
--- a/pkg/vm/lib/metadata/bytecode.dart
+++ b/pkg/vm/lib/metadata/bytecode.dart
@@ -14,9 +14,18 @@
 /// In kernel binary, bytecode metadata is encoded as following:
 ///
 /// type BytecodeMetadata {
+///   UInt flags (HasExceptionsTable, HasNullableFields, HasClosures)
+///
 ///   ConstantPool constantPool
 ///   List<Byte> bytecodes
+///
+///   (optional, present if HasExceptionsTable)
 ///   ExceptionsTable exceptionsTable
+///
+///   (optional, present if HasNullableFields)
+///   List<CanonicalName> nullableFields
+///
+///   (optional, present if HasClosures)
 ///   List<ClosureBytecode> closures
 /// }
 ///
@@ -33,13 +42,27 @@
 /// pkg/vm/lib/bytecode/constant_pool.dart.
 ///
 class BytecodeMetadata {
+  static const hasExceptionsTableFlag = 1 << 0;
+  static const hasNullableFieldsFlag = 1 << 1;
+  static const hasClosuresFlag = 1 << 2;
+
   final ConstantPool constantPool;
   final List<int> bytecodes;
   final ExceptionsTable exceptionsTable;
+  final List<Reference> nullableFields;
   final List<ClosureBytecode> closures;
 
-  BytecodeMetadata(
-      this.constantPool, this.bytecodes, this.exceptionsTable, this.closures);
+  bool get hasExceptionsTable => exceptionsTable.blocks.isNotEmpty;
+  bool get hasNullableFields => nullableFields.isNotEmpty;
+  bool get hasClosures => closures.isNotEmpty;
+
+  int get flags =>
+      (hasExceptionsTable ? hasExceptionsTableFlag : 0) |
+      (hasNullableFields ? hasNullableFieldsFlag : 0) |
+      (hasClosures ? hasClosuresFlag : 0);
+
+  BytecodeMetadata(this.constantPool, this.bytecodes, this.exceptionsTable,
+      this.nullableFields, this.closures);
 
   // TODO(alexmarkov): Consider printing constant pool before bytecode.
   @override
@@ -47,6 +70,7 @@
       "Bytecode {\n"
       "${new BytecodeDisassembler().disassemble(bytecodes, exceptionsTable)}}\n"
       "$exceptionsTable"
+      "${nullableFields.isEmpty ? '' : 'Nullable fields: ${nullableFields.map((ref) => ref.asField).toList()}\n'}"
       "$constantPool"
       "${closures.join('\n')}";
 }
@@ -97,22 +121,44 @@
 
   @override
   void writeToBinary(BytecodeMetadata metadata, Node node, BinarySink sink) {
+    sink.writeUInt30(metadata.flags);
     metadata.constantPool.writeToBinary(node, sink);
     sink.writeByteList(metadata.bytecodes);
-    metadata.exceptionsTable.writeToBinary(sink);
-    sink.writeUInt30(metadata.closures.length);
-    metadata.closures.forEach((c) => c.writeToBinary(sink));
+    if (metadata.hasExceptionsTable) {
+      metadata.exceptionsTable.writeToBinary(sink);
+    }
+    if (metadata.hasNullableFields) {
+      sink.writeUInt30(metadata.nullableFields.length);
+      metadata.nullableFields.forEach((ref) => sink
+          .writeCanonicalNameReference(getCanonicalNameOfMember(ref.asField)));
+    }
+    if (metadata.hasClosures) {
+      sink.writeUInt30(metadata.closures.length);
+      metadata.closures.forEach((c) => c.writeToBinary(sink));
+    }
   }
 
   @override
   BytecodeMetadata readFromBinary(Node node, BinarySource source) {
+    int flags = source.readUInt();
     final ConstantPool constantPool =
         new ConstantPool.readFromBinary(node, source);
     final List<int> bytecodes = source.readByteList();
-    final exceptionsTable = new ExceptionsTable.readFromBinary(source);
-    final List<ClosureBytecode> closures = new List<ClosureBytecode>.generate(
-        source.readUInt(), (_) => new ClosureBytecode.readFromBinary(source));
+    final exceptionsTable =
+        ((flags & BytecodeMetadata.hasExceptionsTableFlag) != 0)
+            ? new ExceptionsTable.readFromBinary(source)
+            : new ExceptionsTable();
+    final List<Reference> nullableFields =
+        ((flags & BytecodeMetadata.hasNullableFieldsFlag) != 0)
+            ? new List<Reference>.generate(source.readUInt(),
+                (_) => source.readCanonicalNameReference().getReference())
+            : const <Reference>[];
+    final List<ClosureBytecode> closures =
+        ((flags & BytecodeMetadata.hasClosuresFlag) != 0)
+            ? new List<ClosureBytecode>.generate(source.readUInt(),
+                (_) => new ClosureBytecode.readFromBinary(source))
+            : const <ClosureBytecode>[];
     return new BytecodeMetadata(
-        constantPool, bytecodes, exceptionsTable, closures);
+        constantPool, bytecodes, exceptionsTable, nullableFields, closures);
   }
 }
diff --git a/pkg/vm/testcases/bytecode/closures.dart.expect b/pkg/vm/testcases/bytecode/closures.dart.expect
index 10f84a5..af62c71 100644
--- a/pkg/vm/testcases/bytecode/closures.dart.expect
+++ b/pkg/vm/testcases/bytecode/closures.dart.expect
@@ -496,6 +496,7 @@
   PushNull
   ReturnTOS
 }
+Nullable fields: [#lib::B::foo]
 ConstantPool {
   [0] = ArgDesc num-args 1, num-type-args 0, names []
   [1] = StaticICData target 'dart.core::Object::', arg-desc CP#0
diff --git a/pkg/vm/testcases/bytecode/field_initializers.dart.expect b/pkg/vm/testcases/bytecode/field_initializers.dart.expect
index 5268d47..16ddfcb 100644
--- a/pkg/vm/testcases/bytecode/field_initializers.dart.expect
+++ b/pkg/vm/testcases/bytecode/field_initializers.dart.expect
@@ -31,6 +31,7 @@
   PushNull
   ReturnTOS
 }
+Nullable fields: [#lib::A::foo1, #lib::A::foo2]
 ConstantPool {
   [0] = InstanceField #lib::A::foo3
   [1] = Reserved
@@ -69,6 +70,7 @@
   PushNull
   ReturnTOS
 }
+Nullable fields: [#lib::A::foo2, #lib::A::foo4]
 ConstantPool {
   [0] = InstanceField #lib::A::foo3
   [1] = Reserved
diff --git a/pkg/vm/testcases/bytecode/instance_creation.dart.expect b/pkg/vm/testcases/bytecode/instance_creation.dart.expect
index 127251d..e0b0287 100644
--- a/pkg/vm/testcases/bytecode/instance_creation.dart.expect
+++ b/pkg/vm/testcases/bytecode/instance_creation.dart.expect
@@ -48,6 +48,7 @@
   PushNull
   ReturnTOS
 }
+Nullable fields: [#lib::Base::t1, #lib::Base::t2]
 ConstantPool {
   [0] = ArgDesc num-args 1, num-type-args 0, names []
   [1] = StaticICData target 'dart.core::Object::', arg-desc CP#0
diff --git a/runtime/vm/compiler/frontend/bytecode_reader.cc b/runtime/vm/compiler/frontend/bytecode_reader.cc
index c68b03f..ac4d998 100644
--- a/runtime/vm/compiler/frontend/bytecode_reader.cc
+++ b/runtime/vm/compiler/frontend/bytecode_reader.cc
@@ -49,6 +49,15 @@
   AlternativeReadingScope alt(&helper_->reader_, &H.metadata_payloads(),
                               md_offset);
 
+  const int kHasExceptionsTableFlag = 1 << 0;
+  const int kHasNullableFieldsFlag = 1 << 1;
+  const int kHasClosuresFlag = 1 << 2;
+
+  const intptr_t flags = helper_->reader_.ReadUInt();
+  const bool has_exceptions_table = (flags & kHasExceptionsTableFlag) != 0;
+  const bool has_nullable_fields = (flags & kHasNullableFieldsFlag) != 0;
+  const bool has_closures = (flags & kHasClosuresFlag) != 0;
+
   // Create object pool and read pool entries.
   const intptr_t obj_count = helper_->reader_.ReadListLength();
   const ObjectPool& pool =
@@ -69,30 +78,51 @@
   function.AttachBytecode(bytecode);
 
   // Read exceptions table.
-  ReadExceptionsTable(bytecode);
+  ReadExceptionsTable(bytecode, has_exceptions_table);
 
   if (FLAG_dump_kernel_bytecode) {
     KernelBytecodeDisassembler::Disassemble(function);
   }
 
+  // Initialization of fields with null literal is elided from bytecode.
+  // Record the corresponding stores if field guards are enabled.
+  if (has_nullable_fields) {
+    ASSERT(function.IsGenerativeConstructor());
+    const intptr_t num_fields = helper_->ReadListLength();
+    if (I->use_field_guards()) {
+      Field& field = Field::Handle(helper_->zone_);
+      for (intptr_t i = 0; i < num_fields; i++) {
+        NameIndex name_index = helper_->ReadCanonicalNameReference();
+        field = H.LookupFieldByKernelField(name_index);
+        field.RecordStore(Object::null_object());
+      }
+    } else {
+      for (intptr_t i = 0; i < num_fields; i++) {
+        helper_->SkipCanonicalNameReference();
+      }
+    }
+  }
+
   // Read closures.
-  Function& closure = Function::Handle(helper_->zone_);
-  Code& closure_bytecode = Code::Handle(helper_->zone_);
-  intptr_t num_closures = helper_->ReadListLength();
-  for (intptr_t i = 0; i < num_closures; i++) {
-    intptr_t closure_index = helper_->ReadUInt();
-    ASSERT(closure_index < obj_count);
-    closure ^= pool.ObjectAt(closure_index);
+  if (has_closures) {
+    Function& closure = Function::Handle(helper_->zone_);
+    Code& closure_bytecode = Code::Handle(helper_->zone_);
+    const intptr_t num_closures = helper_->ReadListLength();
+    for (intptr_t i = 0; i < num_closures; i++) {
+      intptr_t closure_index = helper_->ReadUInt();
+      ASSERT(closure_index < obj_count);
+      closure ^= pool.ObjectAt(closure_index);
 
-    // Read closure bytecode and attach to closure function.
-    closure_bytecode = ReadBytecode(pool);
-    closure.AttachBytecode(closure_bytecode);
+      // Read closure bytecode and attach to closure function.
+      closure_bytecode = ReadBytecode(pool);
+      closure.AttachBytecode(closure_bytecode);
 
-    // Read closure exceptions table.
-    ReadExceptionsTable(closure_bytecode);
+      // Read closure exceptions table.
+      ReadExceptionsTable(closure_bytecode);
 
-    if (FLAG_dump_kernel_bytecode) {
-      KernelBytecodeDisassembler::Disassemble(closure);
+      if (FLAG_dump_kernel_bytecode) {
+        KernelBytecodeDisassembler::Disassemble(closure);
+      }
     }
   }
 }
@@ -580,13 +610,15 @@
                                 pool);
 }
 
-void BytecodeMetadataHelper::ReadExceptionsTable(const Code& bytecode) {
+void BytecodeMetadataHelper::ReadExceptionsTable(const Code& bytecode,
+                                                 bool has_exceptions_table) {
 #if !defined(PRODUCT)
   TimelineDurationScope tds(Thread::Current(), Timeline::GetCompilerStream(),
                             "BytecodeMetadataHelper::ReadExceptionsTable");
 #endif  // !defined(PRODUCT)
 
-  const intptr_t try_block_count = helper_->reader_.ReadListLength();
+  const intptr_t try_block_count =
+      has_exceptions_table ? helper_->reader_.ReadListLength() : 0;
   if (try_block_count > 0) {
     const ObjectPool& pool =
         ObjectPool::Handle(helper_->zone_, bytecode.object_pool());
diff --git a/runtime/vm/compiler/frontend/bytecode_reader.h b/runtime/vm/compiler/frontend/bytecode_reader.h
index 1ea8c46..f422003 100644
--- a/runtime/vm/compiler/frontend/bytecode_reader.h
+++ b/runtime/vm/compiler/frontend/bytecode_reader.h
@@ -31,7 +31,8 @@
                            const ObjectPool& pool,
                            intptr_t from_index);
   RawCode* ReadBytecode(const ObjectPool& pool);
-  void ReadExceptionsTable(const Code& bytecode);
+  void ReadExceptionsTable(const Code& bytecode,
+                           bool has_exceptions_table = true);
   RawTypedData* NativeEntry(const Function& function,
                             const String& external_name);
 
diff --git a/tests/corelib_2/corelib_2.status b/tests/corelib_2/corelib_2.status
index be05e34..e99b1d8 100644
--- a/tests/corelib_2/corelib_2.status
+++ b/tests/corelib_2/corelib_2.status
@@ -63,7 +63,7 @@
 [ $compiler == dartkb ]
 bigint_from_test: Pass, Crash # Please triage
 bigint_parse_radix_test: Pass, Crash # Please triage
-bigint_test: Pass, Crash # Please triage
+bigint_test: Pass, Crash, RuntimeError # Please triage
 
 [ $compiler == precompiler ]
 bigint_test: Pass, Timeout # --no_intrinsify
diff --git a/tests/language_2/language_2_kernel.status b/tests/language_2/language_2_kernel.status
index 8d5af69..c54844e 100644
--- a/tests/language_2/language_2_kernel.status
+++ b/tests/language_2/language_2_kernel.status
@@ -416,19 +416,19 @@
 assertion_initializer_const_error2_test/cc10: MissingCompileTimeError # Not reporting failed assert() at compile time.
 
 [ $compiler == dartkb && $runtime == vm && $strong ]
+async_star_test/03: Pass, RuntimeError # Please triage
+async_star_test/04: Pass, RuntimeError # Please triage
 compile_time_constant_o_test/01: Pass
 compile_time_constant_o_test/02: Pass
 const_dynamic_type_literal_test/02: Pass
 map_literal3_test/01: Pass
 map_literal3_test/02: Pass
-no_such_method_dispatcher_test: Pass, RuntimeError # Please triage.
 type_alias_equality_test/02: RuntimeError # Issue 32783
 vm/bool_check_stack_traces_test/01: RuntimeError # No support for line numbers in stacktraces
 vm/bool_check_stack_traces_test/none: RuntimeError # No support for line numbers in stacktraces
 vm/causal_async_exception_stack2_test: RuntimeError # No support for line numbers in stacktraces
 vm/causal_async_exception_stack_test: RuntimeError # No support for line numbers in stacktraces
 vm/regress_28325_test: RuntimeError # No support for line numbers in stacktraces
-vm/regress_flutter_21957_test: Pass, RuntimeError # Please triage.
 
 [ $compiler == dartkp && $mode == debug && $runtime == dart_precompiled && $strong ]
 external_test/13: Crash
diff --git a/tests/lib_2/lib_2_kernel.status b/tests/lib_2/lib_2_kernel.status
index f39040a..ed0e1cf 100644
--- a/tests/lib_2/lib_2_kernel.status
+++ b/tests/lib_2/lib_2_kernel.status
@@ -69,6 +69,8 @@
 
 # The failures below still need to be investigated and possibly fixed, or marked as skipped.
 [ $compiler == dartkb && $strong ]
+convert/streamed_conversion_json_utf8_decode_test: Pass, Timeout # Please triage.
+io/file_lock_test: Pass, Timeout # Please triage.
 isolate/isolate_complex_messages_test: Pass, Crash # Non-canonical type arguments.
 isolate/mandel_isolate_test: Pass, Timeout, Crash # Please triage.
 isolate/spawn_uri_exported_main_test: Pass, RuntimeError # Please triage.
diff --git a/tests/standalone_2/standalone_2_kernel.status b/tests/standalone_2/standalone_2_kernel.status
index 4904556..011c128 100644
--- a/tests/standalone_2/standalone_2_kernel.status
+++ b/tests/standalone_2/standalone_2_kernel.status
@@ -57,6 +57,7 @@
 [ $compiler == dartkb && $strong ]
 io/dart_std_io_pipe_test: Pass, Crash # Trap bytecode encountered.
 io/file_lock_test: Pass, Crash # Trap bytecode encountered.
+io/http_advanced_test: Pass, RuntimeError # Please triage.
 io/http_client_connect_test: Pass, RuntimeError # Please triage.
 io/http_outgoing_size_test: Pass, RuntimeError # Please triage.
 io/platform_test: RuntimeError # Platform.script points to dill file.
@@ -64,13 +65,14 @@
 io/test_extension_fail_test: RuntimeError # Platform.script points to dill file.
 io/test_extension_test: RuntimeError # Platform.script points to dill file.
 io/web_socket_compression_test: Pass, RuntimeError # Please triage.
+io/web_socket_ping_test: Pass, RuntimeError # Please triage.
 io/web_socket_protocol_test: Pass, RuntimeError # Please triage.
 io/web_socket_typed_data_test: Pass, RuntimeError # Please triage.
 no_lazy_dispatchers_test: SkipByDesign # KBC interpreter doesn't support --no_lazy_dispatchers
 package/scenarios/packages_file_strange_formatting/empty_lines_test: CompileTimeError # Please triage.
 package/scenarios/packages_file_strange_formatting/mixed_line_ends_test: CompileTimeError # Please triage.
 package/scenarios/packages_option_only/packages_option_only_test: CompileTimeError # Please triage.
-typed_data_isolate_test: Pass, Crash # Bytecode compilation.
+typed_data_isolate_test: Pass, Crash, Timeout # Please triage.
 
 [ $compiler == dartkp && $mode == debug && $runtime == dart_precompiled && $strong ]
 io/compile_all_test: Crash # Issue 32373