Skip to content
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

Allow for AsIter(Mut)/AsSlice(Mut) to be implemented in safe Rust #2120

Merged
merged 17 commits into from
Apr 27, 2024

Conversation

langston-barrett
Copy link
Contributor

Redux of #2000.

Previously, MapFeedback required the Observer to implement AsIter, and to give out references via .get and .get_mut. This works for all the existing in-repo MapObservers, which use unsafe to pretend that they own the underlying map/vector/array. This lets them loan out items in the array with the same lifetime as &self.

This wasn't previously possible for observers using RefCell, as the loan of the content of a RefCell by definition has a shorter lifetime than the RefCell itself. With the changes here, we can have a MapObserver built in safe Rust, i.e., RefCellValueObserver<Vec<...>>

MapFeedback already requires that all the values are Copy, and in practice, they're generally small (in fact, they're generally primitive integers, meaning copying is free). The changes in this PR take advantage of this to remove borrowing values from the interface, thus enabling implementation of MapObservers in safe Rust.

We may want to make similar changes for AsIterMut, although it doesn't appear to be necessary for this particular use-case.

These modifications to the LibAFL library were made under funding provided by the U.S. government. The work represented in this PR is provided under Distribution Statement “A” (Approved for Public Release, Distribution Unlimited.)

@langston-barrett langston-barrett force-pushed the lb/safe-map-observer branch 4 times, most recently from b4be1a9 to 6f3d9e0 Compare April 26, 2024 20:48
`Self::Entry` is `Copy`, so there's not much value in returning a
reference from `get()`. Futhermore, returning a reference limits the
possible implementations of `MapObserver`, because it forces the
borrow/reset to outlive the body of the method.
Like the previous commit, this is intended to expand the possible
implementations of `MapObserver` to types with interior mutability,
which can't necessarily loan out their content.
@addisoncrump
Copy link
Collaborator

I recognise fighting CI is no fun, so if at any point you want me to take over, let me know 🙂

@langston-barrett
Copy link
Contributor Author

@addisoncrump That'd be awesome! I'm done for the day, I can return to this on Tuesday if you don't have a minute to push it through before then.

@addisoncrump
Copy link
Collaborator

Also working on AsSlice/AsSliceMut for this pattern as well, since we very likely want to be able to use e.g. SIMD accel on the RefCell maps.

@addisoncrump addisoncrump changed the title impl MapObserver for RefCellValueObserver in safe Rust Allos for AsIter(Mut)/AsSlice(Mut) to be implemented in safe Rust Apr 27, 2024
@addisoncrump addisoncrump changed the title Allos for AsIter(Mut)/AsSlice(Mut) to be implemented in safe Rust Allow for AsIter(Mut)/AsSlice(Mut) to be implemented in safe Rust Apr 27, 2024
@addisoncrump addisoncrump marked this pull request as ready for review April 27, 2024 15:55
@addisoncrump addisoncrump merged commit b024846 into AFLplusplus:main Apr 27, 2024
100 checks passed
@langston-barrett langston-barrett deleted the lb/safe-map-observer branch April 27, 2024 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants