[dart2wasm] Pass source maps to wasm-opt when optimizing

To be able to know when we are generating a source map, make `dart
compile wasm` aware of the `--no-source-maps` flag.

The "name" segments of source mappings are also made `null` with this
patch. Browsers don't use that segment and binaryen doesn't support it.

Change-Id: I7b52c8fb7cef92ed60547e97ad137e0cd3967f26
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/378421
Commit-Queue: Ömer Ağacan <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>

Author: osa1

pkg/dart2wasm/lib/code_generator.dart

@@ -189,8 +189,7 @@ abstract class AstCodeGenerator
     final location = source.getLocation(fileUri, fileOffset);
     final old = _sourceMapFileOffset;
     _sourceMapFileOffset = fileOffset;
-    b.startSourceMapping(fileUri, location.line - 1, location.column - 1,
-        enclosingMember.name.text);
+    b.startSourceMapping(fileUri, location.line - 1, location.column - 1, null);
     return old;
   }
 

pkg/dart2wasm/tool/compile_benchmark

@@ -51,6 +51,7 @@ OPT_FLAGS_L4=($(find_flags 'optimizationLevel4Flags'))
 
 RUN_BINARYEN=1
 RUN_SRC=0
+GENERATE_SOURCE_MAP=1
 COMPILE_BENCHMARK_BASE_NAME=""
 PLATFORM_FILENAME="$BIN_DIR/dart2wasm_platform.dill"
 SNAPSHOT_NAME="dart2wasm"
@@ -130,6 +131,11 @@ while [ $# -gt 0 ]; do
       shift
       ;;
 
+    --no-source-maps)
+      GENERATE_SOURCE_MAP=0
+      shift
+      ;;
+
     -o)
       shift
       WASM_FILE="$1"
@@ -158,6 +164,10 @@ while [ $# -gt 0 ]; do
   esac
 done
 
+if [ $GENERATE_SOURCE_MAP -eq 1 ]; then
+  BINARYEN_FLAGS+=("-ism" "${WASM_FILE}.map" "-osm" "${WASM_FILE}.map")
+fi
+
 if [ -z "$DART_FILE" -o -z "$WASM_FILE" ]; then
   echo "Expected <file.dart> <file.wasm>"
   exit 1

pkg/dartdev/lib/src/commands/compile.dart

@@ -707,6 +707,11 @@ class CompileWasmCommand extends CompileSubcommandCommand {
         },
         hide: !verbose,
       )
+      ..addFlag(
+        'no-source-maps',
+        help: 'Do not generate a source map file.',
+        negatable: false,
+      )
       ..addOption(
         packagesOption.flag,
         abbr: packagesOption.abbr,
@@ -814,6 +819,7 @@ class CompileWasmCommand extends CompileSubcommandCommand {
       if (args.flag('print-wasm')) '--print-wasm',
       if (args.flag('print-kernel')) '--print-kernel',
       if (args.flag('enable-asserts')) '--enable-asserts',
+      if (args.flag('no-source-maps')) '--no-source-maps',
       for (final define in defines) '-D$define',
       if (maxPages != null) ...[
         '--import-shared-memory',
@@ -843,14 +849,26 @@ class CompileWasmCommand extends CompileSubcommandCommand {
     }
 
     final bool strip = args.flag('strip-wasm');
+    final bool generateSourceMap = !args.flag('no-source-maps');
 
     if (runWasmOpt) {
       final unoptFile = '$outputFileBasename.unopt.wasm';
       File(outputFile).renameSync(unoptFile);
 
+      final unoptSourceMapFile = '$outputFileBasename.unopt.wasm.map';
+      if (generateSourceMap) {
+        File('$outputFile.map').renameSync(unoptSourceMapFile);
+      }
+
       final flags = [
         ...binaryenFlags,
         if (!strip) '-g',
+        if (generateSourceMap) ...[
+          '-ism',
+          unoptSourceMapFile,
+          '-osm',
+          '$outputFile.map'
+        ]
       ];
 
       if (verbose) {

pkg/wasm_builder/lib/source_map.dart

@@ -126,13 +126,26 @@ String _serializeSourceMap(List<SourceMapping> mappings) {
   int lastSourceColumn = 0;
   int lastNameIndex = 0;
 
+  bool first = true;
+
   for (int i = 0; i < mappings.length; ++i) {
     final mapping = mappings[i];
+    final sourceInfo = mapping.sourceInfo;
+
+    if (sourceInfo == null && first) {
+      // Initial parts of the code will be unmapped my default, we don't need to
+      // explicitly unmap them. More importantly, current version of binaryen
+      // cannot handle single-segment mappings at the beginning of the mappings.
+      // We can remove this block of code after switching to a version with
+      // https://github.com/WebAssembly/binaryen/pull/6794.
+      continue;
+    }
+
+    first = false;
 
     lastTargetColumn =
         _encodeVLQ(mappingsStr, mapping.instructionOffset, lastTargetColumn);
 
-    final sourceInfo = mapping.sourceInfo;
     if (sourceInfo != null) {
       final sourceIndex = sourceIndices[sourceInfo.fileUri]!;
 

tests/language/language_dart2wasm.status

@@ -5,6 +5,6 @@
 
 [ $compiler == dart2wasm ]
 inference_update_2/why_not_promoted_external_error_test: SkipByDesign # Non-JS-interop external members are not supported
-number/separators_test: SkipByDesign # WASM has real integers.
-number/web_int_literals_test: SkipByDesign # WASM has real integers.
-vm/*: SkipByDesign # Tests for the VM.
+number/separators_test: SkipByDesign # Wasm has real integers.
+number/web_int_literals_test: SkipByDesign # Wasm has real integers.
+vm/*: SkipByDesign # Tests for the VM.

tests/web/wasm/source_map_simple_lib.dart

@@ -0,0 +1,101 @@
+// Copyright (c) 2024, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:js_interop';
+import 'dart:typed_data';
+import 'dart:convert';
+
+import 'package:source_maps/parser.dart';
+
+void f() {
+  g();
+}
+
+void g() {
+  throw 'hi';
+}
+
+runtimeFalse() => int.parse('1') == 0;
+
+// `frameDetails` is (line, column) of the frames we check.
+//
+// Information we don't check are "null": we don't want to check line/column
+// of standard library functions to avoid breaking the test with unrelated
+// changes to the standard library.
+void testMain(String testName, List<(int?, int?)?> frameDetails) {
+  // Use `f` and `g` in a few places to make sure wasm-opt won't inline them
+  // in the test.
+  final fTearOff = f;
+  final gTearOff = g;
+
+  if (runtimeFalse()) f();
+  if (runtimeFalse()) g();
+
+  // Read source map of the current program.
+  final compilationDir = const String.fromEnvironment("TEST_COMPILATION_DIR");
+  final sourceMapFileContents =
+      readfile('$compilationDir/${testName}_test.wasm.map');
+  final mapping = parse(utf8.decode(sourceMapFileContents)) as SingleMapping;
+
+  // Get some simple stack trace.
+  String? stackTraceString;
+  try {
+    f();
+  } catch (e, st) {
+    stackTraceString = st.toString();
+  }
+
+  // Print the stack trace to make it easy to update the test.
+  print("-----");
+  print(stackTraceString);
+  print("-----");
+
+  final stackTraceLines = stackTraceString!.split('\n');
+
+  for (int frameIdx = 0; frameIdx < frameDetails.length; frameIdx += 1) {
+    final line = stackTraceLines[frameIdx];
+    final hexOffsetMatch = stackTraceHexOffsetRegExp.firstMatch(line);
+    if (hexOffsetMatch == null) {
+      throw "Unable to parse hex offset from stack frame $frameIdx";
+    }
+    final hexOffsetStr = hexOffsetMatch.group(1)!; // includes '0x'
+    final offset = int.tryParse(hexOffsetStr);
+    if (offset == null) {
+      throw "Unable to parse hex number in frame $frameIdx: $hexOffsetStr";
+    }
+    final span = mapping.spanFor(0, offset);
+    final frameInfo = frameDetails[frameIdx];
+    if (frameInfo == null) {
+      if (span != null) {
+        throw "Stack frame $frameIdx should not have a source span, but it is mapped: $span";
+      }
+      continue;
+    }
+    if (span == null) {
+      print("Stack frame $frameIdx does not have source mapping");
+    } else {
+      if (frameInfo.$1 != null) {
+        if (span.start.line + 1 != frameInfo.$1) {
+          throw "Stack frame $frameIdx is expected to have line ${frameInfo.$1}, but it has line ${span.start.line + 1}";
+        }
+      }
+      if (frameInfo.$2 != null) {
+        if (span.start.column + 1 != frameInfo.$2) {
+          throw "Stack frame $frameIdx is expected to have column ${frameInfo.$2}, but it has column ${span.start.column + 1}";
+        }
+      }
+    }
+  }
+}
+
+/// Read the file at the given [path].
+///
+/// This relies on the `readbuffer` function provided by d8.
+@JS()
+external JSArrayBuffer readbuffer(JSString path);
+
+/// Read the file at the given [path].
+Uint8List readfile(String path) => Uint8List.view(readbuffer(path.toJS).toDart);
+
+final stackTraceHexOffsetRegExp = RegExp(r'wasm-function.*(0x[0-9a-fA-F]+)\)$');

tests/web/wasm/source_map_simple_optimized_test.dart

@@ -0,0 +1,30 @@
+// Copyright (c) 2024, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// dart2wasmOptions=-O4 --no-strip-wasm --extra-compiler-option=-DTEST_COMPILATION_DIR=$TEST_COMPILATION_DIR
+
+import 'source_map_simple_lib.dart' as Lib;
+
+void main() {
+  Lib.testMain('source_map_simple_optimized', frameDetails);
+}
+
+const List<(int?, int?)?> frameDetails = [
+  (null, null), // _throwWithCurrentStackTrace
+  (16, 3), // g
+  (12, 3), // f
+  (44, 5), // testMain, inlined in main
+  (null, null), // _invokeMain
+];
+
+/*
+at Error._throwWithCurrentStackTrace (wasm://wasm/0008d08e:wasm-function[115]:0xc095)
+at g (wasm://wasm/0008d08e:wasm-function[359]:0x11e15)
+at f (wasm://wasm/0008d08e:wasm-function[358]:0x11e0b)
+at main (wasm://wasm/0008d08e:wasm-function[357]:0x11913)
+at _invokeMain (wasm://wasm/0008d08e:wasm-function[82]:0xb349)
+at Module.invoke (...)
+at main (...)
+at async action (...)
+*/