Skip to content

Add generic/typed take_struct imports + ops #11713

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
+163 −34
Open

Add generic/typed take_struct imports + ops #11713

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
faa4512
add generic/typed take_struct imports + ops
khagankhan Sep 18, 2025
b83186f
Address clippy error
khagankhan Sep 18, 2025
fd2423d
Merge branch 'bytecodealliance:main' into rec-mutators
khagankhan Oct 15, 2025
e1d2bd0
Address comments. Split GC related codes to its own modules. Update T…
khagankhan Oct 18, 2025
0634e51
Remove table ops
khagankhan Oct 18, 2025
74b05e4
Revert "Remove table ops"
Oct 23, 2025
e77a216
Revert "Address comments. Split GC related codes to its own modules. …
Oct 23, 2025
320f01d
Address feedback and immediate generator crashes
khagankhan Oct 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 90 additions & 15 deletions crates/fuzzing/src/generators/table_ops.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ const TABLE_SIZE_RANGE: RangeInclusive<u32> = 0..=100;
const MAX_REC_GROUPS_RANGE: RangeInclusive<u32> = 0..=10;
const MAX_OPS: usize = 100;

const STRUCT_BASE: u32 = 5;
const TYPED_FN_BASE: u32 = 4;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary in this PR, but it would probably be easier/cleaner down the line to switch from constants to using the wasm_encoder::{Type,Func}Section::len methods to keep track of these things, so that we don't need to update these constants every time we unconditionally add a new type/function.

https://docs.rs/wasm-encoder/latest/wasm_encoder/struct.TypeSection.html#method.len


/// RecGroup ID struct definition.
#[derive(
Debug, Copy, Clone, Eq, PartialOrd, PartialEq, Ord, Hash, Default, Serialize, Deserialize,
Expand Down Expand Up @@ -191,6 +194,14 @@ impl TableOps {
vec![ValType::EXTERNREF, ValType::EXTERNREF, ValType::EXTERNREF],
);

types.ty().function(
vec![ValType::Ref(RefType {
nullable: false,
heap_type: wasm_encoder::HeapType::ANY,
})],
vec![],
);

let mut rec_groups: BTreeMap<RecGroupId, Vec<TypeId>> = self
.types
.rec_groups
Expand Down Expand Up @@ -219,16 +230,45 @@ impl TableOps {
}
};

let mut struct_count: u32 = 0;
for type_ids in rec_groups.values() {
let members: Vec<wasm_encoder::SubType> = type_ids.iter().map(encode_ty_id).collect();
types.ty().rec(members);
struct_count += type_ids.len() as u32;
}

let typed_ft_base: u32 = STRUCT_BASE + struct_count;
for i in 0..struct_count {
let concrete = STRUCT_BASE + i;
types.ty().function(
vec![ValType::Ref(RefType {
nullable: false,
heap_type: wasm_encoder::HeapType::Concrete(concrete),
})],
vec![],
);
}

// Import the GC function.
let mut imports = ImportSection::new();
imports.import("", "gc", EntityType::Function(0));
imports.import("", "take_refs", EntityType::Function(2));
imports.import("", "make_refs", EntityType::Function(3));
imports.import("", "take_struct", EntityType::Function(4));

let mut typed_names: Vec<String> = Vec::new();

for i in 0..struct_count {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to have a single-sentence comment for readers here describing what this code is doing, so that they don't have to scrutinize it to figure out what is going on. Something like

For each of our concrete struct types, define a function import that takes an argument of that concrete type.

let concrete = STRUCT_BASE + i;
let ty_idx = typed_ft_base + i; //
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty comment at the end of line?

let name = format!("take_struct_{concrete}");
typed_names.push(name);
imports.import(
"",
typed_names.last().unwrap().as_str(),
EntityType::Function(ty_idx),
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can avoid the wonky .last() stuff by pushing the name after defining the import:

Suggested change
let name = format!("take_struct_{concrete}");
typed_names.push(name);
imports.import(
"",
typed_names.last().unwrap().as_str(),
EntityType::Function(ty_idx),
);
let name = format!("take_struct_{concrete}");
imports.import(
"",
&name,
EntityType::Function(ty_idx),
);
typed_names.push(name);

}

// Define our table.
let mut tables = TableSection::new();
Expand Down Expand Up @@ -258,7 +298,8 @@ impl TableOps {
functions.function(1);

let mut exports = ExportSection::new();
exports.export("run", ExportKind::Func, 3);
let imported_fn_count: u32 = 4 + struct_count;
exports.export("run", ExportKind::Func, imported_fn_count);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned earlier, things get a bit simpler and less brittle if we use the len functions on the sections that define index spaces:

Suggested change
exports.export("run", ExportKind::Func, imported_fn_count);
let mut functions = FunctionSection::new();
let mut exports = ExportSection::new();
// Define the "run" function export.
let run_index = functions.len();
functions.function(1);
exports.export("run", ExportKind::Func, run_index);

I think it is worth doing this to avoid adding more opaque <constant> + <count> incantations.


// Give ourselves one scratch local that we can use in various `TableOp`
// implementations.
Expand Down Expand Up @@ -600,6 +641,8 @@ define_table_ops! {
LocalSet(local_index: |ops| ops.num_params => u32) : 1 => 0,

StructNew(type_index: |ops| ops.max_types => u32) : 0 => 0,
TakeStructCall(type_index: |ops| ops.max_types => u32) : 0 => 0,
TakeTypedStructCall(type_index: |ops| ops.max_types => u32) : 0 => 0,

Drop : 1 => 0,

Expand All @@ -611,6 +654,7 @@ impl TableOp {
let gc_func_idx = 0;
let take_refs_func_idx = 1;
let make_refs_func_idx = 2;
let take_structref_idx = 3;

match self {
Self::Gc() => {
Expand Down Expand Up @@ -651,8 +695,18 @@ impl TableOp {
func.instruction(&Instruction::RefNull(wasm_encoder::HeapType::EXTERN));
}
Self::StructNew(x) => {
func.instruction(&Instruction::StructNew(x + 4));
func.instruction(&Instruction::Drop);
func.instruction(&Instruction::StructNew(x + 5));
func.instruction(&Instruction::Call(take_structref_idx));
}
Self::TakeStructCall(x) => {
func.instruction(&Instruction::StructNew(x + 5));
func.instruction(&Instruction::Call(take_structref_idx));
}
Self::TakeTypedStructCall(x) => {
let s = STRUCT_BASE + x;
let f = TYPED_FN_BASE + x;
func.instruction(&Instruction::StructNew(s));
func.instruction(&Instruction::Call(f));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing to change in this PR, but I want to point out that these emitted instruction sequences are not our ideal end state. We want to separate struct.new from various calls and drops and have the existing TableOp::Drop be used to drop struct refs and we want the take_* calls to take a struct ref from the stack, not create a new one to pass to the import. This will require updating the abstract stack representation to track types, however, so it should be left to a follow up PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree!

}
}
}
Expand Down Expand Up @@ -726,7 +780,7 @@ mod tests {

let mut session = mutatis::Session::new();

for _ in 0..5 {
for _ in 0..2048 {
session.mutate(&mut res)?;
let wasm = res.to_wasm_binary();

Expand All @@ -738,7 +792,6 @@ mod tests {
let wat = wasmprinter::print_bytes(&wasm).expect("[-] Failed .print_bytes(&wasm).");
let result = validator.validate_all(&wasm);
log::debug!("{wat}");
println!("{wat}");
assert!(
result.is_ok(),
"\n[-] Invalid wat: {}\n\t\t==== Failed Wat ====\n{}",
Expand Down Expand Up @@ -788,37 +841,59 @@ mod tests {
(type (;1;) (func (param externref externref)))
(type (;2;) (func (param externref externref externref)))
(type (;3;) (func (result externref externref externref)))
(type (;4;) (func (param (ref any))))
(rec
(type (;4;) (struct))
(type (;5;) (struct))
)
(rec)
(rec
(type (;5;) (struct))
(type (;6;) (struct))
)
(rec
(type (;6;) (struct))
(type (;7;) (struct))
(type (;8;) (struct))
(type (;9;) (struct))
)
(rec
(type (;9;) (struct))
(type (;10;) (struct))
(type (;11;) (struct))
)
(rec
(type (;11;) (struct))
(type (;12;) (struct))
(type (;13;) (struct))
)
(rec
(type (;13;) (struct))
(type (;14;) (struct))
)
(type (;15;) (func (param (ref 5))))
(type (;16;) (func (param (ref 6))))
(type (;17;) (func (param (ref 7))))
(type (;18;) (func (param (ref 8))))
(type (;19;) (func (param (ref 9))))
(type (;20;) (func (param (ref 10))))
(type (;21;) (func (param (ref 11))))
(type (;22;) (func (param (ref 12))))
(type (;23;) (func (param (ref 13))))
(type (;24;) (func (param (ref 14))))
(import "" "gc" (func (;0;) (type 0)))
(import "" "take_refs" (func (;1;) (type 2)))
(import "" "make_refs" (func (;2;) (type 3)))
(import "" "take_struct" (func (;3;) (type 4)))
(import "" "take_struct_5" (func (;4;) (type 15)))
(import "" "take_struct_6" (func (;5;) (type 16)))
(import "" "take_struct_7" (func (;6;) (type 17)))
(import "" "take_struct_8" (func (;7;) (type 18)))
(import "" "take_struct_9" (func (;8;) (type 19)))
(import "" "take_struct_10" (func (;9;) (type 20)))
(import "" "take_struct_11" (func (;10;) (type 21)))
(import "" "take_struct_12" (func (;11;) (type 22)))
(import "" "take_struct_13" (func (;12;) (type 23)))
(import "" "take_struct_14" (func (;13;) (type 24)))
(table (;0;) 5 externref)
(global (;0;) (mut externref) ref.null extern)
(global (;1;) (mut externref) ref.null extern)
(export "run" (func 3))
(func (;3;) (type 1) (param externref externref)
(export "run" (func 14))
(func (;14;) (type 1) (param externref externref)
(local externref)
loop ;; label = @1
ref.null extern
Expand All @@ -828,8 +903,8 @@ mod tests {
local.get 0
global.set 0
global.get 0
struct.new 4
drop
struct.new 5
call 3
drop
drop
drop
Expand Down
32 changes: 32 additions & 0 deletions crates/fuzzing/src/oracles.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,38 @@ pub fn table_ops(
});
linker.define(&store, "", "make_refs", func).unwrap();

let func_ty = FuncType::new(
store.engine(),
vec![ValType::Ref(RefType::new(false, HeapType::Any))],
vec![],
);

let func = Func::new(&mut store, func_ty, {
move |_caller: Caller<'_, StoreLimits>, _params, _results| {
log::info!("table_ops: take_struct(<ref any>)");
Ok(())
}
});

linker.define(&store, "", "take_struct", func).unwrap();

for imp in module.imports() {
if imp.module() == "" {
let name = imp.name();
if name.starts_with("take_struct_") {
if let wasmtime::ExternType::Func(ft) = imp.ty() {
let imp_name = name.to_string();
let func =
Func::new(&mut store, ft.clone(), move |_caller, _params, _results| {
log::info!("table_ops: {imp_name}(<typed structref>)");
Ok(())
});
linker.define(&store, "", name, func).unwrap();
}
}
}
}

let instance = linker.instantiate(&mut store, &module).unwrap();
let run = instance.get_func(&mut store, "run").unwrap();

Expand Down