Skip to content

Commit af0ae83

Browse files
authored
Reduce duplication amongst debug_* helpers (#12386)
Deduplicate the check for `guest_debug`, validity checks, and conversion from `Export` to `Extern`. No functional change here, just a refactoring.
1 parent fdd6550 commit af0ae83

File tree

4 files changed

+65
-92
lines changed

4 files changed

+65
-92
lines changed

crates/environ/src/module.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,17 @@ impl Module {
648648
pub fn num_defined_tags(&self) -> usize {
649649
self.tags.len() - self.num_imported_tags
650650
}
651+
652+
/// Tests whether `index` is valid for this module.
653+
pub fn is_valid(&self, index: EntityIndex) -> bool {
654+
match index {
655+
EntityIndex::Function(i) => self.functions.is_valid(i),
656+
EntityIndex::Table(i) => self.tables.is_valid(i),
657+
EntityIndex::Memory(i) => self.memories.is_valid(i),
658+
EntityIndex::Global(i) => self.globals.is_valid(i),
659+
EntityIndex::Tag(i) => self.tags.is_valid(i),
660+
}
661+
}
651662
}
652663

653664
impl TypeTrace for Module {

crates/wasmtime/src/runtime/debug.rs

Lines changed: 47 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
33
use crate::Result;
44
use crate::{
5-
AnyRef, AsContext, AsContextMut, CodeMemory, ExnRef, ExternRef, Func, Instance, Module,
5+
AnyRef, AsContext, AsContextMut, CodeMemory, ExnRef, Extern, ExternRef, Func, Instance, Module,
66
OwnedRooted, StoreContext, StoreContextMut, Val,
77
code::StoreCodePC,
88
module::ModuleRegistry,
@@ -16,9 +16,9 @@ use core::{ffi::c_void, ptr::NonNull};
1616
#[cfg(feature = "gc")]
1717
use wasmtime_environ::FrameTable;
1818
use wasmtime_environ::{
19-
DefinedFuncIndex, FrameInstPos, FrameStackShape, FrameStateSlot, FrameStateSlotOffset,
20-
FrameTableBreakpointData, FrameTableDescriptorIndex, FrameValType, FuncIndex, FuncKey,
21-
GlobalIndex, MemoryIndex, TableIndex, TagIndex, Trap,
19+
DefinedFuncIndex, EntityIndex, FrameInstPos, FrameStackShape, FrameStateSlot,
20+
FrameStateSlotOffset, FrameTableBreakpointData, FrameTableDescriptorIndex, FrameValType,
21+
FuncIndex, FuncKey, GlobalIndex, MemoryIndex, TableIndex, TagIndex, Trap,
2222
};
2323
use wasmtime_unwinder::Frame;
2424

@@ -86,22 +86,11 @@ impl Instance {
8686
mut store: impl AsContextMut,
8787
global_index: u32,
8888
) -> Option<crate::Global> {
89-
let store = store.as_context_mut().0;
90-
if !store.engine().tunables().debug_guest {
91-
return None;
92-
}
93-
94-
let instance = &store[self.id];
95-
let env_module = self.module(&store).env_module();
96-
// N.B.: `from_bits` here rather than `from_u32` so we don't
97-
// panic on `u32::MAX`. A `u32::MAX` will become an invalid
98-
// entity index which will properly return `None` below.
99-
let global = GlobalIndex::from_bits(global_index);
100-
if env_module.globals.is_valid(global) {
101-
Some(instance.get_exported_global(store.id(), global))
102-
} else {
103-
None
104-
}
89+
self.debug_export(
90+
store.as_context_mut().0,
91+
GlobalIndex::from_bits(global_index).into(),
92+
)
93+
.and_then(|s| s.into_global())
10594
}
10695

10796
/// Get access to a memory (unshared only) within this instance's
@@ -126,23 +115,11 @@ impl Instance {
126115
mut store: impl AsContextMut,
127116
memory_index: u32,
128117
) -> Option<crate::Memory> {
129-
let store = store.as_context_mut().0;
130-
if !store.engine().tunables().debug_guest {
131-
return None;
132-
}
133-
134-
let instance = &store[self.id];
135-
let env_module = self.module(&store).env_module();
136-
let memory = MemoryIndex::from_bits(memory_index);
137-
if env_module.memories.is_valid(memory) {
138-
Some(
139-
instance
140-
.get_exported_memory(store.id(), memory)
141-
.unshared()?,
142-
)
143-
} else {
144-
None
145-
}
118+
self.debug_export(
119+
store.as_context_mut().0,
120+
MemoryIndex::from_bits(memory_index).into(),
121+
)
122+
.and_then(|s| s.into_memory())
146123
}
147124

148125
/// Get access to a shared memory within this instance's memory
@@ -167,22 +144,11 @@ impl Instance {
167144
mut store: impl AsContextMut,
168145
memory_index: u32,
169146
) -> Option<crate::SharedMemory> {
170-
let store = store.as_context_mut().0;
171-
if !store.engine().tunables().debug_guest {
172-
return None;
173-
}
174-
175-
let instance = &store[self.id];
176-
let env_module = self.module(&store).env_module();
177-
let memory = MemoryIndex::from_bits(memory_index);
178-
if env_module.memories.is_valid(memory) {
179-
Some(crate::SharedMemory::from_raw(
180-
instance.get_exported_memory(store.id(), memory).shared()?,
181-
store.engine().clone(),
182-
))
183-
} else {
184-
None
185-
}
147+
self.debug_export(
148+
store.as_context_mut().0,
149+
MemoryIndex::from_bits(memory_index).into(),
150+
)
151+
.and_then(|s| s.into_shared_memory())
186152
}
187153

188154
/// Get access to a table within this instance's table index
@@ -204,19 +170,11 @@ impl Instance {
204170
mut store: impl AsContextMut,
205171
table_index: u32,
206172
) -> Option<crate::Table> {
207-
let store = store.as_context_mut().0;
208-
if !store.engine().tunables().debug_guest {
209-
return None;
210-
}
211-
212-
let instance = &store[self.id];
213-
let env_module = self.module(&store).env_module();
214-
let table = TableIndex::from_bits(table_index);
215-
if env_module.tables.is_valid(table) {
216-
Some(instance.get_exported_table(store.id(), table))
217-
} else {
218-
None
219-
}
173+
self.debug_export(
174+
store.as_context_mut().0,
175+
TableIndex::from_bits(table_index).into(),
176+
)
177+
.and_then(|s| s.into_table())
220178
}
221179

222180
/// Get access to a function within this instance's function index
@@ -239,23 +197,11 @@ impl Instance {
239197
mut store: impl AsContextMut,
240198
function_index: u32,
241199
) -> Option<crate::Func> {
242-
let store = store.as_context_mut().0;
243-
if !store.engine().tunables().debug_guest {
244-
return None;
245-
}
246-
247-
let env_module = self.module(&store).env_module();
248-
let func = FuncIndex::from_bits(function_index);
249-
if env_module.functions.is_valid(func) {
250-
let store_id = store.id();
251-
let (instance, registry) = store.instance_and_module_registry_mut(self.id());
252-
// SAFETY: the `store` and `registry` are associated with
253-
// this instance as we fetched teh instance directly from
254-
// the store above.
255-
unsafe { Some(instance.get_exported_func(registry, store_id, func)) }
256-
} else {
257-
None
258-
}
200+
self.debug_export(
201+
store.as_context_mut().0,
202+
FuncIndex::from_bits(function_index).into(),
203+
)
204+
.and_then(|s| s.into_func())
259205
}
260206

261207
/// Get access to a tag within this instance's tag index space.
@@ -272,19 +218,29 @@ impl Instance {
272218
/// `None` is returned if guest-debugging is not enabled in the
273219
/// engine configuration for this Store.
274220
pub fn debug_tag(&self, mut store: impl AsContextMut, tag_index: u32) -> Option<crate::Tag> {
275-
let store = store.as_context_mut().0;
221+
self.debug_export(
222+
store.as_context_mut().0,
223+
TagIndex::from_bits(tag_index).into(),
224+
)
225+
.and_then(|s| s.into_tag())
226+
}
227+
228+
fn debug_export(&self, store: &mut StoreOpaque, index: EntityIndex) -> Option<Extern> {
276229
if !store.engine().tunables().debug_guest {
277230
return None;
278231
}
279232

280-
let instance = &store[self.id];
281-
let env_module = self.module(&store).env_module();
282-
let tag = TagIndex::from_bits(tag_index);
283-
if env_module.tags.is_valid(tag) {
284-
Some(instance.get_exported_tag(store.id(), tag))
285-
} else {
286-
None
233+
let env_module = self._module(store).env_module();
234+
if !env_module.is_valid(index) {
235+
return None;
287236
}
237+
let store_id = store.id();
238+
let (instance, registry) = store.instance_and_module_registry_mut(self.id());
239+
// SAFETY: the `store` and `registry` are associated with
240+
// this instance as we fetched the instance directly from
241+
// the store above.
242+
let export = unsafe { instance.get_export_by_index_mut(registry, store_id, index) };
243+
Some(Extern::from_wasmtime_export(export, store))
288244
}
289245
}
290246

crates/wasmtime/src/runtime/externals.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,12 @@ impl<'instance> Export<'instance> {
249249
self.definition.into_memory()
250250
}
251251

252+
/// Consume this `Export` and return the contained `SharedMemory`, if it's
253+
/// a shared memory, or `None` otherwise.
254+
pub fn into_shared_memory(self) -> Option<SharedMemory> {
255+
self.definition.into_shared_memory()
256+
}
257+
252258
/// Consume this `Export` and return the contained `Global`, if it's a global,
253259
/// or `None` otherwise.
254260
pub fn into_global(self) -> Option<Global> {

crates/wasmtime/src/runtime/instance.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ impl Instance {
394394
self._module(store.into().0)
395395
}
396396

397-
fn _module<'a>(&self, store: &'a StoreOpaque) -> &'a Module {
397+
pub(crate) fn _module<'a>(&self, store: &'a StoreOpaque) -> &'a Module {
398398
store.module_for_instance(self.id).unwrap()
399399
}
400400

0 commit comments

Comments
 (0)