Implement `Hash` for MapObserver #1989

edwin1729 · 2024-04-01T15:44:38Z

Rename the hash utility function (in MapObserver) to hash_easy
Use hash_slice as a helper function to impl Hash trait

* Rename the hash utility function (in MapObserver) to hash_easy * Use hash_slice as a helper function to impl Hash trait

edwin1729 · 2024-04-01T15:53:07Z

I've also tried implement Hash for libafl_bolts::ownedref::OwnedMutSlice to delete hash_slice. However OwnedMapObserver::map is Vec<T> not OwnedMutSlice<T>, which prevents removal of hash_slice helper in a clean way.

domenukk · 2024-04-02T08:24:35Z

@addisoncrump want to take a look?

domenukk · 2024-04-02T08:27:20Z

I'm not a big fan of hash_easy, maybe rename to _simple to fit the rest of the names in the codebase. Also, do we really still need it?
there's a bunch of things that don't implement Hash yet, such as this guy:

error[E0277]: the trait bound `observers::map::pybind::PythonMapObserverI8: core::hash::Hash` is not satisfied
    --> libafl/src/observers/map.rs:2720:34
     |
2501 | /     macro_rules! define_python_map_observer {
2502 | |         ($struct_name1:ident, $py_name1:tt, $struct_name2:ident, $py_name2:tt, $struct_name_trait:ident, $py_name_trait:tt, $datatype:ty,...
2503 | |
2504 | |             #[pyclass(unsendable, name = $py_name1)]
...    |
2720 | |             impl MapObserver for $struct_name_trait {
     | |                                  ^^^^^^^^^^^^^^^^^^ the trait `core::hash::Hash` is not implemented for `observers::map::pybind::PythonMapObserverI8`
...    |
2794 | |         };
2795 | |     }
     | |_____- in this expansion of `define_python_map_observer!`
2796 |
2797 | /     define_python_map_observer!(
2798 | |         PythonStdMapObserverI8,
2799 | |         "StdMapObserverI8",
2800 | |         PythonOwnedMapObserverI8,
...    |
2805 | |         PythonMapObserverWrapperI8
2806 | |     );
     | |_____- in this macro invocation
     |
note: required by a bound in `observers::map::MapObserver`
    --> libafl/src/observers/map.rs:85:83
     |
85   | pub trait MapObserver: HasLen + Named + Serialize + serde::de::DeserializeOwned + Hash
     |                                                                                   ^^^^ required by this bound in `MapObserver`

addisoncrump · 2024-04-02T08:28:44Z

libafl/src/observers/map.rs

@@ -70,8 +70,7 @@ fn init_count_class_16() {
 }

 /// Compute the hash of a slice
-fn hash_slice<T>(slice: &[T]) -> u64 {
-    let mut hasher = RandomState::with_seeds(0, 0, 0, 0).build_hasher();
+fn hash_slice<T, H: Hasher>(slice: &[T], hasher: &mut H) -> u64 {


This entire function may replaced with .as_slice().hash(&mut hasher), and should be deleted.

hash_slice should be renamed to hash_helper. Its used to define the hash function. I can't see how to remove it.

@addisoncrump any last words on this? Otherwise we'll merge

It can be removed. One sec.

* Also rename hash_easy to hash_simple

edwin1729 · 2024-04-07T14:16:36Z

Isn't there a way to trigger the CI pipeline? I can't get clippy to work, because of python dependency errors (specifically sphinx)

* hash_helper is used to define the implementation of hash function/trait

tokatoka · 2024-04-07T15:46:11Z

Again! python ..🤦
can you merge from main? i removed the python stuff. now you don't have to worry about it.

tokatoka · 2024-04-07T15:47:23Z

if you make this PR ready for review. then CI will run. else it won't

domenukk · 2024-04-12T12:28:01Z

There are still some Errors in CI such as

error[E0407]: method `hash` is not a member of trait `MapObserver`
   --> libafl_targets/src/sancov_8bit.rs:199:9
    |
199 | /         fn hash(&self) -> u64 {
200 | |             let mut hasher = RandomState::with_seeds(0, 0, 0, 0).build_hasher();
201 | |             for map in unsafe { &*addr_of!(COUNTERS_MAPS) } {
202 | |                 let slice = map.as_slice();
...   |
209 | |             hasher.finish()
210 | |         }
    | |_________^ not a member of trait `MapObserver`

…FLplusplus#1977)

edwin1729 · 2024-04-17T05:43:40Z

Hi, I was wondering if the commit messages are alright. I thought maybe a rebase might be necessary

* Use hash_one function to make hash_simple a one-liner

domenukk · 2024-04-17T10:57:46Z

libafl_targets/src/sancov_8bit.rs

-            hasher.finish()
+        #[inline]
+        fn hash_simple(&self) -> u64 {
+            RandomState::with_seeds(0, 0, 0, 0).hash_one(self)


We could use the hash_std method here that can use xxh3 (but probably we don't need to)

LibAFL/libafl_bolts/src/lib.rs

Line 283 in 8bce605

pub fn hash_std(input: &[u8]) -> u64 {

Why does this method even exist? We should just have a type definition for StdHash or something, probably.

Not even sure if it implements any rust traits tbh

hash_simple does not implement any rust traits. If hash_std is okay then can use that instead. I don't really like it but Hash trait in rust doesn't seem to provide any default hashing function, so I made this

Generally, we don't actually want a default hashing function. This should be specified by the user, which is why we wanted the Hash impl (as this allows us to use a generic Hasher).

addisoncrump · 2024-04-18T09:29:22Z

@domenukk I think this is good to go now. Please review latest changes and then merge it up when you're satisfied with it.

domenukk · 2024-04-19T12:02:00Z

Happy if you are. Thanks y'all :)

addisoncrump · 2024-04-19T12:03:59Z

I'll merge this when CI finishes, then 🙂

MapObserver implements Hash

e450ab7

* Rename the hash utility function (in MapObserver) to hash_easy * Use hash_slice as a helper function to impl Hash trait

edwin1729 mentioned this pull request Apr 1, 2024

MapObserver should require Hash, not define a hash function #1977

Closed

domenukk requested a review from addisoncrump April 2, 2024 08:27

addisoncrump reviewed Apr 2, 2024

View reviewed changes

edwin1729 marked this pull request as draft April 6, 2024 13:51

edwin1729 marked this pull request as ready for review April 7, 2024 13:02

edwin1729 marked this pull request as draft April 7, 2024 13:02

define_python_map_observer macro implements Hash trait

5a53b75

* Also rename hash_easy to hash_simple

edwin1729 marked this pull request as ready for review April 7, 2024 14:13

edwin1729 marked this pull request as draft April 7, 2024 14:14

Rename hash_slice to hash_helper

d17012c

* hash_helper is used to define the implementation of hash function/trait

tokatoka marked this pull request as ready for review April 7, 2024 15:46

Merge branch 'main' of github.com:AFLplusplus/LibAFL into main

859a2df

edwin1729 added 2 commits April 15, 2024 11:37

Factor out the Hash trait and function for runtime library structs (A…

8128b7b

…FLplusplus#1977)

Merge remote-tracking branch 'upstream/main' into main

4e6591e

domenukk requested a review from addisoncrump April 17, 2024 01:47

Simplify hash_simple (of trait MapObserver) (AFLplusplus#1977)

054bb40

* Use hash_one function to make hash_simple a one-liner

domenukk reviewed Apr 17, 2024

View reviewed changes

domenukk changed the title ~~MapObserver implements Hash~~ Change MapObserver to implement Hash Apr 17, 2024

domenukk changed the title ~~Change MapObserver to implement Hash~~ Implement Hash for MapObserver Apr 17, 2024

addisoncrump added 2 commits April 17, 2024 16:11

remove hash_helper

3f67786

Merge branch 'main' into main

24af9ca

Merge branch 'main' into main

866b0e8

addisoncrump merged commit c238b69 into AFLplusplus:main Apr 19, 2024
78 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `Hash` for MapObserver #1989

Implement `Hash` for MapObserver #1989

edwin1729 commented Apr 1, 2024

edwin1729 commented Apr 1, 2024

domenukk commented Apr 2, 2024

domenukk commented Apr 2, 2024

addisoncrump Apr 2, 2024

edwin1729 Apr 7, 2024

domenukk Apr 17, 2024

addisoncrump Apr 17, 2024

edwin1729 commented Apr 7, 2024 •

edited

Loading

tokatoka commented Apr 7, 2024

tokatoka commented Apr 7, 2024

domenukk commented Apr 12, 2024

edwin1729 commented Apr 17, 2024 via email •

edited

Loading

domenukk Apr 17, 2024

addisoncrump Apr 17, 2024

domenukk Apr 19, 2024

edwin1729 Apr 19, 2024

addisoncrump Apr 19, 2024

addisoncrump commented Apr 18, 2024

domenukk commented Apr 19, 2024

addisoncrump commented Apr 19, 2024

Implement Hash for MapObserver #1989

Implement Hash for MapObserver #1989

Conversation

edwin1729 commented Apr 1, 2024

edwin1729 commented Apr 1, 2024

domenukk commented Apr 2, 2024

domenukk commented Apr 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edwin1729 commented Apr 7, 2024 • edited Loading

tokatoka commented Apr 7, 2024

tokatoka commented Apr 7, 2024

domenukk commented Apr 12, 2024

edwin1729 commented Apr 17, 2024 via email • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

addisoncrump commented Apr 18, 2024

domenukk commented Apr 19, 2024

addisoncrump commented Apr 19, 2024

Implement `Hash` for MapObserver #1989

Implement `Hash` for MapObserver #1989

edwin1729 commented Apr 7, 2024 •

edited

Loading

edwin1729 commented Apr 17, 2024 via email •

edited

Loading