From 5e5313c8d690b71d89c044ac5ee40bd73a680868 Mon Sep 17 00:00:00 2001
From: Alexander Markov <alexmarkov@google.com>
Date: Wed, 21 Aug 2019 02:28:44 +0000
Subject: [PATCH] [vm/bytecode] Keep unreachable code for code coverage
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This change adds a bytecode generation option 'keep-unreachable-code' to
avoid eliminating unreachable code from constant conditions.
This option is enabled in kernel service.

If unreachable code is eliminated, then call instructions are not generated
and ICData objects are not created. Source code report is collected
using created ICData objects and will not flag the unreachable code as
not executed.

Change-Id: Ic73e19c653505e0dfab4648ab65ef6b90370e0b4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/113960
Reviewed-by: Régis Crelier <regis@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
---
 pkg/vm/bin/kernel_service.dart        | 34 +++++++++++++++------------
 pkg/vm/lib/bytecode/gen_bytecode.dart |  3 +++
 pkg/vm/lib/bytecode/options.dart      |  7 ++++++
 3 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/pkg/vm/bin/kernel_service.dart b/pkg/vm/bin/kernel_service.dart
index 2b0f8244dff..5b5eaa9a63c 100644
--- a/pkg/vm/bin/kernel_service.dart
+++ b/pkg/vm/bin/kernel_service.dart
@@ -158,26 +158,30 @@ abstract class Compiler {
       if (options.bytecode && errors.isEmpty) {
         await runWithFrontEndCompilerContext(script, options, component, () {
           // TODO(alexmarkov): disable source positions, local variables info,
-          //  debugger stops and source files in VM PRODUCT mode.
+          //  debugger stops, source files and unreachable code in VM PRODUCT
+          //  mode.
           // TODO(rmacnak): disable annotations if mirrors are not enabled.
           generateBytecode(component,
               coreTypes: compilerResult.coreTypes,
               hierarchy: compilerResult.classHierarchy,
               options: new BytecodeOptions(
-                  enableAsserts: enableAsserts,
-                  environmentDefines: options.environmentDefines,
-                  // Needed both for stack traces and the debugger.
-                  emitSourcePositions: true,
-                  // Only needed when the debugger is available.
-                  emitLocalVarInfo: true,
-                  // Only needed when the debugger is available.
-                  emitDebuggerStops: true,
-                  // Only needed when the VM service is available.
-                  emitSourceFiles: true,
-                  // Only needed when reload is available.
-                  emitInstanceFieldInitializers: true,
-                  // Only needed when mirrors are available.
-                  emitAnnotations: true));
+                enableAsserts: enableAsserts,
+                environmentDefines: options.environmentDefines,
+                // Needed both for stack traces and the debugger.
+                emitSourcePositions: true,
+                // Only needed when the debugger is available.
+                emitLocalVarInfo: true,
+                // Only needed when the debugger is available.
+                emitDebuggerStops: true,
+                // Only needed when the VM service is available.
+                emitSourceFiles: true,
+                // Only needed when reload is available.
+                emitInstanceFieldInitializers: true,
+                // Only needed when mirrors are available.
+                emitAnnotations: true,
+                // Only needed when observatory (source report) is available.
+                keepUnreachableCode: true,
+              ));
           component = createFreshComponentWithBytecode(component);
         });
       }
diff --git a/pkg/vm/lib/bytecode/gen_bytecode.dart b/pkg/vm/lib/bytecode/gen_bytecode.dart
index 4c38ad94c19..9401255df85 100644
--- a/pkg/vm/lib/bytecode/gen_bytecode.dart
+++ b/pkg/vm/lib/bytecode/gen_bytecode.dart
@@ -1290,6 +1290,9 @@ class BytecodeGenerator extends RecursiveVisitor<Null> {
   /// Returns value of the given expression if it is a bool constant.
   /// Otherwise, returns `null`.
   bool _constantConditionValue(Expression condition) {
+    if (options.keepUnreachableCode) {
+      return null;
+    }
     // TODO(dartbug.com/34585): use constant evaluator to evaluate
     // expressions in a non-constant context.
     if (condition is Not) {
diff --git a/pkg/vm/lib/bytecode/options.dart b/pkg/vm/lib/bytecode/options.dart
index 1772143a211..4546ef02670 100644
--- a/pkg/vm/lib/bytecode/options.dart
+++ b/pkg/vm/lib/bytecode/options.dart
@@ -13,6 +13,8 @@ class BytecodeOptions {
     'show-bytecode-size-stat': 'Show bytecode size breakdown',
     'source-positions': 'Emit source positions',
     'instance-field-initializers': 'Emit separate instance field initializers',
+    'keep-unreachable-code':
+        'Do not remove unreachable code (useful if collecting code coverage)',
   };
 
   bool enableAsserts;
@@ -24,6 +26,7 @@ class BytecodeOptions {
   bool emitAnnotations;
   bool emitInstanceFieldInitializers;
   bool omitAssertSourcePositions;
+  bool keepUnreachableCode;
   bool showBytecodeSizeStatistics;
   Map<String, String> environmentDefines;
 
@@ -37,6 +40,7 @@ class BytecodeOptions {
       this.emitAnnotations = false,
       this.emitInstanceFieldInitializers = false,
       this.omitAssertSourcePositions = false,
+      this.keepUnreachableCode = false,
       this.showBytecodeSizeStatistics = false,
       this.environmentDefines = const <String, String>{}}) {
     causalAsyncStacks ??=
@@ -64,6 +68,9 @@ class BytecodeOptions {
         case 'instance-field-initializers':
           emitInstanceFieldInitializers = true;
           break;
+        case 'keep-unreachable-code':
+          keepUnreachableCode = true;
+          break;
         case 'show-bytecode-size-stat':
           showBytecodeSizeStatistics = true;
           break;