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

[Kostya Serebryany via Phabricator via llvm-commits]

kcc added a comment.

In https://reviews.llvm.org/D40376#935532, @delcypher wrote:

> 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.


Yes, just as you describe. 
This is a heuristic stolen from AFL: http://lcamtuf.coredump.cx/afl/technical_details.txt (search for 128)
Once I have a proper A/B testing for libFuzzer, it may change. Please treat it as an implementation detail.

> 
> 
> - 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?

Because the coverage signal should be stable between runs. 
(please prefer libfuzzer at googlegroups.com for discussions like this)


https://reviews.llvm.org/D40376