Skip to content

Commit

Permalink
Remove Input associated type (#2854)
Browse files Browse the repository at this point in the history
* Completely remove Input as an associated type in multiple traits

* Unify usage of Input as generic instead 

* Remove many unused bounds, in particular HasCorpus

* fix multiple generic ordering

* update and fix CONTRIBUTING.md

* update MIGRATION

* use the same generic input type for new / with_max_iterations to make typing easier in most cases.

* Restore libafl_libfuzzer test in CI
  • Loading branch information
rmalmain authored Jan 17, 2025
1 parent d4add04 commit f8ad61e
Show file tree
Hide file tree
Showing 126 changed files with 1,578 additions and 1,958 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build_and_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ jobs:
- ./fuzzers/structure_aware/forkserver_simple_nautilus

# In-process
# - ./fuzzers/fuzz_anything/cargo_fuzz # Revive after they fix rustc
- ./fuzzers/fuzz_anything/cargo_fuzz
# - ./fuzzers/inprocess/dynamic_analysis
- ./fuzzers/inprocess/fuzzbench
- ./fuzzers/inprocess/fuzzbench_text
Expand Down
31 changes: 25 additions & 6 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,35 +27,54 @@ Before making your pull requests, try to see if your code follows these rules.
- `PhantomData` should have the smallest set of types needed. Try not adding `PhantomData` to your struct unless it is really necessary. Also even when you really need `PhantomData`, try to keep the types `T` used in `PhantomData` as smallest as possible
- Wherever possible, trait implementations with lifetime specifiers should use '_ lifetime elision.
- Complex constructors should be replaced with `typed_builder`, or write code in the builder pattern for yourself.
- Remove generic restrictions at the definitions (e.g., we do not need to specify that types impl `Serialize`, `Deserialize`, or `Debug` anymore at the struct definitions). Therefore, try avoiding code like this unless the contraint is really necessary.
- Remove generic restrictions at the definitions (e.g., we do not need to specify that types impl `Serialize`, `Deserialize`, or `Debug` anymore at the struct definitions). Therefore, try avoiding code like this unless the constraint is really necessary.
```rust
pub struct X<A>
where
A: P // <- Do not add contraints here
{
fn ...
}

```
- Reduce generics to the least restrictive necessary. __Never overspecify the contraints__. There's no automated tool to check the useless constraints, so you have to verify this manually.
- Reduce generics to the least restrictive necessary. __Never overspecify the constraints__. There's no automated tool to check the useless constraints, so you have to verify this manually.
```rust
pub struct X<A>
where
A: P + Q // <- Try to use the as smallest set of constraints as possible. If the code still compiles after deleting Q, then remove it.
{
fn ...
}
```

- Prefer generic to associated types in traits definition as much as possible. They are much easier to use around, and avoid tricky caveats / type repetition in the code. It is also much easier to have unconstrained struct definitions.

Try not to write this:
```rust
pub trait X
{
type A;

fn a(&self) -> Self::A;
}
```
- Traits which have an associated type should refer to the associated type, not the concrete/generic. In other words, you should only have the associated type when you can define a getter to it. For example, in the following code, you can define a associate type.
Try to write this instead:
```rust
pub trait X
pub trait X<A>
{
type A; // <- You should(can) define it as long as you have a getter to it.
fn a(&self) -> A;
}
```

- Traits which have an associated type (if you have made sure you cannot use a generic instead) should refer to the associated type, not the concrete/generic. In other words, you should only have the associated type when you can define a getter to it. For example, in the following code, you can define a associate type.
```rust
pub trait X
{
type A; // <- You should(can) define it as long as you have a getter to it.

fn a(&self) -> Self::A;
}
```

- __Ideally__ the types used in the the arguments of methods in traits should have the same as the types defined on the traits.
```rust
pub trait X<A, B, C> // <- this trait have 3 generics, A, B, and C
Expand Down
4 changes: 3 additions & 1 deletion MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
- Trait restrictions have been simplified
- The `UsesState` and `UsesInput` traits have been removed in favor of regular Generics.
- For the structs/traits that used to use `UsesState`, we bring back the generic for the state.
- For `UsesState`, you can access to the input type through `HasCorpus` and `Corpus` traits
- `Input` is now only accessible through generic. `Input` associated types have been definitely removed.
- `HasCorpus` bound has been removed in many places it was unused before.
- `StdMutationalStage::transforming` must now explicitly state the Inputs types. As a result, `StdMutationalStage::transforming` must be written `StdMutationalStage::<_, _, FirstInputType, SecondInputType, _, _, _>::transforming`.
- The `State` trait is now private in favour of individual and more specific traits
- Restrictions from certain schedulers and stages that required their inner observer to implement `MapObserver` have been lifted in favor of requiring `Hash`
- Related: removed `hash_simple` from `MapObserver`
Expand Down
8 changes: 4 additions & 4 deletions fuzzers/baby/baby_fuzzer_custom_executor/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,17 @@ impl<S> CustomExecutor<S> {
}
}

impl<EM, S, Z> Executor<EM, <S::Corpus as Corpus>::Input, S, Z> for CustomExecutor<S>
impl<EM, I, S, Z> Executor<EM, I, S, Z> for CustomExecutor<S>
where
S: HasCorpus + HasExecutions,
<S::Corpus as Corpus>::Input: HasTargetBytes,
S: HasCorpus<I> + HasExecutions,
I: HasTargetBytes,
{
fn run_target(
&mut self,
_fuzzer: &mut Z,
state: &mut S,
_mgr: &mut EM,
input: &<S::Corpus as Corpus>::Input,
input: &I,
) -> Result<ExitKind, libafl::Error> {
// We need to keep track of the exec count.
*state.executions_mut() += 1;
Expand Down
7 changes: 5 additions & 2 deletions fuzzers/baby/baby_fuzzer_unicode/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ use libafl::{
feedbacks::{CrashFeedback, MaxMapFeedback},
fuzzer::{Fuzzer, StdFuzzer},
inputs::{BytesInput, HasTargetBytes},
mutators::{StdScheduledMutator, UnicodeCategoryRandMutator, UnicodeSubcategoryRandMutator},
mutators::{
StdScheduledMutator, UnicodeCategoryRandMutator, UnicodeInput,
UnicodeSubcategoryRandMutator,
},
observers::StdMapObserver,
schedulers::QueueScheduler,
stages::{mutational::StdMutationalStage, UnicodeIdentificationStage},
Expand Down Expand Up @@ -134,7 +137,7 @@ pub fn main() {
));
let mut stages = tuple_list!(
UnicodeIdentificationStage::new(),
StdMutationalStage::transforming(mutator)
StdMutationalStage::<_, _, UnicodeInput, BytesInput, _, _, _>::transforming(mutator)
);

fuzzer
Expand Down
16 changes: 6 additions & 10 deletions fuzzers/baby/tutorial/src/metadata.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
use std::borrow::Cow;

use libafl::{
corpus::{Corpus, Testcase},
corpus::Testcase,
executors::ExitKind,
feedbacks::{Feedback, MapIndexesMetadata, StateInitializer},
schedulers::{MinimizerScheduler, TestcaseScore},
state::HasCorpus,
Error, HasMetadata,
};
use libafl_bolts::{Named, SerdeAny};
Expand All @@ -20,23 +19,20 @@ pub struct PacketLenMetadata {

pub struct PacketLenTestcaseScore {}

impl<S> TestcaseScore<S> for PacketLenTestcaseScore
impl<I, S> TestcaseScore<I, S> for PacketLenTestcaseScore
where
S: HasMetadata + HasCorpus,
S: HasMetadata,
{
fn compute(
_state: &S,
entry: &mut Testcase<<S::Corpus as Corpus>::Input>,
) -> Result<f64, Error> {
fn compute(_state: &S, entry: &mut Testcase<I>) -> Result<f64, Error> {
Ok(entry
.metadata_map()
.get::<PacketLenMetadata>()
.map_or(1, |m| m.length) as f64)
}
}

pub type PacketLenMinimizerScheduler<CS, S> =
MinimizerScheduler<CS, PacketLenTestcaseScore, MapIndexesMetadata, S>;
pub type PacketLenMinimizerScheduler<CS, I, S> =
MinimizerScheduler<CS, PacketLenTestcaseScore, I, MapIndexesMetadata, S>;

#[derive(Serialize, Deserialize, Default, Clone, Debug)]
pub struct PacketLenFeedback {
Expand Down
8 changes: 4 additions & 4 deletions fuzzers/binary_only/frida_executable_libpng/src/fuzzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,15 @@ unsafe fn fuzz(
let shmem_provider = StdShMemProvider::new()?;

let mut run_client = |state: Option<_>,
mgr: LlmpRestartingEventManager<_, _, _>,
mgr: LlmpRestartingEventManager<_, _, _, _>,
client_description: ClientDescription| {
// The restarting state will spawn the same process again as child, then restarted it each time it crashes.

// println!("{:?}", mgr.mgr_id());

if options.asan && options.asan_cores.contains(client_description.core_id()) {
(|state: Option<_>,
mut mgr: LlmpRestartingEventManager<_, _, _>,
mut mgr: LlmpRestartingEventManager<_, _, _, _>,
_client_description| {
let gum = Gum::obtain();

Expand Down Expand Up @@ -231,7 +231,7 @@ unsafe fn fuzz(
})(state, mgr, client_description)
} else if options.cmplog && options.cmplog_cores.contains(client_description.core_id()) {
(|state: Option<_>,
mut mgr: LlmpRestartingEventManager<_, _, _>,
mut mgr: LlmpRestartingEventManager<_, _, _, _>,
_client_description| {
let gum = Gum::obtain();

Expand Down Expand Up @@ -367,7 +367,7 @@ unsafe fn fuzz(
})(state, mgr, client_description)
} else {
(|state: Option<_>,
mut mgr: LlmpRestartingEventManager<_, _, _>,
mut mgr: LlmpRestartingEventManager<_, _, _, _>,
_client_description| {
let gum = Gum::obtain();

Expand Down
6 changes: 4 additions & 2 deletions fuzzers/binary_only/frida_libpng/src/fuzzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ unsafe fn fuzz(options: &FuzzerOptions) -> Result<(), Error> {
};

let mut run_client = |state: Option<_>,
mgr: LlmpRestartingEventManager<_, _, _>,
mgr: LlmpRestartingEventManager<_, _, _, _>,
client_description: ClientDescription| {
// The restarting state will spawn the same process again as child, then restarted it each time it crashes.

Expand All @@ -100,7 +100,9 @@ unsafe fn fuzz(options: &FuzzerOptions) -> Result<(), Error> {
};

// if options.asan && options.asan_cores.contains(client_description.core_id()) {
(|state: Option<_>, mut mgr: LlmpRestartingEventManager<_, _, _>, _client_description| {
(|state: Option<_>,
mut mgr: LlmpRestartingEventManager<_, _, _, _>,
_client_description| {
let gum = Gum::obtain();

let coverage = CoverageRuntime::new();
Expand Down
8 changes: 4 additions & 4 deletions fuzzers/binary_only/frida_windows_gdiplus/src/fuzzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ unsafe fn fuzz(options: &FuzzerOptions) -> Result<(), Error> {
let shmem_provider = StdShMemProvider::new()?;

let mut run_client = |state: Option<_>,
mgr: LlmpRestartingEventManager<_, _, _>,
mgr: LlmpRestartingEventManager<_, _, _, _>,
client_description: ClientDescription| {
// The restarting state will spawn the same process again as child, then restarted it each time it crashes.

Expand All @@ -98,7 +98,7 @@ unsafe fn fuzz(options: &FuzzerOptions) -> Result<(), Error> {

if options.asan && options.asan_cores.contains(client_description.core_id()) {
(|state: Option<_>,
mut mgr: LlmpRestartingEventManager<_, _, _>,
mut mgr: LlmpRestartingEventManager<_, _, _, _>,
_client_description| {
let gum = Gum::obtain();

Expand Down Expand Up @@ -214,7 +214,7 @@ unsafe fn fuzz(options: &FuzzerOptions) -> Result<(), Error> {
})(state, mgr, client_description)
} else if options.cmplog && options.cmplog_cores.contains(client_description.core_id()) {
(|state: Option<_>,
mut mgr: LlmpRestartingEventManager<_, _, _>,
mut mgr: LlmpRestartingEventManager<_, _, _, _>,
_client_description| {
let gum = Gum::obtain();

Expand Down Expand Up @@ -344,7 +344,7 @@ unsafe fn fuzz(options: &FuzzerOptions) -> Result<(), Error> {
})(state, mgr, client_description)
} else {
(|state: Option<_>,
mut mgr: LlmpRestartingEventManager<_, _, _>,
mut mgr: LlmpRestartingEventManager<_, _, _, _>,
_client_description| {
let gum = Gum::obtain();

Expand Down
7 changes: 3 additions & 4 deletions fuzzers/binary_only/qemu_coverage/src/fuzzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ use clap::{builder::Str, Parser};
use libafl::{
corpus::{Corpus, InMemoryCorpus},
events::{
launcher::Launcher, ClientDescription, EventConfig, EventRestarter,
LlmpRestartingEventManager, ManagerExit,
launcher::Launcher, ClientDescription, EventConfig, LlmpRestartingEventManager, ManagerExit,
},
executors::ExitKind,
fuzzer::StdFuzzer,
Expand All @@ -31,7 +30,7 @@ use libafl_bolts::{
use libafl_qemu::{
elf::EasyElf,
modules::{drcov::DrCovModule, utils::filters::StdAddressFilter},
ArchExtras, CallingConvention, Emulator, GuestAddr, GuestReg, MmapPerms, Qemu, QemuExecutor,
ArchExtras, CallingConvention, Emulator, GuestAddr, GuestReg, MmapPerms, QemuExecutor,
QemuExitReason, QemuRWError, QemuShutdownCause, Regs,
};

Expand Down Expand Up @@ -124,7 +123,7 @@ pub fn fuzz() {
env::remove_var("LD_LIBRARY_PATH");

let mut run_client = |state: Option<_>,
mut mgr: LlmpRestartingEventManager<_, _, _>,
mut mgr: LlmpRestartingEventManager<_, _, _, _>,
client_description: ClientDescription| {
let mut cov_path = options.coverage_path.clone();

Expand Down
2 changes: 1 addition & 1 deletion fuzzers/binary_only/qemu_launcher/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use crate::{

#[expect(clippy::module_name_repetitions)]
pub type ClientState =
StdState<BytesInput, InMemoryOnDiskCorpus<BytesInput>, StdRand, OnDiskCorpus<BytesInput>>;
StdState<InMemoryOnDiskCorpus<BytesInput>, BytesInput, StdRand, OnDiskCorpus<BytesInput>>;

pub struct Client<'a> {
options: &'a FuzzerOptions,
Expand Down
10 changes: 6 additions & 4 deletions fuzzers/binary_only/qemu_launcher/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,15 @@ use typed_builder::TypedBuilder;
use crate::{harness::Harness, options::FuzzerOptions};

pub type ClientState =
StdState<BytesInput, InMemoryOnDiskCorpus<BytesInput>, StdRand, OnDiskCorpus<BytesInput>>;
StdState<InMemoryOnDiskCorpus<BytesInput>, BytesInput, StdRand, OnDiskCorpus<BytesInput>>;

#[cfg(feature = "simplemgr")]
pub type ClientMgr<M> = SimpleEventManager<M, ClientState>;
#[cfg(not(feature = "simplemgr"))]
pub type ClientMgr<M> =
MonitorTypedEventManager<LlmpRestartingEventManager<(), ClientState, StdShMemProvider>, M>;
pub type ClientMgr<M> = MonitorTypedEventManager<
LlmpRestartingEventManager<(), BytesInput, ClientState, StdShMemProvider>,
M,
>;

#[derive(TypedBuilder)]
pub struct Instance<'a, M: Monitor> {
Expand Down Expand Up @@ -321,7 +323,7 @@ impl<M: Monitor> Instance<'_, M> {
stages: &mut ST,
) -> Result<(), Error>
where
Z: Fuzzer<E, ClientMgr<M>, ClientState, ST>
Z: Fuzzer<E, ClientMgr<M>, BytesInput, ClientState, ST>
+ Evaluator<E, ClientMgr<M>, BytesInput, ClientState>,
ST: StagesTuple<E, ClientMgr<M>, ClientState, Z>,
{
Expand Down
2 changes: 1 addition & 1 deletion fuzzers/forkserver/fuzzbench_forkserver_cmplog/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ fn fuzz(

let cb = |_fuzzer: &mut _,
_executor: &mut _,
state: &mut StdState<_, InMemoryOnDiskCorpus<_>, _, _>,
state: &mut StdState<InMemoryOnDiskCorpus<_>, _, _, _>,
_event_manager: &mut _|
-> Result<bool, Error> {
let testcase = state.current_testcase()?;
Expand Down
Loading

0 comments on commit f8ad61e

Please sign in to comment.