[PATCH] D40376: [LibFuzzer] Fix `CounterToFeature()` so that it doesn't ignore the 6th bit.

[Dan Liew via Phabricator via llvm-commits]

delcypher added a comment.

In https://reviews.llvm.org/D40376#935496, @kcc wrote:

> Why do you think this is a bug? 
>  The user-provided counter is a counter, not a bit set.


I thought this was a bug because initially I thought the counters were supposed to be bit set. I realised much later when I ran into the `TableLookupTest.cpp` test that it was not your intention was for  `"__libfuzzer_extra_counters` to be a bit set.
However questions still remain and I thought illustrating my thoughts with a patch might be easier to understand.

- Why is `CounterToFeature()` implemented the way it currently is? The obvious implementation is the one I sketch in the patch that treats each bit in the 8-bit counter as a "new feature". With that implementation we would record new features when the counter hits values `[1, 2, 4, 8, 16, 32, 64, 128]` (i.e. a bit set). In the current implementation we record new features when the counter hits values `[1, 2, 3, 4, 8, 16, 32, 128]`. It is not obvious why you've done this so I think it would be good to provide an explanation and put that in comments for the `CounterToFeature()` function. My guess is that you wanted events occurring a very small number (<=4) of times to be treated as features, hence the initially linear behaviour of `CounterToFeature()` that then becomes (sort of) exponential after 4 events have occurred.

- On a semi-related note why is the entire `__libfuzzer_extra_counters` section implicitly zero-ed on every call to `LLVMFuzzerTestOneInput`? In my case I don't actually want this because persisting the counters across calls to `LLVMFuzzerTestOneInput` is not a problem so I'm just wasting time by zero-ing the counters before every call to `LLVMFuzzerTestOneInput`. I can certainly see use cases where you would want to zero the counters before every call to `LLVMFuzzerTestOneInput`, however shouldn't the user just do this themselves if this is the behaviour they want?


https://reviews.llvm.org/D40376