Re: Git v2.13.1 SHA1 very broken
- From
Adam Dinwoodie <adam@dinwoodie.org>
- Date
- Jun 6, 2017, 10:03 UTC
- Message-ID
- <20170606100355.GC25777@dinwoodie.org>
- In-Reply-To
- <xmqq4lvtap3m.fsf@gitster.mtv.corp.google.com>
On Tue, Jun 06, 2017 at 10:20:45AM +0900, Junio C Hamano wrote:
Show 11 quoted lines
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > > > That looks scary, can you please comment out this: > > > > #define SHA1DC_ALLOW_UNALIGNED_ACCESS > > > > In sha1dc/sha1.c and see if that helps, alternatively comment out the > > ifdefs guarded by "#ifdef _MSC_VER" calls in sha1dc/sha1.c > > That is merely a performance (and theoretical correctness) thing, > no?
Confirmed rebuilding with either of these suggested changes has t0000.46 still failing.
Show 8 quoted lines
> > The functional differences between 2.13.0 and 2.13.1 on that platform > > should be none aside from possibly those changes, unless I've missed > > something. > > If it does not hash correctly, the cause is more likely that the > endianness detection is going haywire. > > make CFLAGS="-DSHA1DC_FORCE_LITTLEENDIAN -g -O2 -Wall"
Confirmed rebuilding with this option has t0000 passing. I can also get the test to pass with Ramsay Jones' suggestion of using OpenSSL's SHA1.
Digging briefly into the endianness detection, it appears Cygwin has both _LITTLE_ENDIAN and _BIG_ENDIAN defined. Git's detection works by assuming it's in a little endian environment and switching to big endian if it detects any of the defines that indicate such, and a010391 adds _BIG_ENDIAN to the set of defines that indicate big endianness.
The obvious quick fix would be one of the two below patches. I'll also take the discussion to the Cygwin mailing list in the hope that someone can explain why Cygwin defines both _LITTLE_ENDIAN and _BIG_ENDIAN (and indeed _PDP_ENDIAN).
Patch 1 (probably safer?):
diff --git a/sha1dc/sha1.c b/sha1dc/sha1.c index 3dff80ac7..e47d004b1 100644 --- a/sha1dc/sha1.c +++ b/sha1dc/sha1.c @@ -36,6 +36,7 @@ #undef SHA1DC_BIGENDIAN #endif #if (!defined SHA1DC_FORCE_LITTLEENDIAN) && \ + (!defined _LITTLE_ENDIAN) && \ ((defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || \ (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __BIG_ENDIAN__)) || \ defined(_BIG_ENDIAN) || defined(__BIG_ENDIAN__) || defined(__ARMEB__) || defined(__THUMBEB__) || defined(__AARCH64EB__) || \ Patch 2: diff --git a/sha1dc/sha1.c b/sha1dc/sha1.c index 3dff80ac7..8d7b1ee7d 100644 --- a/sha1dc/sha1.c +++ b/sha1dc/sha1.c @@ -38,7 +38,7 @@ #if (!defined SHA1DC_FORCE_LITTLEENDIAN) && \ ((defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || \ (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __BIG_ENDIAN__)) || \ - defined(_BIG_ENDIAN) || defined(__BIG_ENDIAN__) || defined(__ARMEB__) || defined(__THUMBEB__) || defined(__AARCH64EB__) || \ + defined(__BIG_ENDIAN__) || defined(__ARMEB__) || defined(__THUMBEB__) || defined(__AARCH64EB__) || \ defined(_MIPSEB) || defined(__MIPSEB) || defined(__MIPSEB__) || defined(SHA1DC_FORCE_BIGENDIAN)) #define SHA1DC_BIGENDIAN