Skip to content

Commit 0d3c2ee

Browse files
authored
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).
1 parent 90d8185 commit 0d3c2ee

File tree

2 files changed

+58
-3
lines changed

2 files changed

+58
-3
lines changed

src/wasm-traversal.h

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,24 @@ struct Walker : public VisitorType {
128128
auto* curr = getCurrent();
129129
auto iter = debugLocations.find(curr);
130130
if (iter != debugLocations.end()) {
131-
auto location = iter->second;
132-
debugLocations.erase(iter);
133-
debugLocations[expression] = location;
131+
debugLocations[expression] = iter->second;
132+
// Note that we do *not* erase the debug info of the expression being
133+
// replaced, because it may still exist: we might replace
134+
//
135+
// (call
136+
// (block ..
137+
//
138+
// with
139+
//
140+
// (block
141+
// (call ..
142+
//
143+
// We still want the call here to have its old debug info.
144+
//
145+
// (In most cases, of course, we do remove the replaced expression,
146+
// which means we accumulate unused garbage in debugLocations, but
147+
// that's not that bad; we use arena allocation for Expressions, after
148+
// all.)
134149
}
135150
}
136151
}

test/lit/debug/replace-keep.wat

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited.
2+
3+
;; RUN: env BINARYEN_PRINT_FULL=1 wasm-opt %s --merge-blocks -S -o - | filecheck %s
4+
5+
;; The optimization below will replace the local.set with the block it contains
6+
;; (i.e. it reorders them). While doing so we should keep the debug info of the
7+
;; local.set.
8+
9+
(module
10+
;; CHECK: (func $test
11+
;; CHECK-NEXT: (local $temp i32)
12+
;; CHECK-NEXT: [none] ;;@ src.cpp:200:2
13+
;; CHECK-NEXT: [none](block
14+
;; CHECK-NEXT: [none] ;;@ src.cpp:200:2
15+
;; CHECK-NEXT: (call $test)
16+
;; CHECK-NEXT: [none] ;;@ src.cpp:200:2
17+
;; CHECK-NEXT: (local.set $temp
18+
;; CHECK-NEXT: [i32] ;;@ src.cpp:200:2
19+
;; CHECK-NEXT: (i32.const 1)
20+
;; CHECK-NEXT: )
21+
;; CHECK-NEXT: ) ;; end block
22+
;; CHECK-NEXT: )
23+
(func $test
24+
(local $temp i32)
25+
26+
;; Everything here should stay with 200 as their debug info, while we
27+
;; optimize (we can remove the inner block, move the call up to before
28+
;; the local.set, and merge the outer blocks).
29+
30+
;;@ src.cpp:200:2
31+
(block
32+
(local.set $temp
33+
(block (result i32)
34+
(call $test)
35+
(i32.const 1)
36+
)
37+
)
38+
)
39+
)
40+
)

0 commit comments

Comments
 (0)