Skip to content

Commit c71e18d

Browse files
fix(parser): do not fuse local.get/local.tee across a branch target
`visit_local_tee` peeked at the last emitted instruction and, when it was a `local.get`, replaced the pair with `LocalCopy(src, dst); LocalGet(dst)`. It never checked whether the tee's position is a jump target. When the `local.get` is a block's fallthrough result and the `local.tee` is the first instruction after `end`, every branch to that label lands on `LocalGet(dst)` instead of the tee: the branch's value is left dangling on the stack and a stale local is pushed on top. Loop starts and if/else joins have the same shape. TinyCC compiled with clang -Os/-Oz hit this in musl's realloc: the `if (!p) { r = malloc(n); br 1 }` path returned n instead of r, so the caller wrote into address 1200 and later trapped with an out-of-bounds load, while V8 and wabt ran the same module correctly. The lowering step now always emits a plain `LocalTee`. The fusion moves into the peephole rewriter, which only matches within a basic block and so cannot cross a label. Regression test covers the block-end, if/else-join and loop-start cases.
1 parent c67ce64 commit c71e18d

3 files changed

Lines changed: 96 additions & 23 deletions

File tree

crates/parser/src/optimize/rewrite.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -859,6 +859,9 @@ fn rewrite_local_tee32(
859859
replace!(output, *read, 1 => Instruction::SubConstTee32(I32LocalArg { value, local: dst }));
860860
}
861861
Instruction::LocalGet32(src) if src == dst => replace!(output, *read, 1 => Instruction::LocalGet32(src)),
862+
Instruction::LocalGet32(src) => {
863+
replace!(output, *read, 1 => [Instruction::LocalCopy32(src, dst), Instruction::LocalGet32(dst)]);
864+
}
862865
Instruction::BinOpLocalLocal32(op, left, right) => {
863866
let replacement = if op == BinOp::IAdd {
864867
Instruction::AddLocalLocalTee32(LocalTripleArg { left, right, dst })
@@ -931,6 +934,9 @@ fn rewrite_local_tee64(
931934
replace!(output, *read, 1 => Instruction::SubConstTee64(PackedOp::new(dst, packed.index)));
932935
}
933936
Instruction::LocalGet64(src) if src == dst => replace!(output, *read, 1 => Instruction::LocalGet64(src)),
937+
Instruction::LocalGet64(src) => {
938+
replace!(output, *read, 1 => [Instruction::LocalCopy64(src, dst), Instruction::LocalGet64(dst)]);
939+
}
934940
Instruction::BinOpLocalLocal64(op, left, right) => {
935941
let index = data.push_operand64(Operand64::<(u16, u16, u16)>::new(left, right, dst))?;
936942
replace!(output, *read, 1 => Instruction::BinOpLocalLocalTee64(PackedOp::new(op, index)));
@@ -967,6 +973,9 @@ fn rewrite_local_tee128(
967973
if *read > output.block_start {
968974
match output[*read - 1] {
969975
Instruction::LocalGet128(src) if src == dst => replace!(output, *read, 1 => Instruction::LocalGet128(src)),
976+
Instruction::LocalGet128(src) => {
977+
replace!(output, *read, 1 => [Instruction::LocalCopy128(src, dst), Instruction::LocalGet128(dst)]);
978+
}
970979
Instruction::BinOpLocalLocal128(op, left, right) => {
971980
let index = data.push_operand64(Operand64::<(u16, u16, u16)>::new(left, right, dst))?;
972981
replace!(output, *read, 1 => Instruction::BinOpLocalLocalTee128(PackedOp::new(op, index)));

crates/parser/src/visit.rs

Lines changed: 10 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -735,29 +735,16 @@ impl<'a> wasmparser::VisitOperator<'a> for FunctionBuilder<'_> {
735735

736736
fn visit_local_tee(&mut self, idx: u32) -> Self::Output {
737737
let (size, local_idx) = self.local(idx)?;
738-
self.apply_effect(&[size], &[size])?;
739-
let src = match (size, self.instructions.last()) {
740-
(ValueLane::S32, Some(Instruction::LocalGet32(src))) => Some(*src),
741-
(ValueLane::S64, Some(Instruction::LocalGet64(src))) => Some(*src),
742-
(ValueLane::S128, Some(Instruction::LocalGet128(src))) => Some(*src),
743-
_ => None,
744-
};
745-
if let Some(src) = src {
746-
self.instructions.pop();
747-
let instructions = match size {
748-
ValueLane::S32 => [Instruction::LocalCopy32(src, local_idx), Instruction::LocalGet32(local_idx)],
749-
ValueLane::S64 => [Instruction::LocalCopy64(src, local_idx), Instruction::LocalGet64(local_idx)],
750-
ValueLane::S128 => [Instruction::LocalCopy128(src, local_idx), Instruction::LocalGet128(local_idx)],
751-
};
752-
self.instructions.extend(instructions);
753-
} else {
754-
self.instructions.push(size.select(
755-
Instruction::LocalTee32(local_idx),
756-
Instruction::LocalTee64(local_idx),
757-
Instruction::LocalTee128(local_idx),
758-
));
759-
}
760-
Ok(())
738+
// No peephole here: this position may be a branch target (block end,
739+
// loop start, if/else join), and fusing with the preceding `local.get`
740+
// would move the label past the tee. The rewriter fuses the same pair
741+
// within a basic block, where no branch can land between them.
742+
let instruction = size.select(
743+
Instruction::LocalTee32(local_idx),
744+
Instruction::LocalTee64(local_idx),
745+
Instruction::LocalTee128(local_idx),
746+
);
747+
self.emit(&[size], &[size], instruction)
761748
}
762749

763750
fn visit_block(&mut self, blockty: wasmparser::BlockType) -> Self::Output {
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
use tinywasm::{ModuleInstance, Store};
2+
3+
/// A `local.get` right before a label and a `local.tee` right after it must
4+
/// not be fused: branches land on the label with the value on the stack and
5+
/// still have to execute the tee.
6+
#[test]
7+
fn branches_landing_on_local_tee_still_tee() -> Result<(), Box<dyn core::error::Error>> {
8+
let wasm = wat::parse_str(
9+
r#"
10+
(module
11+
(func $forty_two (result i32) i32.const 42)
12+
13+
;; block end label: `br` out of the `if` carries the callee result
14+
(func (export "block_end") (param i32 i32) (result i32)
15+
block (result i32)
16+
local.get 0
17+
i32.eqz
18+
if
19+
call $forty_two
20+
br 1
21+
end
22+
local.get 0
23+
end
24+
local.tee 1
25+
drop
26+
local.get 1)
27+
28+
;; if/else join label: the then-arm's implicit jump lands on the tee
29+
(func (export "if_else_join") (param i32 i32) (result i32)
30+
local.get 0
31+
if (result i32)
32+
call $forty_two
33+
else
34+
local.get 0
35+
end
36+
local.tee 1
37+
drop
38+
local.get 1)
39+
40+
;; loop start label: `br 0` re-enters at the tee with the loop param
41+
(func (export "loop_start") (param i32 i32) (result i32)
42+
local.get 0
43+
loop (param i32) (result i32)
44+
local.tee 1
45+
i32.const 40
46+
i32.lt_u
47+
if (result i32)
48+
local.get 1
49+
i32.const 21
50+
i32.add
51+
br 1
52+
else
53+
local.get 1
54+
end
55+
end)
56+
)
57+
"#,
58+
)?;
59+
60+
let module = tinywasm::parse_bytes(&wasm)?;
61+
let mut store = Store::default();
62+
let instance = ModuleInstance::instantiate(&mut store, &module, None)?;
63+
64+
let block_end = instance.func::<(i32, i32), i32>(&store, "block_end")?;
65+
assert_eq!(block_end.call(&mut store, (5, 7))?, 5, "fallthrough keeps local 0");
66+
assert_eq!(block_end.call(&mut store, (0, 7))?, 42, "branch path must tee the callee result");
67+
68+
let if_else_join = instance.func::<(i32, i32), i32>(&store, "if_else_join")?;
69+
assert_eq!(if_else_join.call(&mut store, (0, 7))?, 0, "else arm keeps local 0");
70+
assert_eq!(if_else_join.call(&mut store, (1, 7))?, 42, "then arm must tee the callee result");
71+
72+
let loop_start = instance.func::<(i32, i32), i32>(&store, "loop_start")?;
73+
assert_eq!(loop_start.call(&mut store, (40, 7))?, 40, "no iteration keeps the param");
74+
assert_eq!(loop_start.call(&mut store, (0, 7))?, 42, "re-entering the loop must tee the carried value");
75+
76+
Ok(())
77+
}

0 commit comments

Comments
 (0)