[dart2wasm] Add option to verify type check implementations

This adds a `--verify-type-checks` option to dart2wasm to instrument
the code such that whenever we are able to generate specialized code
for a type check, we generate both the specialized code and also call
the general fallback path, then compare the results.

This can be used to expose bugs in the type check specializations, or
in the reference implementation, as it may be.

Change-Id: I081540a8eedc7d029b332919283810220b21b3ea
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/336023
Reviewed-by: Ömer Ağacan <omersa@google.com>

pkg/dart2wasm/README.md

@@ -28,17 +28,18 @@ where *options* include:
 | `--dart-sdk=`*path*                     | relative to script | The location of the `sdk` directory inside the Dart SDK, containing the core library sources.
 | `--platform=`*path*                     | none    | The location of the platform `dill` file containing the compiled core libraries.
 | `--depfile=`*path*                      | none    | Write a Ninja depfile listing the input sources for the compilation.
+| `--`[`no-`]`enable-asserts`             | no      | Enable assertions at runtime.
 | `--`[`no-`]`export-all`                 | no      | Export all functions; otherwise, just export `main`.
 | `--`[`no-`]`import-shared-memory`       | no      | Import a shared memory buffer. If this is on, `--shared-memory-max-pages` must also be specified.
 | `--`[`no-`]`inlining`                   | yes     | Enable function inlining.
 | `--inlining-limit` *size*               | 0       | Always inline functions no larger than this number of AST nodes, if inlining is enabled.
+| `--`[`no-`]`js-compatibility`           | no      | Enable JS compatibility mode.
 | `--`[`no-`]`name-section`               | yes     | Emit Name Section with function names.
 | `--`[`no-`]`omit-type-checks`           | no      | Omit runtime type checks, such as covariance checks and downcasts.
 | `--`[`no-`]`polymorphic-specialization` | no      | Do virtual calls by switching on the class ID instead of using `call_indirect`.
 | `--`[`no-`]`print-kernel`               | no      | Print IR for each function before compiling it.
 | `--`[`no-`]`print-wasm`                 | no      | Print Wasm instructions of each compiled function.
-| `--`[`no-`]`enable-asserts`             | no      | Enable assertions at runtime.
-| `--`[`no-`]`js-compatibility`           | no      | Enable JS compatibility mode.
+| `--`[`no-`]`verify-type-checks`         | no      | Instrument code to verify that type check optimizations produce the correct result.
 | `--shared-memory-max-pages` *pagecount* |         | Max size of the imported memory buffer. If `--shared-import-memory` is specified, this must also be specified.
 | `--watch` *offset*                      |         | Print stack trace leading to the byte at offset *offset* in the `.wasm` output file. Can be specified multiple times.
 

pkg/dart2wasm/lib/async.dart

@@ -1065,8 +1065,8 @@ class AsyncCodeGenerator extends CodeGenerator {
       if (emitGuard) {
         _getCurrentException();
         b.ref_as_non_null();
-        types.emitTypeTest(
-            this, catch_.guard, translator.coreTypes.objectNonNullableRawType);
+        types.emitTypeCheck(this, catch_.guard,
+            translator.coreTypes.objectNonNullableRawType, catch_);
         b.i32_eqz();
         // When generating guards we can't generate the catch body inside the
         // `if` block for the guard as the catch body can have suspension

pkg/dart2wasm/lib/code_generator.dart

@@ -1161,8 +1161,8 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
       // Only emit the type test if the guard is not [Object].
       if (emitGuard) {
         b.local_get(thrownException);
-        types.emitTypeTest(
-            this, guard, translator.coreTypes.objectNonNullableRawType);
+        types.emitTypeCheck(
+            this, guard, translator.coreTypes.objectNonNullableRawType, catch_);
         b.i32_eqz();
         b.br_if(catchBlock);
       }
@@ -2988,7 +2988,7 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
   @override
   w.ValueType visitIsExpression(IsExpression node, w.ValueType expectedType) {
     wrap(node.operand, translator.topInfo.nullableType);
-    types.emitTypeTest(this, node.type, dartTypeOf(node.operand));
+    types.emitTypeCheck(this, node.type, dartTypeOf(node.operand), node);
     return w.NumType.i32;
   }
 
@@ -3005,7 +3005,7 @@ class CodeGenerator extends ExpressionVisitor1<w.ValueType, w.ValueType>
 
     // We lower an `as` expression to a type test, throwing a [TypeError] if
     // the type test fails.
-    types.emitTypeTest(this, node.type, dartTypeOf(node.operand));
+    types.emitTypeCheck(this, node.type, dartTypeOf(node.operand), node);
     b.br_if(asCheckBlock);
     b.local_get(operand);
     types.makeType(this, node.type);

pkg/dart2wasm/lib/dart2wasm.dart

@@ -43,6 +43,9 @@ final List<Option> options = [
   Flag("omit-type-checks",
       (o, value) => o.translatorOptions.omitTypeChecks = value,
       defaultsTo: _d.translatorOptions.omitTypeChecks),
+  Flag("verify-type-checks",
+      (o, value) => o.translatorOptions.verifyTypeChecks = value,
+      defaultsTo: _d.translatorOptions.verifyTypeChecks),
   IntOption(
       "inlining-limit", (o, value) => o.translatorOptions.inliningLimit = value,
       defaultsTo: "${_d.translatorOptions.inliningLimit}"),

pkg/dart2wasm/lib/kernel_nodes.dart

@@ -249,6 +249,8 @@ mixin KernelNodes {
       index.getTopLevelProcedure("dart:core", "_isSubtype");
   late final Procedure isTypeSubtype =
       index.getTopLevelProcedure("dart:core", "_isTypeSubtype");
+  late final Procedure verifyOptimizedTypeCheck =
+      index.getTopLevelProcedure("dart:core", "_verifyOptimizedTypeCheck");
   late final Procedure checkClosureShape =
       index.getTopLevelProcedure("dart:core", "_checkClosureShape");
   late final Procedure checkClosureType =

pkg/dart2wasm/lib/translator.dart

@@ -32,15 +32,13 @@ class TranslatorOptions {
   bool exportAll = false;
   bool importSharedMemory = false;
   bool inlining = true;
+  bool jsCompatibility = false;
   bool nameSection = true;
   bool omitTypeChecks = false;
   bool polymorphicSpecialization = false;
   bool printKernel = false;
   bool printWasm = false;
-  // If the default value for [useStringref] is changed, also update the
-  // `sdk/bin/dart2wasm` script.
-  bool useStringref = false;
-  bool jsCompatibility = false;
+  bool verifyTypeChecks = false;
   int inliningLimit = 0;
   int? sharedMemoryMaxPages;
   List<int> watchPoints = [];

pkg/dart2wasm/lib/types.dart

@@ -627,37 +627,47 @@ class Types {
     return functionTypeParameterIndex[type]!;
   }
 
-  /// Test value against a Dart type. Expects the value on the stack as a
-  /// (ref null #Top) and leaves the result on the stack as an i32.
-  /// TODO(joshualitt): Remove dependency on [CodeGenerator]
-  void emitTypeTest(
-      CodeGenerator codeGen, DartType type, DartType operandType) {
+  /// Emit code for testing a value against a Dart type. Expects the value on
+  /// the stack as a (ref null #Top) and leaves the result on the stack as an
+  /// i32.
+  void emitTypeCheck(CodeGenerator codeGen, DartType type, DartType operandType,
+      [TreeNode? node]) {
     final b = codeGen.b;
-    if (type is! InterfaceType) {
+    w.Local? operandTemp;
+    if (translator.options.verifyTypeChecks) {
+      operandTemp = codeGen.addLocal(translator.topInfo.nullableType);
+      b.local_tee(operandTemp);
+    }
+    if (_emitOptimizedTypeCheck(codeGen, type, operandType)) {
+      if (translator.options.verifyTypeChecks) {
+        b.local_get(operandTemp!);
+        makeType(codeGen, type);
+        if (node != null && node.location != null) {
+          w.FunctionType verifyFunctionType = translator.functions
+              .getFunctionType(translator.verifyOptimizedTypeCheck.reference);
+          String location = node.location.toString();
+          translator.constants.instantiateConstant(codeGen.function, b,
+              StringConstant(location), verifyFunctionType.inputs.last);
+        } else {
+          b.ref_null(w.HeapType.none);
+        }
+        codeGen.call(translator.verifyOptimizedTypeCheck.reference);
+      }
+    } else {
+      // General fallback path
       makeType(codeGen, type);
       codeGen.call(translator.isSubtype.reference);
-      return;
-    }
-    bool isPotentiallyNullable = operandType.isPotentiallyNullable;
-    w.Label? resultLabel;
-    if (isPotentiallyNullable) {
-      // Store operand in a temporary variable, since Binaryen does not support
-      // block inputs.
-      w.Local operand = codeGen.addLocal(translator.topInfo.nullableType);
-      b.local_set(operand);
-      resultLabel = b.block(const [], const [w.NumType.i32]);
-      w.Label nullLabel = b.block(const [], const []);
-      b.local_get(operand);
-      b.br_on_null(nullLabel);
-    }
-    void _endPotentiallyNullableBlock() {
-      if (isPotentiallyNullable) {
-        b.br(resultLabel!);
-        b.end(); // nullLabel
-        b.i32_const(encodedNullability(type));
-        b.end(); // resultLabel
-      }
     }
+  }
+
+  /// Emit optimized code for testing a value against a Dart type. If the type
+  /// to be tested against is of a shape where we can generate more efficient
+  /// code than the general fallback path, generate such code and return `true`.
+  /// Otherwise, return `false` to indicate that the general path should be
+  /// taken.
+  bool _emitOptimizedTypeCheck(
+      CodeGenerator codeGen, DartType type, DartType operandType) {
+    if (type is! InterfaceType) return false;
 
     if (type.typeArguments.any((t) => t is! DynamicType)) {
       // Type has at least one type argument that is not `dynamic`.
@@ -677,12 +687,24 @@ class Types {
           operandType.typeArguments.length == type.typeArguments.length;
 
       if (!(sameNumTypeParams && base == operandType)) {
-        makeType(codeGen, type);
-        codeGen.call(translator.isSubtype.reference);
-        _endPotentiallyNullableBlock();
-        return;
+        return false;
       }
     }
+
+    final b = codeGen.b;
+    bool isPotentiallyNullable = operandType.isPotentiallyNullable;
+    w.Label? resultLabel;
+    if (isPotentiallyNullable) {
+      // Store operand in a temporary variable, since Binaryen does not support
+      // block inputs.
+      w.Local operand = codeGen.addLocal(translator.topInfo.nullableType);
+      b.local_set(operand);
+      resultLabel = b.block(const [], const [w.NumType.i32]);
+      w.Label nullLabel = b.block(const [], const []);
+      b.local_get(operand);
+      b.br_on_null(nullLabel);
+    }
+
     List<Class> concrete = _getConcreteSubtypes(type.classNode).toList();
     if (type.classNode == coreTypes.objectClass) {
       b.drop();
@@ -714,7 +736,15 @@ class Types {
       b.i32_const(0);
       b.end(); // done
     }
-    _endPotentiallyNullableBlock();
+
+    if (isPotentiallyNullable) {
+      b.br(resultLabel!);
+      b.end(); // nullLabel
+      b.i32_const(encodedNullability(type));
+      b.end(); // resultLabel
+    }
+
+    return true;
   }
 
   int encodedNullability(DartType type) =>

sdk/lib/_internal/wasm/lib/type.dart

@@ -994,9 +994,9 @@ class _TypeUniverse {
 _TypeUniverse _typeUniverse = _TypeUniverse.create();
 
 @pragma("wasm:entry-point")
-bool _isSubtype(Object? s, _Type t) {
+bool _isSubtype(Object? o, _Type t) {
   return _typeUniverse.isSubtype(
-      _getActualRuntimeTypeNullable(s), null, t, null);
+      _getActualRuntimeTypeNullable(o), null, t, null);
 }
 
 @pragma("wasm:entry-point")
@@ -1004,6 +1004,35 @@ bool _isTypeSubtype(_Type s, _Type t) {
   return _typeUniverse.isSubtype(s, null, t, null);
 }
 
+@pragma("wasm:entry-point")
+bool _verifyOptimizedTypeCheck(
+    bool result, Object? o, _Type t, String? location) {
+  _Type s = _getActualRuntimeTypeNullable(o);
+  bool reference = _isTypeSubtype(s, t);
+  if (result != reference) {
+    throw _TypeCheckVerificationError(s, t, result, reference, location);
+  }
+  return result;
+}
+
+class _TypeCheckVerificationError extends Error {
+  final _Type left;
+  final _Type right;
+  final bool optimized;
+  final bool reference;
+  final String? location;
+
+  _TypeCheckVerificationError(
+      this.left, this.right, this.optimized, this.reference, this.location);
+
+  String toString() {
+    String locationString = location != null ? " at $location" : "";
+    return "Type check verification error$locationString\n"
+        "Checking $left <: $right\n"
+        "Optimized result $optimized, reference result $reference\n";
+  }
+}
+
 /// Checks that argument lists have expected number of arguments for the
 /// closure.
 ///