git @2.14.1 generates corrupt SHA1 hashes on Tiger when built with gcc42, but works OK when built with gcc6

[kencu (Ken)]

This is a somewhat odd one. It's a corner case to be sure, but I thought I'd note it here as others are likely to run into it as well, I think.

Came across this after the recent upgrade of git, which builds without errors with gcc-4.2 by default on Tiger. On Tiger, git was unable to add or commit files after the upgrade to 2.14.1; also unable to clone repositories, with errors such as:

$ sudo git clone https://github.com/kencu/TigerPorts.git
Cloning into 'TigerPorts'...
remote: Counting objects: 475, done.
remote: Compressing objects: 100% (9/9), done.
remote: Total 475 (delta 0), reused 9 (delta 0), pack-reused 465
Receiving objects: 100% (475/475), 206.52 KiB | 538.00 KiB/s, done.
fatal: pack is corrupted (SHA1 mismatch)
fatal: index-pack failed

I tried rebuilding it again using the default gcc-4.2, but ran into the same errors.

Rebuilding git 2.14.1 using gcc6 however generates a normally-working git:

$ sudo git clone https://github.com/kencu/TigerPorts.git
Cloning into 'TigerPorts'...
remote: Counting objects: 475, done.
remote: Compressing objects: 100% (9/9), done.
remote: Total 475 (delta 0), reused 9 (delta 0), pack-reused 465
Receiving objects: 100% (475/475), 206.52 KiB | 599.00 KiB/s, done.
Resolving deltas: 100% (112/112), done.

Just to make it nice and confusing, the same version of git works perfectly well on Leopard PPC when built with gcc-4.2.

[ryandesign (Ryan Carsten Schmidt)]

Gosh, I have no idea where to begin with this, but it sounds like it's not a MacPorts-specific problem. Please report it to the developers of git.

You should be more specific about the compilers, however. When you say "gcc-4.2 ... on Tiger", I assume you mean the Apple version of gcc 4.2 installed by the apple-gcc42 port? And when you talk about gcc-4.2 on Leopard, I assume you mean the Apple version of gcc 4.2 installed by Xcode 3.1.4?

[kencu (Ken)]

Will do. Your assumption about the compilers is correct. I'll also try it on a different Tiger system I have and see what happens there.

[kencu (Ken)]

I haven't dug in on this (because it worked with gcc6), but it is worthy of an upstream report now that others are coming across it as well.

[mf2k (Frank Schima)]

Has duplicate #54786.

[kencu (Ken)]

I can't see much in the way of configuration differences, building with apple-gcc-4.2 vs macports-gcc-6. I'll attach the full build logs for each.

/opt/local/bin/gcc-apple-4.2 -o common-main.o -c -MF ./.depend/common-main.o.d -MQ common-main.o -MMD -MP  -I. -Wall -O2 -I/opt/local/include -arch ppc -I. -DPRECOMPOSE_UNICODE -DPROTECT_HFS_DEFAULT=1 -DUSE_LIBPCRE2 -I/opt/local/include -I/opt/local/include -DUSE_CURL_FOR_IMAP_SEND -I/opt/local/include -I/opt/local/include -DUSE_ST_TIMESPEC -DOLD_ICONV -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" -DSHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_C="\"sha1dc_git.c\"" -DSHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_H="\"sha1dc_git.h\"" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\""  -DHAVE_DEV_TTY -DHAVE_BSD_SYSCTL  -DFREAD_READS_DIRECTORIES -DNO_MEMMEM -Icompat/regex -DSHELL_PATH='"/bin/sh"' -DPAGER_ENV='"LESS=FRX LV=-c"'  common-main.c

/opt/local/bin/gcc-mp-6      -o common-main.o -c -MF ./.depend/common-main.o.d -MQ common-main.o -MMD -MP  -I. -Wall -O2 -I/opt/local/include -m32      -I. -DPRECOMPOSE_UNICODE -DPROTECT_HFS_DEFAULT=1 -DUSE_LIBPCRE2 -I/opt/local/include -I/opt/local/include -DUSE_CURL_FOR_IMAP_SEND -I/opt/local/include -I/opt/local/include -DUSE_ST_TIMESPEC -DOLD_ICONV -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" -DSHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_C="\"sha1dc_git.c\"" -DSHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_H="\"sha1dc_git.h\"" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\""  -DHAVE_DEV_TTY -DHAVE_BSD_SYSCTL  -DFREAD_READS_DIRECTORIES -DNO_MEMMEM -Icompat/regex -DSHELL_PATH='"/bin/sh"' -DPAGER_ENV='"LESS=FRX LV=-c"'  common-main.c

[ryandesign (Ryan Carsten Schmidt)]

Did you file an upstream bug report? If so can you share the URL?

For some additional datapoints, on Tiger PPC, I rebuilt git 2.17.0 with Xcode gcc-4.0, which has the same problem. And on Leopard i386, I rebuilt git with MacPorts apple-gcc-4.2, which did not exhibit the problem.

[kencu (Ken)]

You know, I admit I never did file a report. Such an old system, odd error with ancient compiler, easily fixed by using a newer compiler -- I thought they'd laugh me out of the git world...

[ryandesign (Ryan Carsten Schmidt)]

Requiring a newer compiler for git makes bootstrapping an installation of git that much more difficult, or at least time-consuming. And the problem isn't just the old compiler, since it seems like the same old compiler works fine on Leopard. If upstream doesn't want to fix it, fine; we can accept the compiler requirement. But I think it's only fair for us to give the developers an opportunity to fix it.

[kencu (Ken)]

I had time to look at this today, in the "what else do I have to do?" category.

Git introduced a new SHA1 calculating scheme with ​this commit and this made it into git 13.1. I noticed git 12.x worked properly and git 14.x did not, so that fits with the timing. This only applies to systems that do not use Apple Common Crypto (i.e. < 10.5) so not noted on Leopard PPC. There were issues with this commit noted on ​several systems.

This was reworked, and then ​reworked again. Looking this over, an error in the endianess of this seemed to be the most likely thing that has gone wrong on PowerPC Tiger. Sure enough, forcing the endianess by adding the following to the CFLAGS:

-DSHA1DC_FORCE_BIGENDIAN=1

fixes git on Tiger PPC when built with stock compilers. I suppose gcc6 must define one of the tests that git is using to decide on BIGENDIAN, whereas gcc-4.x does not define it.

It's not so easy to add a cppflag to the git build, as it doesn't use configure, and it hard codes in all the flags directly. One way to do it is to put this block below the current CFLAGS definintion

platform darwin 8 powerpc {
    set CFLAGS "${CFLAGS} -DSHA1DC_FORCE_BIGENDIAN=1"
}

better, of course, would be to have the git-ters fix it properly, if they want to.

[kencu (Ken)]

It looks like there is another environment variable that could be used when building git, PPC_SHA1, that might turn out to be simpler to implement.

Edit: It is simpler to implement, but pulls in a source file written in assembly that gcc-4.2 barfs on, probably because it's 64bit assembly.

[ryandesign (Ryan Carsten Schmidt)]

Thanks for looking into it.

Forcing endianness would mess up the universal variant. Probably simplest to disable the universal variant on darwin 8 then.

Did you file an upstream bug report?

[kencu (Ken)]

There is one more possibility to consider -- perhaps the SHA1 routine in OPENSSL might work:

# Define OPENSSL_SHA1 environment variable when running make to link
# with the SHA1 routine from openssl library.

I'll do a bug report as soon as I figure it out fully. Still not sure why gcc6 works. Like to get that nailed.

[kencu (Ken)]

see <​https://github.com/cr-marcstevens/sha1collisiondetection/issues/40> for the upstream bug reports.

[ken-cunningham-webuse]

In 4dbe3e0a8deed4fc0dbc54013702c54bda3c69f9/macports-ports (master):

git: fix SHA1 calculation for older compilers

fixes stock Apple compilers on older PPC machines
by correcting endianness checks
closes: #54602

[ryandesign (Ryan Carsten Schmidt)]

Are you certain that fix is correct for Intel Macs?

[ryandesign (Ryan Carsten Schmidt)]

After looking at it further, it looks fine to me. I was just misled by your comment:

/* older gcc compilers which are the default on Apple PPC do not define __BYTE_ORDER__ */

Those older compilers are the default on older systems regardless of architecture, but I understand now that the code defaults to little-endian, so the problem only occurs on big-endian machines.