[PATCH] D74359: [compiler-rt] FuzzedDataProvider: add ConsumeMemory and hash methods.

[Jonathan Metzman via Phabricator via llvm-commits]

metzman accepted this revision.
metzman added a comment.
This revision is now accepted and ready to land.

Mostly LGTM.
What's hash for? To seed an RNG?
I'm not 100% hashing the whole array is the right way to do things but it might be better than nothing.
I guess changing this is possible but not ideal since corpus elements won't map to the same behavior if we change it to say, the last byte or something similar.



================
Comment at: compiler-rt/include/fuzzer/FuzzedDataProvider.h:252
+
+  // Returns the hash of the remaining data. Not recommended in general, but is
+  // useful when you need a pseudo-random value that changes significantly with
----------------
I think saying "Not recommended in general" is a bit weird. What's wrong with it for the intended purpose?
maybe say it's the `std::hash`. When I read "the hash" my first thought was "which? md5? sha1? etc."


================
Comment at: compiler-rt/include/fuzzer/FuzzedDataProvider.h:255
+  // any change in the fuzzing data.
+  size_t hash() {
+    // TODO(Dor1s): change to `string_view` once C++17 becomes mainstream.
----------------
I think we need to include `<functional>` to use `std::hash` right?


================
Comment at: compiler-rt/include/fuzzer/FuzzedDataProvider.h:256
+  size_t hash() {
+    // TODO(Dor1s): change to `string_view` once C++17 becomes mainstream.
+    std::string str(data_ptr_, remaining_bytes_);
----------------
I would guess this will be a long way off.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74359/new/

https://reviews.llvm.org/D74359