Remove {update,clear}_hash from ObserverWithHashField, add hasher (extending #1019) (#1028)

* libafl: Remove `{update,clear}_hash` from `ObserverWithHashField`

These methods aren't used by `NewHashFeedback`, so there's no compelling reason
to keep them in the interface. They preclude implementations of
`ObserverWithHashField` that calculcate a hash on-the-fly from a value. For
example, my use-case is to store the stdout of a process, and use
`NewHashFeedback` to only collect inputs that result in new messages on stdout.

Both of these methods are pretty suspicious to begin with - why should other
code be able to update the internal state of the observer? What are the
semantics of `update_hash`? If there are compelling reasons to keep these
methods, let's clarify their intent in the documentation.

* libafl: Return hash by value from `ObserverWithHashField`

This allows implementors of this trait to not store the hash, but rather to
compute it on-the-fly. Since `Option<u64>` is `Copy` (and quite small), and
this method is called once per execution of the target program, this is likely
to have negligible performance impact.

* libafl: Implement `ObserverWithHashField` for `ValueObserver`

This demonstrates the utility of the previous two commits. Now, `ValueObserver`
can be used with `NewHashFeedback`.

* Clippy, move to ahasher

* Oops :)

---------

Co-authored-by: Langston Barrett <langston.barrett@gmail.com>

libafl/src/bolts/core_affinity.rs

@@ -593,6 +593,7 @@ mod apple {
     }
 
     #[cfg(target_arch = "aarch64")]
+    #[allow(clippy::unnecessary_wraps)]
     pub fn set_for_current(_core_id: CoreId) -> Result<(), Error> {
         // This is the best we can do, unlike on intel architecture
         // the system does not allow to pin a process/thread to specific cpu.

libafl/src/feedbacks/new_hash_feedback.rs

@@ -109,7 +109,7 @@ where
         match observer.hash() {
             Some(hash) => {
                 let res = backtrace_state
-                    .update_hash_set(*hash)
+                    .update_hash_set(hash)
                     .expect("Failed to update the hash state");
                 Ok(res)
             }

libafl/src/observers/mod.rs

@@ -280,11 +280,7 @@ where
 /// A trait for [`Observer`]`s` with a hash field
 pub trait ObserverWithHashField {
     /// get the value of the hash field
-    fn hash(&self) -> &Option<u64>;
-    /// update the hash field with the given value
-    fn update_hash(&mut self, hash: u64);
-    /// clears the current value of the hash and sets it to None
-    fn clear_hash(&mut self);
+    fn hash(&self) -> Option<u64>;
 }
 
 /// A trait for [`Observer`]`s` which observe over differential execution.

libafl/src/observers/stacktrace.rs

@@ -77,26 +77,26 @@ impl<'a> BacktraceObserver<'a> {
             harness_type,
         }
     }
-}
-
-impl<'a> ObserverWithHashField for BacktraceObserver<'a> {
-    /// Gets the hash value of this observer.
-    #[must_use]
-    fn hash(&self) -> &Option<u64> {
-        self.hash.as_ref()
-    }
 
     /// Updates the hash value of this observer.
     fn update_hash(&mut self, hash: u64) {
         *self.hash.as_mut() = Some(hash);
     }
 
-    /// Clears the current hash value
+    /// Clears the current hash value (sets it to `None`)
     fn clear_hash(&mut self) {
         *self.hash.as_mut() = None;
     }
 }
 
+impl<'a> ObserverWithHashField for BacktraceObserver<'a> {
+    /// Gets the hash value of this observer.
+    #[must_use]
+    fn hash(&self) -> Option<u64> {
+        *self.hash.as_ref()
+    }
+}
+
 impl<'a, S> Observer<S> for BacktraceObserver<'a>
 where
     S: UsesInput,
@@ -219,23 +219,18 @@ impl AsanBacktraceObserver {
         });
         self.update_hash(hash);
     }
-}
-
-impl ObserverWithHashField for AsanBacktraceObserver {
-    /// Gets the hash value of this observer.
-    #[must_use]
-    fn hash(&self) -> &Option<u64> {
-        &self.hash
-    }
 
     /// Updates the hash value of this observer.
     fn update_hash(&mut self, hash: u64) {
         self.hash = Some(hash);
     }
+}
 
-    /// Clears the current hash value
-    fn clear_hash(&mut self) {
-        self.hash = None;
+impl ObserverWithHashField for AsanBacktraceObserver {
+    /// Gets the hash value of this observer.
+    #[must_use]
+    fn hash(&self) -> Option<u64> {
+        self.hash
     }
 }
 

libafl/src/observers/value.rs

@@ -4,14 +4,19 @@ use alloc::{
     boxed::Box,
     string::{String, ToString},
 };
-use core::fmt::Debug;
+use core::{
+    fmt::Debug,
+    hash::{BuildHasher, Hash, Hasher},
+};
 
+use ahash::RandomState;
 use serde::{Deserialize, Serialize};
 
 use super::Observer;
 use crate::{
     bolts::{ownedref::OwnedRef, tuples::Named},
     inputs::UsesInput,
+    observers::ObserverWithHashField,
     Error,
 };
 
@@ -88,3 +93,14 @@ where
         &self.name
     }
 }
+
+impl<'a, T: Hash> ObserverWithHashField for ValueObserver<'a, T>
+where
+    T: Debug + Serialize + serde::de::DeserializeOwned,
+{
+    fn hash(&self) -> Option<u64> {
+        let mut s = RandomState::with_seeds(1, 2, 3, 4).build_hasher();
+        Hash::hash(self.value.as_ref(), &mut s);
+        Some(s.finish())
+    }
+}