Add intial porting of table_ops from arbitrary to mutatis #10924
Conversation
Subscribe to Label Actioncc @fitzgen
This issue or pull request has been labeled: "fuzzing"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
There was a problem hiding this comment.
This is on the right track!
You can rebase on main to get the cargo vet audits for mutatis so that that part of CI stops failing.
Also make sure to check that the actual fuzz target that runs this stuff continues to build via
$ cargo fuzz check --no-default-features -s none misc
You probably need to fix some stuff up in there and maybe hook up a custom mutator for libfuzzer, as described here: https://docs.rs/mutatis/latest/mutatis/_guide/fuzzer_integration/index.html
| /// Computes the abstract stack depth after executing all operations | ||
| pub fn abstract_stack_depth(&self) -> usize { | ||
| let mut stack: usize = 0; | ||
| for op in &self.ops { | ||
| let pop = op.operands_len(); | ||
| let push = op.results_len(); | ||
| stack = stack.saturating_sub(pop); | ||
| stack += push; | ||
| } | ||
| stack | ||
| } |
There was a problem hiding this comment.
I think we won't need this helper anymore, given the fixup helper function I suggested in private communication.
| // 1. Append | ||
| c.mutation(|ctx| { | ||
| let mut stack = ops.abstract_stack_depth(); | ||
| add_table_op_mutatis(ops, ctx, &mut stack) | ||
| })?; |
There was a problem hiding this comment.
And this can become an arbitrary insert instead of append (which is a subset of an insert operation).
There was a problem hiding this comment.
And the insert mutation would be something like
c.mutation(|ctx| {
let i = ctx.rng().gen_index(ops.ops.len() + 1).unwrap();
ops.ops.insert(random_op());
Ok(())
})?;| // 3. Replace | ||
| c.mutation(|ctx| { |
There was a problem hiding this comment.
Similarly, I think this function body will get cleaned up a ton with the fixup pass. Additionally, whenever possible we don't want to report mutations that we know will fail so if the ops are empty we shouldn't call c.mutation(...). Putting these things together, I expect this will become something like:
if !ops.ops.is_empty() {
c.mutation(|ctx| {
let i = ctx.rng().gen_index(ops.ops.len()).unwrap();
ops.ops[i] = choose_random_op();
Ok(())
})?;
}| // 2. Remove | ||
| c.mutation(|ctx| { | ||
| if ops.ops.is_empty() { | ||
| return Ok(()); | ||
| } | ||
| if let Some(idx) = ctx.rng().gen_index(ops.ops.len()) { | ||
| let removed = ops.ops.remove(idx); | ||
| log::debug!("Remove called at idx: {} with opcode {:?}", idx, removed); | ||
| let mut stack = 0isize; | ||
| for op in &ops.ops[..idx] { | ||
| stack += op.results_len() as isize; | ||
| stack -= op.operands_len() as isize; | ||
| } | ||
| stack -= removed.results_len() as isize; | ||
| stack += removed.operands_len() as isize; | ||
|
|
||
| log::debug!( | ||
| "op = {:?}, pop = {}, push = {}, resulting stack = {}", | ||
| removed, | ||
| removed.operands_len(), | ||
| removed.results_len(), | ||
| stack | ||
| ); | ||
| for _ in 0..removed.results_len() { | ||
| ops.ops.insert(idx, TableOp::Null()); | ||
| log::debug!( | ||
| "Patched with ref.null at idx {} after removing {:?}", | ||
| idx, | ||
| removed | ||
| ); | ||
| stack += 1; | ||
| } | ||
| } | ||
| Ok(()) | ||
| })?; |
There was a problem hiding this comment.
And this can become something like
if !ops.ops.is_empty() {
c.mutation(|ctx| {
let i = ctx.rng().gen_index(ops.ops.len()).unwrap();
ops.ops.remove(i);
Ok(())
})?;
}There was a problem hiding this comment.
Yes! I am working on that and a fixup method results in much cleaner code.
| }; | ||
| while ops.len() < MAX_OPS { | ||
| temp_ops.ops = ops.clone(); | ||
| let mut stack = temp_ops.abstract_stack_depth(); |
There was a problem hiding this comment.
This is accidentally O(n*n) because the helper walks every op in every iteration of this loop. You'll want to have a local stack variable that is adjusted as ops are generated here.
| macro_rules! empty_table_ops { | ||
| ($num_params:expr, $num_globals:expr, $table_size:expr) => { | ||
| TableOps { | ||
| num_params: $num_params, | ||
| num_globals: $num_globals, | ||
| table_size: $table_size, | ||
| ops: vec![], | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
This macro should just be TableOps::default(), which can simply be derived on the type rather than written out.
There was a problem hiding this comment.
Added new empty() method
| use std::ops::RangeInclusive; | ||
| use wasm_encoder::{ | ||
| CodeSection, ConstExpr, EntityType, ExportKind, ExportSection, Function, FunctionSection, | ||
| GlobalSection, ImportSection, Instruction, Module, RefType, TableSection, TableType, | ||
| TypeSection, ValType, | ||
| }; | ||
|
|
||
| /// A description of a Wasm module that makes a series of `externref` table | ||
| /// operations. | ||
| #[derive(Debug)] |
There was a problem hiding this comment.
Here we would add Default next to Debug.
| macro_rules! default_table_ops { | ||
| ($num_params:expr, $num_globals:expr, $table_size:expr) => { | ||
| TableOps { | ||
| num_params: $num_params, | ||
| num_globals: $num_globals, | ||
| table_size: $table_size, | ||
| ops: vec![ | ||
| TableOp::Null(), | ||
| TableOp::Drop(), | ||
| TableOp::Gc(), | ||
| TableOp::LocalSet(0), | ||
| TableOp::LocalGet(0), | ||
| TableOp::GlobalSet(0), | ||
| TableOp::GlobalGet(0), | ||
| ], | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
I don't think we should need this, and can simply use an empty TableOps::default() instead, but if we did want to create this particular sequence in a bunch of places we should just use a function rather than a macro.
There was a problem hiding this comment.
added new methods instead of macros
| #[test] | ||
| fn test_tableops_mutate_with() -> mutatis::Result<()> { | ||
| let _ = env_logger::try_init(); | ||
| let mut res = default_table_ops![5, 5, 5]; | ||
| let mut generator = TableOpsMutator; | ||
| let mut session = mutatis::Session::new(); | ||
|
|
||
| for _ in 0..=1024 { | ||
| session.mutate_with(&mut generator, &mut res)?; |
There was a problem hiding this comment.
Because the TableOpsMutator is the default, we don't actually need this test; it is redundant with mutate_table_ops_with_default_mutator.
| assert!(unseen_ops.is_empty(), "Failed to generate {unseen_ops:?}"); | ||
| Ok(()) | ||
| } | ||
| // TODO: Write a test for expected wat |
|
Thanks, @fitzgen! currently working on it. Will integrate your suggestions in the next commit. |
khagankhan
force-pushed
the
initial-mutatis
branch
from
56496e8 to
b4bb440
Compare
…on. Add new test for checking every generated opcode
khagankhan
force-pushed
the
initial-mutatis
branch
from
56c9c96 to
79674ad
Compare
This is the initial (unofficial) porting of
table_opsfromarbitrarytomutatis.No new features are added. This is a direct port of the existing implementation using the
arbitrarycrate to themutatisframework.Open to review and feedback.
Initial work towards: #10327
cc @ospencer @fitzgen