Re: [RFC][PATCH] index-pack: add testcases found using AFL

From: Jeff King <peff@peff.net>
Date: Mar 10, 2017, 19:06 UTC
Message-ID: <20170310190641.i7geazhrlmzzfna6@sigill.intra.peff.net>
In-Reply-To: <20170310151556.18490-1-vegard.nossum@oracle.com>

[Note: your original email didn't make it to the list because it's over 100K; I'll quote liberally].

On Fri, Mar 10, 2017 at 04:15:56PM +0100, Vegard Nossum wrote:

Show 23 quoted lines

> I've used AFL to generate a corpus of pack files that maximises the edge
> coverage for 'git index-pack'.
> 
> This is a supplement to (and not a replacement for) the regular test cases
> where we know exactly what each test is checking for. These testcases are
> more useful for avoiding regressions in edge cases or as a starting point
> for future fuzzing efforts.
> 
> To see the output of running 'git index-pack' on each file, you can do
> something like this:
> 
>   make -C t GIT_TEST_OPTS="--run=34 --verbose" t5300-pack-object.sh
> 
> I observe the following coverage changes (for t5300 only):
> 
>   path                  old%  new%    pp
>   ----------------------------------------
>   builtin/index-pack.c  74.3  76.6   2.3
>   pack-write.c          79.8  80.4    .6
>   patch-delta.c         67.4  81.4  14.0
>   usage.c               26.6  35.5   8.9
>   wrapper.c             42.0  46.1   4.1
>   zlib.c                58.7  64.1   5.4

I'm not sure how I feel about this. More coverage is good, I guess, but we don't have any idea what these packfiles are doing, or whether index-pack is behaving sanely in the new lines. The most we can say is that we tested more lines of code and that nothing segfaulted or triggered something like ASAN.

That's something I guess, but I'm not enthused by the idea of just dumping a bunch of binary test cases that nobody, not even the author, understands.

-Peff