Skip to content

Commit

Permalink
[dart2wasm] Pass source maps to wasm-opt when optimizing
Browse files Browse the repository at this point in the history
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 <omersa@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
  • Loading branch information
osa1 authored and Commit Queue committed Aug 9, 2024
1 parent d71837a commit 1b1740e
Show file tree
Hide file tree
Showing 9 changed files with 202 additions and 117 deletions.
3 changes: 1 addition & 2 deletions pkg/dart2wasm/lib/code_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
10 changes: 10 additions & 0 deletions pkg/dart2wasm/tool/compile_benchmark
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -130,6 +131,11 @@ while [ $# -gt 0 ]; do
shift
;;

--no-source-maps)
GENERATE_SOURCE_MAP=0
shift
;;

-o)
shift
WASM_FILE="$1"
Expand Down Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions pkg/dartdev/lib/src/commands/compile.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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) {
Expand Down
15 changes: 14 additions & 1 deletion pkg/wasm_builder/lib/source_map.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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]!;

Expand Down
6 changes: 3 additions & 3 deletions tests/language/language_dart2wasm.status
Original file line number Diff line number Diff line change
Expand Up @@ -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.
101 changes: 101 additions & 0 deletions tests/web/wasm/source_map_simple_lib.dart
Original file line number Diff line number Diff line change
@@ -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]+)\)$');
30 changes: 30 additions & 0 deletions tests/web/wasm/source_map_simple_optimized_test.dart
Original file line number Diff line number Diff line change
@@ -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 (...)
*/
Loading

0 comments on commit 1b1740e

Please sign in to comment.