Skip to content

Commit 398a73f

Browse files
authored
Merge pull request from GHSA-4873-36h9-wv49
Stop doing fuzzy search for stack maps
2 parents 1019987 + ec4e48d commit 398a73f

File tree

1 file changed

+52
-48
lines changed

1 file changed

+52
-48
lines changed

Diff for: crates/wasmtime/src/module/registry.rs

+52-48
Original file line numberDiff line numberDiff line change
@@ -122,61 +122,65 @@ impl ModuleInfo for RegisteredModule {
122122
let info = self.module.func_info(index);
123123

124124
// Do a binary search to find the stack map for the given offset.
125-
//
126-
// Because GC safepoints are technically only associated with a single
127-
// PC, we should ideally only care about `Ok(index)` values returned
128-
// from the binary search. However, safepoints are inserted right before
129-
// calls, and there are two things that can disturb the PC/offset
130-
// associated with the safepoint versus the PC we actually use to query
131-
// for the stack map:
132-
//
133-
// 1. The `backtrace` crate gives us the PC in a frame that will be
134-
// *returned to*, and where execution will continue from, rather than
135-
// the PC of the call we are currently at. So we would need to
136-
// disassemble one instruction backwards to query the actual PC for
137-
// the stack map.
138-
//
139-
// TODO: One thing we *could* do to make this a little less error
140-
// prone, would be to assert/check that the nearest GC safepoint
141-
// found is within `max_encoded_size(any kind of call instruction)`
142-
// our queried PC for the target architecture.
143-
//
144-
// 2. Cranelift's stack maps only handle the stack, not
145-
// registers. However, some references that are arguments to a call
146-
// may need to be in registers. In these cases, what Cranelift will
147-
// do is:
148-
//
149-
// a. spill all the live references,
150-
// b. insert a GC safepoint for those references,
151-
// c. reload the references into registers, and finally
152-
// d. make the call.
153-
//
154-
// Step (c) adds drift between the GC safepoint and the location of
155-
// the call, which is where we actually walk the stack frame and
156-
// collect its live references.
157-
//
158-
// Luckily, the spill stack slots for the live references are still
159-
// up to date, so we can still find all the on-stack roots.
160-
// Furthermore, we do not have a moving GC, so we don't need to worry
161-
// whether the following code will reuse the references in registers
162-
// (which would not have been updated to point to the moved objects)
163-
// or reload from the stack slots (which would have been updated to
164-
// point to the moved objects).
165-
166125
let index = match info
167126
.stack_maps
168127
.binary_search_by_key(&func_offset, |i| i.code_offset)
169128
{
170-
// Exact hit.
129+
// Found it.
171130
Ok(i) => i,
172131

173-
// `Err(0)` means that the associated stack map would have been the
174-
// first element in the array if this pc had an associated stack
175-
// map, but this pc does not have an associated stack map. This can
176-
// only happen inside a Wasm frame if there are no live refs at this
177-
// pc.
132+
// No stack map associated with this PC.
133+
//
134+
// Because we know we are in Wasm code, and we must be at some kind
135+
// of call/safepoint, then the Cranelift backend must have avoided
136+
// emitting a stack map for this location because no refs were live.
137+
#[cfg(not(feature = "old-x86-backend"))]
138+
Err(_) => return None,
139+
140+
// ### Old x86_64 backend specific code.
141+
//
142+
// Because GC safepoints are technically only associated with a
143+
// single PC, we should ideally only care about `Ok(index)` values
144+
// returned from the binary search. However, safepoints are inserted
145+
// right before calls, and there are two things that can disturb the
146+
// PC/offset associated with the safepoint versus the PC we actually
147+
// use to query for the stack map:
148+
//
149+
// 1. The `backtrace` crate gives us the PC in a frame that will be
150+
// *returned to*, and where execution will continue from, rather than
151+
// the PC of the call we are currently at. So we would need to
152+
// disassemble one instruction backwards to query the actual PC for
153+
// the stack map.
154+
//
155+
// TODO: One thing we *could* do to make this a little less error
156+
// prone, would be to assert/check that the nearest GC safepoint
157+
// found is within `max_encoded_size(any kind of call instruction)`
158+
// our queried PC for the target architecture.
159+
//
160+
// 2. Cranelift's stack maps only handle the stack, not
161+
// registers. However, some references that are arguments to a call
162+
// may need to be in registers. In these cases, what Cranelift will
163+
// do is:
164+
//
165+
// a. spill all the live references,
166+
// b. insert a GC safepoint for those references,
167+
// c. reload the references into registers, and finally
168+
// d. make the call.
169+
//
170+
// Step (c) adds drift between the GC safepoint and the location of
171+
// the call, which is where we actually walk the stack frame and
172+
// collect its live references.
173+
//
174+
// Luckily, the spill stack slots for the live references are still
175+
// up to date, so we can still find all the on-stack roots.
176+
// Furthermore, we do not have a moving GC, so we don't need to worry
177+
// whether the following code will reuse the references in registers
178+
// (which would not have been updated to point to the moved objects)
179+
// or reload from the stack slots (which would have been updated to
180+
// point to the moved objects).
181+
#[cfg(feature = "old-x86-backend")]
178182
Err(0) => return None,
179-
183+
#[cfg(feature = "old-x86-backend")]
180184
Err(i) => i - 1,
181185
};
182186

0 commit comments

Comments
 (0)