-
-
Notifications
You must be signed in to change notification settings - Fork 324
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.
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
Deriving Mapping Mutators #2585
base: main
Are you sure you want to change the base?
Deriving Mapping Mutators #2585
Conversation
The current problem I'm running into is that the types of the combined mutators have to be specified in the macro — generics don't seem to work or at least I couldn't figure out how to do it. Current Error Logs
And this is just with a custom input containing a single Vec. It would be somewhat more doable if there was If anyone wants to suggest ideas or take a look at the current situation, please do! |
This is cool stuff! It might make sense to take a good look at how Arbitrary works for different types. Also, if we want to do this perfectly, we probably want different mutators for each type at some point. |
What are you suggesting with this? Instead of using mapping mutators just using a simple
For sure, that's the goal. I'm probably going to implement mutators for struct parts of types I hope to basically build a foundation with this, that should be fairly flexible and extensible for whatever else, basically defining default mutators for each type (or type combination). |
My point about Arbitrary is that they have derives for all possible types. |
Ah, that makes sense. Thank you for the suggestion! I'll see what I can do. |
use super::HasHavocMutators; | ||
use crate::mutators::{StdScheduledMutator, Vec}; | ||
|
||
#[derive(HasHavocMutators)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd just say #[derive(Mutators)]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about DefaultMutators
? Depending on what you're trying to accomplish, you may want different implementations.
1fb2c28
to
3f86aaf
Compare
So, merging Now I'm back to struggling with timelines: pub trait DefaultMutators {
type Type;
fn default_mutators() -> Self::Type;
}
#[derive(Debug, Deserialize, Serialize, SerdeAny, Clone)]
struct CustomInput {
vec: Vec<u8>,
}
impl CustomInput {
fn vec_mut(&mut self) -> MutVecInput<'_> {
(&mut self.vec).into()
}
fn vec(&self) -> &[u8] {
&self.vec
}
}
impl DefaultMutators for CustomInput {
type Type = MappedHavocMutationsType<
for<'a> fn(&'a mut CustomInput) -> MutVecInput<'a>,
fn(&CustomInput) -> &[u8],
MutVecInput<'a>,
&'a [u8],
>;
fn default_mutators() -> Self::Type {
mapped_havoc_mutations(CustomInput::vec_mut, CustomInput::vec)
}
} The issue here are the lifetimes for To ensure this, we introduced these weird traits that are nothing but a type with a lifetime (see If nobody (@domenukk?) has an idea of how to deal with this, I think this is the only way forward. |
I may have another idea, hold on. |
So I may have found a solution, but I needed to tear the entire way generics are handled in mapping mutators apart. I've also now introduced the following trait instead of /// Extensions of [`crate::inputs::Input`]s that have default mutators
pub trait DefaultMutators<S, MT>: Sized {
/// Get the default mutators for this type
fn default_mutators() -> MT
where
MT: MutatorsTuple<Self, S>,
S: HasCorpus,
<S as HasCorpus>::Corpus: Corpus<Input = Self>;
} There is also a proof of concept implementation for a custom input in test case in We're currently back at having to specify types for the outer mapping mutator interface, but I hope to fix this at some point. (We may get away with removing some of the generics, I'll have to check the compiler's opinion.) |
So I may have found a solution, but I needed to tear the entire way generics are handled in mapping mutators apart. I've also now introduced the following trait instead of /// Extensions of [`crate::inputs::Input`]s that have default mutators
pub trait DefaultMutators<S, MT>: Sized {
/// Get the default mutators for this type
fn default_mutators() -> MT
where
MT: MutatorsTuple<Self, S>,
S: HasCorpus,
<S as HasCorpus>::Corpus: Corpus<Input = Self>;
} There is also a proof of concept implementation for a custom input in We're currently back at having to specify types for the outer mapping mutator interface, but I hope to fix this at some point. |
|
@@ -42,7 +43,7 @@ use { | |||
/// Coverage map with explicit assignments due to the lack of instrumentation | |||
const SIGNALS_LEN: usize = 16; | |||
static mut SIGNALS: [u8; SIGNALS_LEN] = [0; 16]; | |||
static mut SIGNALS_PTR: *mut u8 = addr_of_mut!(SIGNALS) as _; | |||
static mut SIGNALS_PTR: *mut u8 = unsafe { addr_of_mut!(SIGNALS) as _ }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any chance you have an old compiler?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't think so. The unsafe
is not necessary when built with +nightly
, but it breaks without.
$ rustup --version
rustup 1.27.1 (54dd3d00f 2024-04-24)
info: This is the version for the rustup toolchain manager, not the rustc compiler.
info: The currently active `rustc` version is `rustc 1.81.0 (eeb90cda1 2024-09-04)`
$ cargo --version
cargo 1.81.0 (2dbb1af80 2024-08-20)
error[E0133]: use of mutable static is unsafe and requires unsafe function or block
--> src/main.rs:46:48
|
46 | static mut SIGNALS_PTR: *mut u8 = addr_of_mut!(SIGNALS) as _;
| ^^^^^^^ use of mutable static
|
= note: mutable statics can be mutated by multiple threads: aliasing violations or data races will cause undefined behavior
For more information about this error, try `rustc --explain E0133`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Odd, it doesn't do that for me
I get that for the standard implementations overall, independent of other types or parts of the fuzzer. In this case, I personally think it's closer to
I took out a lot of unnecessary constraints, including all on /// Extensions of [`crate::inputs::Input`]s that have default mutators
pub trait DefaultMutators {
/// The resulting mutator list type
type Type;
/// Get the default mutators for this type
#[must_use]
fn default_mutators() -> Self::Type;
} Regarding overwriting: You can always implement other mutators for your input type and/or combine them with a subset of existing ones. Also: I can't for the life of me figure out how to fix the typing such that users don't have to specify their types — the fix from last time doesn't work anymore because of the different generic architecture. But also: this is not an issue if you just use the |
Do you have an example where the mutators are derived? |
Not yet, no — that's the end goal of this PR. But I need something to derive first, so I'm writing the interface, then a manual implementation in a test case, and then I'll continue with the macro. |
2a14db9
to
4593747
Compare
4593747
to
9eea795
Compare
I'm attempting to derive everything necessary for havoc-style mutators on custom inputs using mapping mutators introduced in #2422.
Still to do:
Vec<u8>