Re: [PATCH v4 08/23] ewah: compressed bitmap implementation

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jan 23, 2014, 19:52 UTC
Message-ID: <20140123195206.GU18964@google.com>
In-Reply-To: <20140123183320.GA22995@sigill.intra.peff.net>

Jeff King wrote:

> Here's a patch series (on top of jk/pack-bitmap, naturally) that lets
> t5310 pass there. I assume the ARM problem is the same, though seeing
> the failure in realloc() is unexpected. Can you try it on both your
> platforms with these patches?

Thanks.  Trying it out now.

[...]

Show 9 quoted lines

>> Hopefully it's possible to get the alignment right in the caller
>> and tweak the signature to require that instead of using unaligned
>> reads like this.  There's still something wrong after this patch ---
>> the new result is a NULL pointer dereference in t5310.7 "enumerate
>> --objects (full bitmap)".
>
> After my patches, t5310 runs fine for me. I didn't try your patch, but
> mine are similar. Let me know if you still see the problem (there may
> simply be a bug in yours, but I didn't see it).

I had left out a cast to unsigned, producing an overflow.

My main worry about the patches is that they will probably run into an analagous problem to the one that v1.7.12-rc0~1^2~2 (block-sha1: avoid pointer conversion that violates alignment constraints, 2012-07-22) solved. By casting the pointer to (uint32_t *) we are telling the compiler it is 32-bit aligned (C99 section 6.3.2.3).

Thanks, Jonathan