smite-ir/mutators: Add SpliceMutator#135
Conversation
| for instr in &self.splice.instructions { | ||
| let shifted_inputs: Vec<usize> = instr | ||
| .inputs | ||
| .iter() | ||
| .map(|&input| input + insert_idx) | ||
| .collect(); | ||
| builder.append(instr.operation.clone(), &shifted_inputs); |
There was a problem hiding this comment.
This logic is the only logical difference between this mutator and GeneratorInsertion. It could also be implemented as:
| for instr in &self.splice.instructions { | |
| let shifted_inputs: Vec<usize> = instr | |
| .inputs | |
| .iter() | |
| .map(|&input| input + insert_idx) | |
| .collect(); | |
| builder.append(instr.operation.clone(), &shifted_inputs); | |
| for instr in &self.splice.instructions { | |
| let mut inputs = vec![]; | |
| for var_type in instr.operation.input_types() { | |
| inputs.push(builder.pick_variable(var_type, rng)); | |
| } | |
| builder.append(instr.operation.clone(), &inputs); | |
| } |
which I guess would "wire it stronger" to the rest of the program, but I'm not sure if that's a worthwhile tradeoff for the simplicity of the current approach.
There was a problem hiding this comment.
It's an interesting idea, but I think the current simpler approach may be better. By doing pick_variable, we end up losing the specific values that the splice program used (which were selected as interesting after many random mutations), which probably hurts fuzzing effectiveness. We'd also need to figure out how to handle variable types for which pick_variable currently panics.
We also can get some of the same behavior already if InputSwapMutator is stacked after the splice.
There was a problem hiding this comment.
We'd also need to figure out how to handle variable types for which
pick_variablecurrently panics.
Would we? pick_variable() currently panics if we try to generate_fresh() a type that cannot be generated (messages, affine types, etc.).
If the corpus always has valid programs and every mutation preserves validity, won't both the input programs to SpliceMutator always be valid? That is, wouldn't every panicking type already have at least one matching candidate?
There was a problem hiding this comment.
Yes, right -- we only have panic issues with generate_fresh, which shouldn't be called in this case.
We should do an experiment with the following configs to evaluate: (1) baseline, (2) this mutator, (3) a pick_variable splice mutator, (4) this mutator + a pick_variable splice mutator.
| for instr in &self.splice.instructions { | ||
| let shifted_inputs: Vec<usize> = instr | ||
| .inputs | ||
| .iter() | ||
| .map(|&input| input + insert_idx) | ||
| .collect(); | ||
| builder.append(instr.operation.clone(), &shifted_inputs); |
There was a problem hiding this comment.
It's an interesting idea, but I think the current simpler approach may be better. By doing pick_variable, we end up losing the specific values that the splice program used (which were selected as interesting after many random mutations), which probably hurts fuzzing effectiveness. We'd also need to figure out how to handle variable types for which pick_variable currently panics.
We also can get some of the same behavior already if InputSwapMutator is stacked after the splice.
| } | ||
|
|
||
| // Insert the spliced program. | ||
| for instr in &self.splice.instructions { |
There was a problem hiding this comment.
I think it would be interesting to do an experiment where we compare full-program insertion with only inserting a random subset (prefix) of the spliced program.
Full-program insertion will tend to create longer (and slower) programs, so it's possible that prefix insertion actually performs better.
There was a problem hiding this comment.
I think it would be interesting to do an experiment where we compare full-program insertion with only inserting a random subset (prefix) of the spliced program.
Hmm, we're inserting the prefix (instead of a random slice) so that we don't have to worry about variable dependencies. Should be easily implementable but I'm not fully sold on the "performs better" part, although I agree that it is an interesting experiment.
I think a more interesting (and perhaps advanced) version of this would be FlowInsertionMutator that selects a random Act instruction in the spliced program, yanks out its entire lineage, and inserts it at a random point in the given program.
There was a problem hiding this comment.
I suspect a prefix insertion mutator and a flow insertion mutator will perform similarly in practice, since we have the DeadCodeEliminator that will also periodically do the dependency analysis and dead instruction removal that FlowInsertionMutator would. So either one would be interesting to evaluate IMO.
It may also be interesting to combine this FlowInsertionMutator idea with the pick_variable rewiring you suggested as an experiment.
morehouse
left a comment
There was a problem hiding this comment.
Code LGTM. This is ready for experimental evaluation.
| for instr in &self.splice.instructions { | ||
| let shifted_inputs: Vec<usize> = instr | ||
| .inputs | ||
| .iter() | ||
| .map(|&input| input + insert_idx) | ||
| .collect(); | ||
| builder.append(instr.operation.clone(), &shifted_inputs); |
There was a problem hiding this comment.
Yes, right -- we only have panic issues with generate_fresh, which shouldn't be called in this case.
We should do an experiment with the following configs to evaluate: (1) baseline, (2) this mutator, (3) a pick_variable splice mutator, (4) this mutator + a pick_variable splice mutator.
| } | ||
|
|
||
| // Insert the spliced program. | ||
| for instr in &self.splice.instructions { |
There was a problem hiding this comment.
I suspect a prefix insertion mutator and a flow insertion mutator will perform similarly in practice, since we have the DeadCodeEliminator that will also periodically do the dependency analysis and dead instruction removal that FlowInsertionMutator would. So either one would be interesting to evaluate IMO.
It may also be interesting to combine this FlowInsertionMutator idea with the pick_variable rewiring you suggested as an experiment.
| fn mutate_stacked(&mut self, program: &mut Program, splice: Option<Program>) { | ||
| self.last_sequence.clear(); | ||
| // Power-of-two stack count: 1, 2, 4, 8, or 16 mutations. | ||
| let stack = 1u32 << self.rng.random_range(0..=4); | ||
| let splice_mutator = splice.map(SpliceMutator::new); | ||
| // Only roll up to 6 if we actually have a splice input. | ||
| let upper_bound = if splice_mutator.is_some() { 6 } else { 5 }; | ||
| for _ in 0..stack { | ||
| // Uniform pick between the available mutators. | ||
| let name = match self.rng.random_range(0..5) { | ||
| let name = match self.rng.random_range(0..upper_bound) { | ||
| 0 => { | ||
| OperationParamMutator.mutate(program, &mut self.rng); | ||
| "op-param" | ||
| } | ||
| 1 => { | ||
| InputSwapMutator.mutate(program, &mut self.rng); | ||
| "input-swap" | ||
| } | ||
| 2 => { | ||
| InstructionDeleteMutator.mutate(program, &mut self.rng); | ||
| "instr-delete" | ||
| } | ||
| 3 => { | ||
| InstructionReorderMutator.mutate(program, &mut self.rng); | ||
| "instr-reorder" | ||
| } | ||
| 4 => { | ||
| let generator = *AnyGenerator::ALL | ||
| .iter() | ||
| .choose(&mut self.rng) | ||
| .expect("AnyGenerator::ALL is non-empty"); | ||
| let mutator = GeneratorInsertionMutator::new(generator); | ||
| mutator.mutate(program, &mut self.rng); | ||
| "gen-insert" | ||
| } | ||
| 5 => { | ||
| splice_mutator | ||
| .as_ref() | ||
| .expect("splice present") | ||
| .mutate(program, &mut self.rng); | ||
| "splice" | ||
| } | ||
| _ => unreachable!("random_range() bound out of sync with match arms"), | ||
| }; | ||
| self.last_sequence.push(name); | ||
| } | ||
| } |
There was a problem hiding this comment.
nit: We could refactor to avoid hardcoding numbers that would need changing if we added or removed mutators.
| /// Mutators available to [`MutatorState::mutate_stacked`]. The pool of | |
| /// candidates is built per call, so optional entries (splice) are included by | |
| /// pushing a variant. | |
| #[derive(Clone, Copy)] | |
| enum StackedMutator { | |
| OpParam, | |
| InputSwap, | |
| InstrDelete, | |
| InstrReorder, | |
| GenInsert, | |
| Splice, | |
| } | |
| fn mutate_stacked(&mut self, program: &mut Program, splice: Option<Program>) { | |
| self.last_sequence.clear(); | |
| // Power-of-two stack count: 1, 2, 4, 8, or 16 mutations. | |
| let stack = 1u32 << self.rng.random_range(0..=4); | |
| let splice_mutator = splice.map(SpliceMutator::new); | |
| let mut pool = vec![ | |
| StackedMutator::OpParam, | |
| StackedMutator::InputSwap, | |
| StackedMutator::InstrDelete, | |
| StackedMutator::InstrReorder, | |
| StackedMutator::GenInsert, | |
| ]; | |
| if splice_mutator.is_some() { | |
| pool.push(StackedMutator::Splice); | |
| } | |
| for _ in 0..stack { | |
| // Uniform pick between the available mutators. | |
| let choice = *pool | |
| .iter() | |
| .choose(&mut self.rng) | |
| .expect("pool is non-empty"); | |
| let name = match choice { | |
| StackedMutator::OpParam => { | |
| OperationParamMutator.mutate(program, &mut self.rng); | |
| "op-param" | |
| } | |
| StackedMutator::InputSwap => { | |
| InputSwapMutator.mutate(program, &mut self.rng); | |
| "input-swap" | |
| } | |
| StackedMutator::InstrDelete => { | |
| InstructionDeleteMutator.mutate(program, &mut self.rng); | |
| "instr-delete" | |
| } | |
| StackedMutator::InstrReorder => { | |
| InstructionReorderMutator.mutate(program, &mut self.rng); | |
| "instr-reorder" | |
| } | |
| StackedMutator::GenInsert => { | |
| let generator = *AnyGenerator::ALL | |
| .iter() | |
| .choose(&mut self.rng) | |
| .expect("AnyGenerator::ALL is non-empty"); | |
| let mutator = GeneratorInsertionMutator::new(generator); | |
| mutator.mutate(program, &mut self.rng); | |
| "gen-insert" | |
| } | |
| StackedMutator::Splice => { | |
| splice_mutator | |
| .as_ref() | |
| .expect("Splice is only pooled when a splice input exists") | |
| .mutate(program, &mut self.rng); | |
| "splice" | |
| } | |
| }; | |
| self.last_sequence.push(name); | |
| } | |
| } |
There was a problem hiding this comment.
So basically the AnyGenerator variant for mutators. It does improve readability, but I feel like the change doesn't fit well within this PR. I'll follow up with a separate one implementing it.
The following commit will implement `SpliceInsertionMutator`, which uses the decoded program from this buffer. Additionally, add a test to ensure parsing a valid `add_buf` via postcard doesn't panic or corrupt the state.
Refactor the common logic from `GeneratorInsertionMutator` tests that we will need for implementing `SpliceInsertionMutator` tests.
Add
SpliceMutatorfor smite-IR. Mutates a given program by inserting a spliced input at a random point in the said program.