[vm/kernel/bytecode] Fix serialization of closures with type arguments

Before this change, ConstantClosureFunction was holding a clone of original
FunctionNode without body (in order to omit body during serialization).
It is not correct: original FunctionNode, which is used to generate
closure's bytecode may reference original type parameters and those
type parameters could be used in types written into constant pool.
A cloned FunctionNode has its own cloned copies of type parameters
and it does not provide a sufficient serialization/deserialization context.
This resulted in 'Type parameter T is not indexed' errors.

This CL fixes this problem by removing cloning. In order to avoid
serialization of the FunctionNode body, body is temporarily dropped
while writing ConstantClosureFunction.

Also, this CL fixes type finalization of signature types of closure functions
read from bytecode. If signature type is not finalized, VM crashes with
'../../runtime/vm/object.cc: 18580: error: expected: IsFinalized()'.

Change-Id: Id88e99609ee387352fc011a8c297024d05e24037
Reviewed-on: https://dart-review.googlesource.com/67960
Reviewed-by: Régis Crelier <regis@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>

pkg/vm/lib/bytecode/constant_pool.dart

@@ -984,22 +984,34 @@ class ConstantClosureFunction extends ConstantPoolEntry {
 
   @override
   void writeValueToBinary(BinarySink sink) {
-    assert(function.body == null);
     sink.writeStringReference(name);
-    sink.writeNode(function);
+    _withoutFunctionBody(() {
+      sink.writeNode(function);
+    });
   }
 
   ConstantClosureFunction.readFromBinary(BinarySource source)
       : name = source.readStringReference(),
-        function = source.readFunctionNode();
+        function = source.readFunctionNode() {
+    assert(function.body == null);
+  }
 
   @override
   String toString() {
     StringBuffer buffer = new StringBuffer();
-    new Printer(buffer).writeFunction(function);
+    _withoutFunctionBody(() {
+      new Printer(buffer).writeFunction(function);
+    });
     return 'ClosureFunction $name ${buffer.toString().trim()}';
   }
 
+  _withoutFunctionBody(action()) {
+    final savedBody = function.body;
+    function.body = null;
+    action();
+    function.body = savedBody;
+  }
+
   // ConstantClosureFunction entries are created per closure and should not
   // be merged, so ConstantClosureFunction class uses identity [hashCode] and
   // [operator ==].

pkg/vm/lib/bytecode/gen_bytecode.dart

@@ -6,7 +6,6 @@ library vm.bytecode.gen_bytecode;
 
 import 'package:kernel/ast.dart' hide MapEntry;
 import 'package:kernel/class_hierarchy.dart' show ClassHierarchy;
-import 'package:kernel/clone.dart';
 import 'package:kernel/core_types.dart' show CoreTypes;
 import 'package:kernel/external_name.dart' show getExternalName;
 import 'package:kernel/library_index.dart' show LibraryIndex;
@@ -818,8 +817,8 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
     List<Label> savedYieldPoints = yieldPoints;
     yieldPoints = locals.isSyncYieldingFrame ? <Label>[] : null;
 
-    final int closureFunctionIndex = cp.add(new ConstantClosureFunction(
-        name, new CloneWithoutBody().visitFunctionNode(function)));
+    final int closureFunctionIndex =
+        cp.add(new ConstantClosureFunction(name, function));
 
     _genPrologue(node, function);
 

runtime/vm/compiler/frontend/bytecode_reader.cc

@@ -5,6 +5,7 @@
 #include "vm/compiler/frontend/bytecode_reader.h"
 
 #include "vm/bootstrap.h"
+#include "vm/class_finalizer.h"
 #include "vm/code_descriptors.h"
 #include "vm/compiler/assembler/disassembler_kbc.h"
 #include "vm/constants_kbc.h"
@@ -424,6 +425,13 @@ intptr_t BytecodeMetadataHelper::ReadPoolEntries(const Function& function,
         // The closure has no body.
         function_node_helper.ReadUntilExcluding(FunctionNodeHelper::kEnd);
 
+        // Finalize function type.
+        Type& signature_type =
+            Type::Handle(helper_->zone_, closure.SignatureType());
+        signature_type ^= ClassFinalizer::FinalizeType(*(active_class_->klass),
+                                                       signature_type);
+        closure.SetSignatureType(signature_type);
+
         pool.SetTypeAt(i, ObjectPool::kTaggedObject);
         pool.SetObjectAt(i, closure);