From 6414db5327ee9a7be967fa213e3b49f7ebbf592c Mon Sep 17 00:00:00 2001
From: Mayank Patke <fishythefish@google.com>
Date: Tue, 7 Mar 2023 19:36:10 +0000
Subject: [PATCH] [dart2js] Disable kernel transformer when using modular
 analysis.

Mutating the kernel AST can cause serialization and deserialization to
go out of sync when modular analysis is enabled, crashing dart2js.

Change-Id: If19afabacf1cb120a6804bd12ef268316de41f1b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/286862
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Nate Biggs <natebiggs@google.com>
Commit-Queue: Mayank Patke <fishythefish@google.com>
---
 pkg/compiler/lib/src/compiler.dart | 88 +++++++++++++++++-------------
 1 file changed, 49 insertions(+), 39 deletions(-)

diff --git a/pkg/compiler/lib/src/compiler.dart b/pkg/compiler/lib/src/compiler.dart
index 1782649c1aa..57e7b91fdd0 100644
--- a/pkg/compiler/lib/src/compiler.dart
+++ b/pkg/compiler/lib/src/compiler.dart
@@ -397,7 +397,7 @@ class Compiler {
   }
 
   Future<load_kernel.Output?> produceKernel() async {
-    if (options.readClosedWorldUri == null) {
+    if (shouldComputeClosedWorld) {
       load_kernel.Output? output = await loadKernel();
       if (output == null || compilationFailed) return null;
       ir.Component component = output.component;
@@ -445,42 +445,37 @@ class Compiler {
   bool shouldStopAfterLoadKernel(load_kernel.Output? output) =>
       output == null || compilationFailed || options.cfeOnly;
 
-  void simplifyConstConditionals(load_kernel.Output output) {
-    if (options.readClosedWorldUri == null) {
-      void reportMessage(
-          fe.LocatedMessage message, List<fe.LocatedMessage>? context) {
-        reportLocatedMessage(reporter, message, context);
-      }
-
-      bool shouldNotInline(ir.TreeNode node) {
-        if (node is! ir.Annotatable) {
-          return false;
-        }
-        return computePragmaAnnotationDataFromIr(node).any((pragma) =>
-            pragma == const PragmaAnnotationData('noInline') ||
-            pragma == const PragmaAnnotationData('never-inline'));
-      }
-
-      // No existing closed world means we're in phase 1, so run the
-      // transformer.
-      fe.ConstConditionalSimplifier(
-              const Dart2jsDartLibrarySupport(),
-              const Dart2jsConstantsBackend(
-                  supportsUnevaluatedConstants: false),
-              output.component,
-              reportMessage,
-              environmentDefines: environment.definitions,
-              evaluationMode: options.useLegacySubtyping
-                  ? fe.EvaluationMode.weak
-                  : fe.EvaluationMode.strong,
-              shouldNotInline: shouldNotInline)
-          .run();
+  void simplifyConstConditionals(ir.Component component) {
+    void reportMessage(
+        fe.LocatedMessage message, List<fe.LocatedMessage>? context) {
+      reportLocatedMessage(reporter, message, context);
     }
 
-    // If the closed world is deserialized instead, then the input .dill should
-    // already have the modified AST.
+    bool shouldNotInline(ir.TreeNode node) {
+      if (node is! ir.Annotatable) {
+        return false;
+      }
+      return computePragmaAnnotationDataFromIr(node).any((pragma) =>
+          pragma == const PragmaAnnotationData('noInline') ||
+          pragma == const PragmaAnnotationData('never-inline'));
+    }
+
+    fe.ConstConditionalSimplifier(
+            const Dart2jsDartLibrarySupport(),
+            const Dart2jsConstantsBackend(supportsUnevaluatedConstants: false),
+            component,
+            reportMessage,
+            environmentDefines: environment.definitions,
+            evaluationMode: options.useLegacySubtyping
+                ? fe.EvaluationMode.weak
+                : fe.EvaluationMode.strong,
+            shouldNotInline: shouldNotInline)
+        .run();
   }
 
+  bool get usingModularAnalysis =>
+      options.modularMode || options.hasModularAnalysisInputs;
+
   Future<ModuleData> runModularAnalysis(
       load_kernel.Output output, Set<Uri> moduleLibraries) async {
     ir.Component component = output.component;
@@ -600,11 +595,28 @@ class Compiler {
         globalTypeInferenceResultsData);
   }
 
+  bool get shouldComputeClosedWorld => options.readClosedWorldUri == null;
+
   Future<DataAndIndices<JClosedWorld>?> produceClosedWorld(
       load_kernel.Output output, ModuleData? moduleData) async {
     ir.Component component = output.component;
     DataAndIndices<JClosedWorld> closedWorldAndIndices;
-    if (options.readClosedWorldUri == null) {
+    if (shouldComputeClosedWorld) {
+      if (!usingModularAnalysis) {
+        // If we're deserializing the closed world, the input .dill already
+        // contains the modified AST, so the transformer only needs to run if
+        // the closed world is being computed from scratch.
+        //
+        // However, the transformer is not currently compatible with modular
+        // analysis. When modular analysis is enabled in Blaze, some aspects run
+        // before this phase of the compiler. This can cause dart2js to crash if
+        // the kernel AST is mutated, since we will attempt to serialize and
+        // deserialize against different ASTs.
+        //
+        // TODO(fishythefish): Make this compatible with modular analysis.
+        simplifyConstConditionals(component);
+      }
+
       Uri rootLibraryUri = output.rootLibraryUri!;
       List<Uri> libraries = output.libraries!;
       final closedWorld =
@@ -724,19 +736,17 @@ class Compiler {
     final output = await produceKernel();
     if (shouldStopAfterLoadKernel(output)) return;
 
-    simplifyConstConditionals(output!);
-
     // Run modular analysis. This may be null if modular analysis was not
     // requested for this pipeline.
     ModuleData? moduleData;
-    if (options.modularMode || options.hasModularAnalysisInputs) {
-      moduleData = await produceModuleData(output);
+    if (usingModularAnalysis) {
+      moduleData = await produceModuleData(output!);
     }
     if (shouldStopAfterModularAnalysis) return;
 
     // Compute closed world.
     DataAndIndices<JClosedWorld>? closedWorldAndIndices =
-        await produceClosedWorld(output, moduleData);
+        await produceClosedWorld(output!, moduleData);
     if (shouldStopAfterClosedWorld(closedWorldAndIndices)) return;
 
     // Run global analysis.