Skip to content

Commit 15df7fe

Browse files
kripkenkripken
authored
Fix BranchHinting + source maps (#7585)
We move the Code section around when writing BranchHints (since we emit the Code first to find the offsets, then put BranchHints before it as the spec requires). Source map offsets are not relative to the Code section but absolute to the start of the binary, so we must adjust.
1 parent 6e7a4a0 commit 15df7fe

File tree

2 files changed

+86
-14
lines changed

2 files changed

+86
-14
lines changed

‎src/wasm/wasm-binary.cpp

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -482,13 +482,22 @@ void WasmBinaryWriter::writeFunctions() {
482482
// We need to move the code section and put the annotations before it.
483483
auto& annotationsBuffer = *annotations;
484484
auto oldSize = o.size();
485-
o.resize(oldSize + annotationsBuffer.size());
485+
auto annotationsSectionSize = annotationsBuffer.size();
486+
o.resize(oldSize + annotationsSectionSize);
486487

487488
// |sectionStart| is the start of the contents of the section. Subtract 1 to
488489
// include the section code as well, so we move all of it.
489490
std::move_backward(&o[sectionStart - 1], &o[oldSize], o.end());
490491
std::copy(
491492
annotationsBuffer.begin(), annotationsBuffer.end(), &o[sectionStart - 1]);
493+
494+
// Source map offsets are absolute (from the start of the binary) so we must
495+
// adjust them after moving the code section.
496+
if (sourceMap) {
497+
for (auto& location : sourceMapLocations) {
498+
location.first += annotationsSectionSize;
499+
}
500+
}
492501
}
493502
}
494503

@@ -1608,13 +1617,6 @@ std::optional<BufferWithRandomAccess> WasmBinaryWriter::writeCodeAnnotations() {
16081617
return {};
16091618
}
16101619

1611-
if (sourceMap) {
1612-
// TODO: This mode may not matter (when debugging, code annotations are an
1613-
// optimization that can be skipped), but atm source maps cause
1614-
// annotations to break.
1615-
Fatal() << "Annotations are not supported with source maps";
1616-
}
1617-
16181620
BufferWithRandomAccess buffer;
16191621

16201622
// We found data: emit the section.

‎test/lit/wat-annotations.wast

Lines changed: 76 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
22

33
;; RUN: wasm-opt -all %s -S -o - | filecheck %s
4+
45
;; RUN: wasm-opt -all --roundtrip %s -S -o - | filecheck %s --check-prefix=RTRIP
56

7+
;; RUN: wasm-opt -all %s -g -o %t.wasm -osm %t.map
8+
;; RUN: wasm-opt -all %t.wasm -ism %t.map -S -o - | filecheck %s --check-prefix=SRCMP
9+
610
(module
711

812
;; CHECK: (type $0 (func (param i32)))
@@ -27,6 +31,17 @@
2731
;; RTRIP-NEXT: )
2832
;; RTRIP-NEXT: )
2933
;; RTRIP-NEXT: )
34+
;; SRCMP: (type $0 (func (param i32)))
35+
36+
;; SRCMP: (type $1 (func (param anyref)))
37+
38+
;; SRCMP: (func $no-annotations (type $0) (param $x i32)
39+
;; SRCMP-NEXT: (block $block
40+
;; SRCMP-NEXT: (br_if $block
41+
;; SRCMP-NEXT: (local.get $x)
42+
;; SRCMP-NEXT: )
43+
;; SRCMP-NEXT: )
44+
;; SRCMP-NEXT: )
3045
(func $no-annotations (param $x i32)
3146
;; A function with no annotations. This tests that we use function indexes
3247
;; properly in the section.
@@ -69,6 +84,22 @@
6984
;; RTRIP-NEXT: )
7085
;; RTRIP-NEXT: )
7186
;; RTRIP-NEXT: )
87+
;; SRCMP: (func $branch-hints-br_if (type $0) (param $x i32)
88+
;; SRCMP-NEXT: (block $block
89+
;; SRCMP-NEXT: (@metadata.code.branch_hint "\00")
90+
;; SRCMP-NEXT: (br_if $block
91+
;; SRCMP-NEXT: (local.get $x)
92+
;; SRCMP-NEXT: )
93+
;; SRCMP-NEXT: (@metadata.code.branch_hint "\01")
94+
;; SRCMP-NEXT: (br_if $block
95+
;; SRCMP-NEXT: (local.get $x)
96+
;; SRCMP-NEXT: )
97+
;; SRCMP-NEXT: (@metadata.code.branch_hint "\00")
98+
;; SRCMP-NEXT: (br_if $block
99+
;; SRCMP-NEXT: (local.get $x)
100+
;; SRCMP-NEXT: )
101+
;; SRCMP-NEXT: )
102+
;; SRCMP-NEXT: )
72103
(func $branch-hints-br_if (param $x i32)
73104
(block $out
74105
;; A branch annotated as unlikely, and one as likely.
@@ -107,6 +138,15 @@
107138
;; RTRIP-NEXT: )
108139
;; RTRIP-NEXT: )
109140
;; RTRIP-NEXT: )
141+
;; SRCMP: (func $branch_hints-br_if-2 (type $0) (param $x i32)
142+
;; SRCMP-NEXT: (local $unused f64)
143+
;; SRCMP-NEXT: (block $block
144+
;; SRCMP-NEXT: (@metadata.code.branch_hint "\01")
145+
;; SRCMP-NEXT: (br_if $block
146+
;; SRCMP-NEXT: (local.get $x)
147+
;; SRCMP-NEXT: )
148+
;; SRCMP-NEXT: )
149+
;; SRCMP-NEXT: )
110150
(func $branch_hints-br_if-2 (param $x i32)
111151
(local $unused f64)
112152
;; A second function with hints. This one also has local definitions, which
@@ -137,13 +177,18 @@
137177
;; RTRIP-NEXT: )
138178
;; RTRIP-NEXT: )
139179
;; RTRIP-NEXT: )
180+
;; SRCMP: (func $mixing (type $0) (param $x i32)
181+
;; SRCMP-NEXT: ;;@ mixing.src:1337:42
182+
;; SRCMP-NEXT: (block $block
183+
;; SRCMP-NEXT: (@metadata.code.branch_hint "\01")
184+
;; SRCMP-NEXT: (br_if $block
185+
;; SRCMP-NEXT: (local.get $x)
186+
;; SRCMP-NEXT: )
187+
;; SRCMP-NEXT: )
188+
;; SRCMP-NEXT: )
140189
(func $mixing (param $x i32)
141-
;; Mix branch hints with source locations. Both hints should remain.
142-
;; TODO: Fix this in the binary format. Atm we test with --roundtrip here,
143-
;; which does not use source maps, so it is expected for the source
144-
;; annotation to vanish. But using source maps does not fix it, see
145-
;; the TODO in the code.
146-
190+
;; Mix branch hints with source locations. Both hints should remain (though
191+
;; not in --roundtrip, which does not use source maps).
147192
;;@ mixing.src:1337:42
148193
(block $out
149194
(@metadata.code.branch_hint "\01")
@@ -183,6 +228,21 @@
183228
;; RTRIP-NEXT: )
184229
;; RTRIP-NEXT: )
185230
;; RTRIP-NEXT: )
231+
;; SRCMP: (func $branch-hints-if (type $0) (param $x i32)
232+
;; SRCMP-NEXT: (@metadata.code.branch_hint "\00")
233+
;; SRCMP-NEXT: (if
234+
;; SRCMP-NEXT: (local.get $x)
235+
;; SRCMP-NEXT: (then
236+
;; SRCMP-NEXT: (@metadata.code.branch_hint "\01")
237+
;; SRCMP-NEXT: (if
238+
;; SRCMP-NEXT: (local.get $x)
239+
;; SRCMP-NEXT: (then
240+
;; SRCMP-NEXT: (nop)
241+
;; SRCMP-NEXT: )
242+
;; SRCMP-NEXT: )
243+
;; SRCMP-NEXT: )
244+
;; SRCMP-NEXT: )
245+
;; SRCMP-NEXT: )
186246
(func $branch-hints-if (param $x i32)
187247
(@metadata.code.branch_hint "\00")
188248
(if
@@ -219,6 +279,16 @@
219279
;; RTRIP-NEXT: )
220280
;; RTRIP-NEXT: )
221281
;; RTRIP-NEXT: )
282+
;; SRCMP: (func $branch-hints-br_on (type $1) (param $x anyref)
283+
;; SRCMP-NEXT: (block $block
284+
;; SRCMP-NEXT: (drop
285+
;; SRCMP-NEXT: (@metadata.code.branch_hint "\00")
286+
;; SRCMP-NEXT: (br_on_null $block
287+
;; SRCMP-NEXT: (local.get $x)
288+
;; SRCMP-NEXT: )
289+
;; SRCMP-NEXT: )
290+
;; SRCMP-NEXT: )
291+
;; SRCMP-NEXT: )
222292
(func $branch-hints-br_on (param $x anyref)
223293
(block $out
224294
(drop

0 commit comments

Comments
 (0)