summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlon Zakai <azakai@google.com>2023-08-31 14:04:06 -0700
committerGitHub <noreply@github.com>2023-08-31 14:04:06 -0700
commit0d3c2eeb634913643f2cade4b5d738962b2308c2 (patch)
treebeccb8c6ac99b892b25d38c5d48e93f658b1999a
parent90d8185ba2be34fa6b6a8f8ce0cbb87e0a9ed0da (diff)
downloadbinaryen-0d3c2eeb634913643f2cade4b5d738962b2308c2.tar.gz
binaryen-0d3c2eeb634913643f2cade4b5d738962b2308c2.tar.bz2
binaryen-0d3c2eeb634913643f2cade4b5d738962b2308c2.zip
DebugInfo: Fix loss of debug info in replaceCurrent() (#5914)
The logic there assumed that we are removing the current node and replacing it with the given one, so it copied debug info to the new one and deleted it for the old. But the old one might now be a child of the new one, if we reordered, so we were dropping debug info, in particular in MergeBlocks which reorders like this: (call (block .. => (block (call (it moves blocks outwards so it can merge them).
-rw-r--r--src/wasm-traversal.h21
-rw-r--r--test/lit/debug/replace-keep.wat40
2 files changed, 58 insertions, 3 deletions
diff --git a/src/wasm-traversal.h b/src/wasm-traversal.h
index 32225fd3f..80378242a 100644
--- a/src/wasm-traversal.h
+++ b/src/wasm-traversal.h
@@ -128,9 +128,24 @@ struct Walker : public VisitorType {
auto* curr = getCurrent();
auto iter = debugLocations.find(curr);
if (iter != debugLocations.end()) {
- auto location = iter->second;
- debugLocations.erase(iter);
- debugLocations[expression] = location;
+ debugLocations[expression] = iter->second;
+ // Note that we do *not* erase the debug info of the expression being
+ // replaced, because it may still exist: we might replace
+ //
+ // (call
+ // (block ..
+ //
+ // with
+ //
+ // (block
+ // (call ..
+ //
+ // We still want the call here to have its old debug info.
+ //
+ // (In most cases, of course, we do remove the replaced expression,
+ // which means we accumulate unused garbage in debugLocations, but
+ // that's not that bad; we use arena allocation for Expressions, after
+ // all.)
}
}
}
diff --git a/test/lit/debug/replace-keep.wat b/test/lit/debug/replace-keep.wat
new file mode 100644
index 000000000..1418d13dd
--- /dev/null
+++ b/test/lit/debug/replace-keep.wat
@@ -0,0 +1,40 @@
+;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited.
+
+;; RUN: env BINARYEN_PRINT_FULL=1 wasm-opt %s --merge-blocks -S -o - | filecheck %s
+
+;; The optimization below will replace the local.set with the block it contains
+;; (i.e. it reorders them). While doing so we should keep the debug info of the
+;; local.set.
+
+(module
+ ;; CHECK: (func $test
+ ;; CHECK-NEXT: (local $temp i32)
+ ;; CHECK-NEXT: [none] ;;@ src.cpp:200:2
+ ;; CHECK-NEXT: [none](block
+ ;; CHECK-NEXT: [none] ;;@ src.cpp:200:2
+ ;; CHECK-NEXT: (call $test)
+ ;; CHECK-NEXT: [none] ;;@ src.cpp:200:2
+ ;; CHECK-NEXT: (local.set $temp
+ ;; CHECK-NEXT: [i32] ;;@ src.cpp:200:2
+ ;; CHECK-NEXT: (i32.const 1)
+ ;; CHECK-NEXT: )
+ ;; CHECK-NEXT: ) ;; end block
+ ;; CHECK-NEXT: )
+ (func $test
+ (local $temp i32)
+
+ ;; Everything here should stay with 200 as their debug info, while we
+ ;; optimize (we can remove the inner block, move the call up to before
+ ;; the local.set, and merge the outer blocks).
+
+ ;;@ src.cpp:200:2
+ (block
+ (local.set $temp
+ (block (result i32)
+ (call $test)
+ (i32.const 1)
+ )
+ )
+ )
+ )
+)