Skip to content

Commit 763622c

Browse files
fitzgenfitzgen
authored
Preserve try_call[_indirect] stack maps during lowering (#12934)
* Preserve `try_call[_indirect]` stack maps during lowering Branch instructions are skipped in the main lowering loop, which means the stack map forwarding code is never reached for them. The branch lowering path didn't forward stack maps either. This was fine because branch instructions couldn't previously ever be safepoints. However, with the introduction of `try_call` and `try_call_indirect`, we now have instructions that are both safepoints and branches. This caused GC references live across `try_call[_indirect]` instructions to not be traced during garbage collection, leading to use-after-free within the GC heap sandbox when the collector swept those untraced-but-still-live objects. The fix adds stack map forwarding after branch lowering, mirroring the existing logic for non-branch instructions. Fixes #11753. * update disas test
1 parent 4678098 commit 763622c

5 files changed

Lines changed: 144 additions & 0 deletions

File tree

‎cranelift/codegen/src/machinst/lower.rs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1151,8 +1151,27 @@ impl<'func, I: VCodeInst> Lower<'func, I> {
11511151
// End branch.
11521152
if let Some(bb) = lb.orig_block() {
11531153
if let Some(branch) = self.collect_branch_and_targets(bindex, bb, &mut targets) {
1154+
let branch_start = self.vcode.vcode.num_insts();
11541155
self.lower_clif_branch(backend, bindex, bb, branch, &targets)?;
11551156
self.finish_ir_inst(self.srcloc(branch));
1157+
1158+
// Branch instructions like try_call can also be safepoints
1159+
// that need stack maps. Forward the stack map from the CLIF
1160+
// branch to the VCode safepoint, just like we do for
1161+
// non-branch instructions in `lower_clif_block`.
1162+
if let Some(entries) = self.f.dfg.user_stack_map_entries(branch) {
1163+
let branch_end = self.vcode.vcode.num_insts();
1164+
for i in branch_start..branch_end {
1165+
let iix = InsnIndex::new(i);
1166+
if self.vcode.vcode[iix].is_safepoint() {
1167+
self.vcode.add_user_stack_map(
1168+
BackwardsInsnIndex::new(iix.index()),
1169+
entries,
1170+
);
1171+
break;
1172+
}
1173+
}
1174+
}
11561175
}
11571176
} else {
11581177
// If no orig block, this must be a pure edge block;

‎crates/wast/src/wast.rs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,17 @@ impl WastContext {
236236
Ok(())
237237
}
238238

239+
/// Register the "wasmtime" module, which provides utilities that our misc
240+
/// tests use.
241+
pub fn register_wasmtime(&mut self) -> Result<()> {
242+
self.core_linker
243+
.func_wrap("wasmtime", "gc", |mut caller: Caller<_>| {
244+
caller.gc(None)?;
245+
Ok(())
246+
})?;
247+
Ok(())
248+
}
249+
239250
/// Perform the action portion of a command.
240251
fn perform_action(&mut self, action: &Action<'_>) -> Result<Outcome> {
241252
// Need to simultaneously borrow `self.async_runtime` and a `&mut

‎tests/disas/gc/issue-11753.wat‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
;;! target = "x86_64"
2+
;;! flags = "-W gc,exceptions"
3+
;;! test = "compile"
4+
;;! objdump = "--stack-maps=true"
5+
6+
(module
7+
(type $s (struct (field (mut i32))))
8+
9+
(import "" "gc" (func $gc))
10+
11+
(func (export "run") (result i32)
12+
(struct.new $s (i32.const 42))
13+
14+
block $b
15+
try_table (catch_all $b)
16+
;; This should have both exception handlers and stack maps in the
17+
;; disassembly below.
18+
call $gc
19+
end
20+
end
21+
22+
struct.get $s 0
23+
)
24+
)
25+
;; wasm[0]::function[1]:
26+
;; pushq %rbp
27+
;; movq %rsp, %rbp
28+
;; movq 8(%rdi), %r10
29+
;; movq 0x18(%r10), %r10
30+
;; addq $0x60, %r10
31+
;; cmpq %rsp, %r10
32+
;; ja 0xbe
33+
;; 19: subq $0x50, %rsp
34+
;; movq %rbx, 0x20(%rsp)
35+
;; movq %r12, 0x28(%rsp)
36+
;; movq %r13, 0x30(%rsp)
37+
;; movq %r14, 0x38(%rsp)
38+
;; movq %r15, 0x40(%rsp)
39+
;; movq %rdi, 8(%rsp)
40+
;; movl $0xb0000000, %esi
41+
;; xorl %edx, %edx
42+
;; movl $0x20, %ecx
43+
;; movl $8, %r8d
44+
;; movq 8(%rsp), %rbx
45+
;; movq %rbx, %rdi
46+
;; callq 0x215
47+
;; movl %eax, (%rsp)
48+
;; movq 8(%rbx), %rcx
49+
;; movq 0x20(%rcx), %rcx
50+
;; movl %eax, %eax
51+
;; movl $0x2a, 0x18(%rcx, %rax)
52+
;; movq %rcx, 0x10(%rsp)
53+
;; movq 0x30(%rbx), %rax
54+
;; movq 0x40(%rbx), %rdi
55+
;; movq %rbx, %rsi
56+
;; movq %rbx, 8(%rsp)
57+
;; callq *%rax
58+
;; ├─╼ exception frame offset: SP = FP - 0x50
59+
;; ╰─╼ exception handler: default handler, context at [SP+0x8], handler=0x86
60+
;; movl (%rsp), %eax
61+
;; ╰─╼ stack_map: frame_size=80, frame_offsets=[0]
62+
;; testl %eax, %eax
63+
;; je 0xc0
64+
;; 91: movl %eax, %eax
65+
;; movq 0x10(%rsp), %rcx
66+
;; movl 0x18(%rcx, %rax), %eax
67+
;; movq 0x20(%rsp), %rbx
68+
;; movq 0x28(%rsp), %r12
69+
;; movq 0x30(%rsp), %r13
70+
;; movq 0x38(%rsp), %r14
71+
;; movq 0x40(%rsp), %r15
72+
;; addq $0x50, %rsp
73+
;; movq %rbp, %rsp
74+
;; popq %rbp
75+
;; retq
76+
;; be: ud2
77+
;; ╰─╼ trap: StackOverflow
78+
;; c0: ud2
79+
;; ╰─╼ trap: NullReference

‎tests/misc_testsuite/gc/issue-11753.wast‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
;;! gc = true
2+
;;! exceptions = true
3+
4+
(module
5+
(type $s (struct (field (mut i32))))
6+
7+
(import "wasmtime" "gc" (func $gc))
8+
9+
(func $observe (param (ref null $s)) (result (ref null $s))
10+
local.get 0
11+
)
12+
13+
(func (export "run") (result i32)
14+
(struct.new $s (i32.const 42))
15+
call $observe
16+
17+
block $b
18+
try_table (catch_all $b)
19+
call $gc
20+
(drop (struct.new $s (i32.const 0)))
21+
end
22+
end
23+
24+
struct.get $s 0
25+
)
26+
)
27+
28+
(assert_return (invoke "run") (i32.const 42))

‎tests/wast.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,13 @@ fn run_wast(test: &WastTest, config: WastConfig) -> wasmtime::Result<()> {
267267
use_shared_memory: true,
268268
suppress_prints: true,
269269
})?;
270+
if test
271+
.path
272+
.to_str()
273+
.is_some_and(|s| s.contains("misc_testsuite"))
274+
{
275+
wast_context.register_wasmtime()?;
276+
}
270277
wast_context
271278
.run_wast(test.path.to_str().unwrap(), test.contents.as_bytes())
272279
.with_context(|| format!("failed to run spec test with {desc} engine"))

0 commit comments

Comments
 (0)