From 8f049464d7d2f2a024a08c52eb8bfeb1dd205010 Mon Sep 17 00:00:00 2001 From: chen ruixiang Date: Wed, 22 Feb 2023 12:05:47 +0800 Subject: [PATCH 1/3] fix: sourcemap generation for call instruction --- src/wasm-stack.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wasm-stack.h b/src/wasm-stack.h index 8f72c13844c..62fe053f42e 100644 --- a/src/wasm-stack.h +++ b/src/wasm-stack.h @@ -232,7 +232,6 @@ void BinaryenIRWriter::visitPossibleBlockContents(Expression* curr) { template void BinaryenIRWriter::visit(Expression* curr) { - emitDebugLocation(curr); // We emit unreachable instructions that create unreachability, but not // unreachable instructions that just inherit unreachability from their // children, since the latter won't be reached. This (together with logic in @@ -257,6 +256,7 @@ void BinaryenIRWriter::visit(Expression* curr) { if (Properties::isControlFlowStructure(curr)) { Visitor::visit(curr); } else { + emitDebugLocation(curr); emit(curr); } } From 904694af23be95b9a83d35e69ec021b414e6a9f9 Mon Sep 17 00:00:00 2001 From: chen ruixiang Date: Wed, 22 Feb 2023 12:41:52 +0800 Subject: [PATCH 2/3] fix: optimize border check of source map --- src/wasm-binary.h | 2 +- src/wasm/wasm-binary.cpp | 29 ++++++++++++++++++----------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/wasm-binary.h b/src/wasm-binary.h index 4d9f4f5aed9..49e1403a1a5 100644 --- a/src/wasm-binary.h +++ b/src/wasm-binary.h @@ -1430,7 +1430,7 @@ class WasmBinaryBuilder { MixedArena& allocator; const std::vector& input; std::istream* sourceMap; - std::pair nextDebugLocation; + std::tuple nextDebugLocation; // available pos, previous pos, next debug location bool debugInfo = true; bool DWARF = false; bool skipFunctionBodies = false; diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index b616b5a31d2..a3c4b7b615f 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -1573,7 +1573,7 @@ WasmBinaryBuilder::WasmBinaryBuilder(Module& wasm, FeatureSet features, const std::vector& input) : wasm(wasm), allocator(wasm.allocator), input(input), sourceMap(nullptr), - nextDebugLocation(0, {0, 0, 0}), debugLocation() { + nextDebugLocation(0, 0, {0, 0, 0}), debugLocation() { wasm.features = features; } @@ -2741,7 +2741,7 @@ void WasmBinaryBuilder::readSourceMapHeader() { mustReadChar('\"'); if (maybeReadChar('\"')) { // empty mappings - nextDebugLocation.first = 0; + std::get<0>(nextDebugLocation) = 0; return; } // read first debug location @@ -2750,7 +2750,7 @@ void WasmBinaryBuilder::readSourceMapHeader() { uint32_t lineNumber = readBase64VLQ(*sourceMap) + 1; // adjust zero-based line number uint32_t columnNumber = readBase64VLQ(*sourceMap); - nextDebugLocation = {position, {fileIndex, lineNumber, columnNumber}}; + nextDebugLocation = {position, position, {fileIndex, lineNumber, columnNumber}}; } void WasmBinaryBuilder::readNextDebugLocation() { @@ -2758,17 +2758,24 @@ void WasmBinaryBuilder::readNextDebugLocation() { return; } - while (nextDebugLocation.first && nextDebugLocation.first <= pos) { + if (std::get<0>(nextDebugLocation) == 0 && std::get<1>(nextDebugLocation) <= pos) { + // if source map file had already reached the end and cache position also cannot cover the pos + // clear the debug location + debugLocation.clear(); + return; + } + + while (std::get<0>(nextDebugLocation) && std::get<0>(nextDebugLocation) <= pos) { debugLocation.clear(); // use debugLocation only for function expressions if (currFunction) { - debugLocation.insert(nextDebugLocation.second); + debugLocation.insert(std::get<2>(nextDebugLocation)); } char ch; *sourceMap >> ch; if (ch == '\"') { // end of records - nextDebugLocation.first = 0; + std::get<0>(nextDebugLocation) = 0; break; } if (ch != ',') { @@ -2776,16 +2783,16 @@ void WasmBinaryBuilder::readNextDebugLocation() { } int32_t positionDelta = readBase64VLQ(*sourceMap); - uint32_t position = nextDebugLocation.first + positionDelta; + uint32_t position = std::get<0>(nextDebugLocation) + positionDelta; int32_t fileIndexDelta = readBase64VLQ(*sourceMap); - uint32_t fileIndex = nextDebugLocation.second.fileIndex + fileIndexDelta; + uint32_t fileIndex = std::get<2>(nextDebugLocation).fileIndex + fileIndexDelta; int32_t lineNumberDelta = readBase64VLQ(*sourceMap); - uint32_t lineNumber = nextDebugLocation.second.lineNumber + lineNumberDelta; + uint32_t lineNumber = std::get<2>(nextDebugLocation).lineNumber + lineNumberDelta; int32_t columnNumberDelta = readBase64VLQ(*sourceMap); uint32_t columnNumber = - nextDebugLocation.second.columnNumber + columnNumberDelta; + std::get<2>(nextDebugLocation).columnNumber + columnNumberDelta; - nextDebugLocation = {position, {fileIndex, lineNumber, columnNumber}}; + nextDebugLocation = {position, std::get<0>(nextDebugLocation), {fileIndex, lineNumber, columnNumber}}; } } From d649bee7bf1e5930b093d534d4b1477d98b51a06 Mon Sep 17 00:00:00 2001 From: chen ruixiang Date: Fri, 24 Feb 2023 11:41:41 +0800 Subject: [PATCH 3/3] fix: source map test issue and refactor the code --- src/wasm-binary.h | 7 +++++- src/wasm/wasm-binary.cpp | 35 +++++++++++++++------------ test/fib-dbg.wasm.fromBinary | 2 +- test/lit/source-map.wast | 46 ++++++++++++++++++++++++++++++++++++ 4 files changed, 73 insertions(+), 17 deletions(-) create mode 100644 test/lit/source-map.wast diff --git a/src/wasm-binary.h b/src/wasm-binary.h index 49e1403a1a5..a5b4202a01f 100644 --- a/src/wasm-binary.h +++ b/src/wasm-binary.h @@ -1430,7 +1430,12 @@ class WasmBinaryBuilder { MixedArena& allocator; const std::vector& input; std::istream* sourceMap; - std::tuple nextDebugLocation; // available pos, previous pos, next debug location + struct NextDebugLocation { + uint32_t availablePos; + uint32_t previousPos; + Function::DebugLocation next; + }; + NextDebugLocation nextDebugLocation; bool debugInfo = true; bool DWARF = false; bool skipFunctionBodies = false; diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index a3c4b7b615f..bb6f082f80d 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -1572,8 +1572,8 @@ void WasmBinaryWriter::writeField(const Field& field) { WasmBinaryBuilder::WasmBinaryBuilder(Module& wasm, FeatureSet features, const std::vector& input) - : wasm(wasm), allocator(wasm.allocator), input(input), sourceMap(nullptr), - nextDebugLocation(0, 0, {0, 0, 0}), debugLocation() { + : wasm(wasm), allocator(wasm.allocator), input(input), + sourceMap(nullptr), nextDebugLocation{0, 0, {0, 0, 0}}, debugLocation() { wasm.features = features; } @@ -2741,7 +2741,7 @@ void WasmBinaryBuilder::readSourceMapHeader() { mustReadChar('\"'); if (maybeReadChar('\"')) { // empty mappings - std::get<0>(nextDebugLocation) = 0; + nextDebugLocation.availablePos = 0; return; } // read first debug location @@ -2750,7 +2750,8 @@ void WasmBinaryBuilder::readSourceMapHeader() { uint32_t lineNumber = readBase64VLQ(*sourceMap) + 1; // adjust zero-based line number uint32_t columnNumber = readBase64VLQ(*sourceMap); - nextDebugLocation = {position, position, {fileIndex, lineNumber, columnNumber}}; + nextDebugLocation = { + position, position, {fileIndex, lineNumber, columnNumber}}; } void WasmBinaryBuilder::readNextDebugLocation() { @@ -2758,24 +2759,26 @@ void WasmBinaryBuilder::readNextDebugLocation() { return; } - if (std::get<0>(nextDebugLocation) == 0 && std::get<1>(nextDebugLocation) <= pos) { - // if source map file had already reached the end and cache position also cannot cover the pos - // clear the debug location + if (nextDebugLocation.availablePos == 0 && + nextDebugLocation.previousPos <= pos) { + // if source map file had already reached the end and cache position also + // cannot cover the pos clear the debug location debugLocation.clear(); return; } - while (std::get<0>(nextDebugLocation) && std::get<0>(nextDebugLocation) <= pos) { + while (nextDebugLocation.availablePos && + nextDebugLocation.availablePos <= pos) { debugLocation.clear(); // use debugLocation only for function expressions if (currFunction) { - debugLocation.insert(std::get<2>(nextDebugLocation)); + debugLocation.insert(nextDebugLocation.next); } char ch; *sourceMap >> ch; if (ch == '\"') { // end of records - std::get<0>(nextDebugLocation) = 0; + nextDebugLocation.availablePos = 0; break; } if (ch != ',') { @@ -2783,16 +2786,18 @@ void WasmBinaryBuilder::readNextDebugLocation() { } int32_t positionDelta = readBase64VLQ(*sourceMap); - uint32_t position = std::get<0>(nextDebugLocation) + positionDelta; + uint32_t position = nextDebugLocation.availablePos + positionDelta; int32_t fileIndexDelta = readBase64VLQ(*sourceMap); - uint32_t fileIndex = std::get<2>(nextDebugLocation).fileIndex + fileIndexDelta; + uint32_t fileIndex = nextDebugLocation.next.fileIndex + fileIndexDelta; int32_t lineNumberDelta = readBase64VLQ(*sourceMap); - uint32_t lineNumber = std::get<2>(nextDebugLocation).lineNumber + lineNumberDelta; + uint32_t lineNumber = nextDebugLocation.next.lineNumber + lineNumberDelta; int32_t columnNumberDelta = readBase64VLQ(*sourceMap); uint32_t columnNumber = - std::get<2>(nextDebugLocation).columnNumber + columnNumberDelta; + nextDebugLocation.next.columnNumber + columnNumberDelta; - nextDebugLocation = {position, std::get<0>(nextDebugLocation), {fileIndex, lineNumber, columnNumber}}; + nextDebugLocation = {position, + nextDebugLocation.availablePos, + {fileIndex, lineNumber, columnNumber}}; } } diff --git a/test/fib-dbg.wasm.fromBinary b/test/fib-dbg.wasm.fromBinary index 8db3246ad4a..8abbef76430 100644 --- a/test/fib-dbg.wasm.fromBinary +++ b/test/fib-dbg.wasm.fromBinary @@ -204,8 +204,8 @@ (br $label$4) ) ) - ;;@ fib.c:8:0 (return + ;;@ fib.c:8:0 (local.get $4) ) ) diff --git a/test/lit/source-map.wast b/test/lit/source-map.wast new file mode 100644 index 00000000000..a0e969c23b1 --- /dev/null +++ b/test/lit/source-map.wast @@ -0,0 +1,46 @@ +;; RUN: wasm-opt %s -o %t.wasm -osm %t.map -g -q +;; RUN: wasm-opt %t.wasm -ism %t.map -q -o - -S | filecheck %s + +(module + (func $foo (param $x i32) (param $y i32) + ;;@ src.cpp:10:1 + (if + ;;@ src.cpp:20:1 + (i32.add + ;;@ src.cpp:30:1 + (local.get $x) + ;;@ src.cpp:40:1 + (local.get $y) + ) + ;;@ src.cpp:50:1 + (return) + ) + ;;@ src.cpp:60:1 + (call $foo + ;;@ src.cpp:70:1 + (local.get $x) + ;;@ src.cpp:80:1 + (local.get $y) + ) + ) +) + +;; CHECK: (func $foo (param $x i32) (param $y i32) +;; CHECK-NEXT: ;;@ src.cpp:20:1 +;; CHECK-NEXT: (if +;; CHECK-NEXT: (i32.add +;; CHECK-NEXT: ;;@ src.cpp:30:1 +;; CHECK-NEXT: (local.get $x) +;; CHECK-NEXT: ;;@ src.cpp:40:1 +;; CHECK-NEXT: (local.get $y) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ;;@ src.cpp:50:1 +;; CHECK-NEXT: (return) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ;;@ src.cpp:60:1 +;; CHECK-NEXT: (call $foo +;; CHECK-NEXT: ;;@ src.cpp:70:1 +;; CHECK-NEXT: (local.get $x) +;; CHECK-NEXT: ;;@ src.cpp:80:1 +;; CHECK-NEXT: (local.get $y) +;; CHECK-NEXT: )