Skip to content

Commit 9dc523a

Browse files
avargitster
authored andcommitted
Makefile + hash.h: remove PPC_SHA1 implementation
Remove the PPC_SHA1 implementation added in a6ef351 ([PATCH] PPC assembly implementation of SHA1, 2005-04-22). When this was added Apple consumer hardware used the PPC architecture, and the implementation was intended to improve SHA-1 speed there. Since it was added we've moved to using sha1collisiondetection by default, and anyone wanting hard-rolled non-DC SHA-1 implementation can use OpenSSL's via the OPENSSL_SHA1 knob. The PPC_SHA1 originally originally targeted 32 bit PPC, and later the 64 bit PPC 970 (a.k.a. Apple PowerPC G5). See 926172c (block-sha1: improve code on large-register-set machines, 2009-08-10) for a reference about the performance on G5 (a comment in block-sha1/sha1.c being removed here). I can't get it to do anything but segfault on both the BE and LE POWER machines in the GCC compile farm[1]. Anyone who's concerned about performance on PPC these days is likely to be using the IBM POWER processors. There have been proposals to entirely remove non-sha1collisiondetection implementations from the tree[2]. I think per [3] that would be a bit overzealous. I.e. there are various set-ups git's speed is going to be more important than the relatively implausible SHA-1 collision attack, or where such attacks are entirely mitigated by other means (e.g. by incoming objects being checked with DC_SHA1). But that really doesn't apply to PPC_SHA1 in particular, which seems to have outlived its usefulness. As this gets rid of the only in-tree *.S assembly file we can remove the small bits of logic from the Makefile needed to build objects from *.S (as opposed to *.c) The code being removed here was also throwing warnings with the "-pedantic" flag, it could have been fixed as 544d93b (block-sha1: remove use of obsolete x86 assembly, 2022-03-10) did for block-sha1/*, but as noted above let's remove it instead. 1. https://cfarm.tetaneutral.net/machines/list/ Tested on gcc{110,112,135,203}, a mixture of POWER [789] ppc64 and ppc64le. All segfault in anything needing object hashing (e.g. t/t1007-hash-object.sh) when compiled with PPC_SHA1=Y. 2. https://lore.kernel.org/git/[email protected]/ 3. https://lore.kernel.org/git/[email protected]/ Acked-by: brian m. carlson" <[email protected]> Signed-off-by: Ævar Arnfjörð Bjarmason <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent d42b38d commit 9dc523a

File tree

8 files changed

+8
-347
lines changed

8 files changed

+8
-347
lines changed

INSTALL

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,7 @@ Issues of note:
135135

136136
By default, git uses OpenSSL for SHA1 but it will use its own
137137
library (inspired by Mozilla's) with either NO_OPENSSL or
138-
BLK_SHA1. Also included is a version optimized for PowerPC
139-
(PPC_SHA1).
138+
BLK_SHA1.
140139

141140
- "libcurl" library is used for fetching and pushing
142141
repositories over http:// or https://, as well as by

Makefile

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -155,9 +155,6 @@ include shared.mak
155155
# Define BLK_SHA1 environment variable to make use of the bundled
156156
# optimized C SHA1 routine.
157157
#
158-
# Define PPC_SHA1 environment variable when running make to make use of
159-
# a bundled SHA1 routine optimized for PowerPC.
160-
#
161158
# Define DC_SHA1 to unconditionally enable the collision-detecting sha1
162159
# algorithm. This is slower, but may detect attempted collision attacks.
163160
# Takes priority over other *_SHA1 knobs.
@@ -1802,6 +1799,10 @@ ifdef APPLE_COMMON_CRYPTO
18021799
SHA1_MAX_BLOCK_SIZE = 1024L*1024L*1024L
18031800
endif
18041801

1802+
ifdef PPC_SHA1
1803+
$(error the PPC_SHA1 flag has been removed along with the PowerPC-specific SHA-1 implementation.)
1804+
endif
1805+
18051806
ifdef OPENSSL_SHA1
18061807
EXTLIBS += $(LIB_4_CRYPTO)
18071808
BASIC_CFLAGS += -DSHA1_OPENSSL
@@ -1810,10 +1811,6 @@ ifdef BLK_SHA1
18101811
LIB_OBJS += block-sha1/sha1.o
18111812
BASIC_CFLAGS += -DSHA1_BLK
18121813
else
1813-
ifdef PPC_SHA1
1814-
LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
1815-
BASIC_CFLAGS += -DSHA1_PPC
1816-
else
18171814
ifdef APPLE_COMMON_CRYPTO
18181815
COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
18191816
BASIC_CFLAGS += -DSHA1_APPLE
@@ -1847,7 +1844,6 @@ endif
18471844
endif
18481845
endif
18491846
endif
1850-
endif
18511847

18521848
ifdef OPENSSL_SHA256
18531849
EXTLIBS += $(LIB_4_CRYPTO)
@@ -2594,14 +2590,10 @@ missing_compdb_dir =
25942590
compdb_args =
25952591
endif
25962592

2597-
ASM_SRC := $(wildcard $(OBJECTS:o=S))
2598-
ASM_OBJ := $(ASM_SRC:S=o)
2599-
C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
2593+
C_OBJ := $(OBJECTS)
26002594

26012595
$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
26022596
$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
2603-
$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
2604-
$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
26052597

26062598
%.s: %.c GIT-CFLAGS FORCE
26072599
$(QUIET_CC)$(CC) -o $@ -S $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<

block-sha1/sha1.c

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,6 @@
2828
* try to do the silly "optimize away loads" part because it won't
2929
* see what the value will be).
3030
*
31-
* Ben Herrenschmidt reports that on PPC, the C version comes close
32-
* to the optimized asm with this (ie on PPC you don't want that
33-
* 'volatile', since there are lots of registers).
34-
*
3531
* On ARM we get the best code generation by forcing a full memory barrier
3632
* between each SHA_ROUND, otherwise gcc happily get wild with spilling and
3733
* the stack frame size simply explode and performance goes down the drain.

configure.ac

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -237,9 +237,6 @@ AC_MSG_NOTICE([CHECKS for site configuration])
237237
# tests. These tests take up a significant amount of the total test time
238238
# but are not needed unless you plan to talk to SVN repos.
239239
#
240-
# Define PPC_SHA1 environment variable when running make to make use of
241-
# a bundled SHA1 routine optimized for PowerPC.
242-
#
243240
# Define NO_OPENSSL environment variable if you do not have OpenSSL.
244241
#
245242
# Define OPENSSLDIR=/foo/bar if your openssl header and library files are in

hash.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,7 @@
44
#include "git-compat-util.h"
55
#include "repository.h"
66

7-
#if defined(SHA1_PPC)
8-
#include "ppc/sha1.h"
9-
#elif defined(SHA1_APPLE)
7+
#if defined(SHA1_APPLE)
108
#include <CommonCrypto/CommonDigest.h>
119
#elif defined(SHA1_OPENSSL)
1210
#include <openssl/sha.h>
@@ -32,7 +30,7 @@
3230
* platform's underlying implementation of SHA-1; could be OpenSSL,
3331
* blk_SHA, Apple CommonCrypto, etc... Note that the relevant
3432
* SHA-1 header may have already defined platform_SHA_CTX for our
35-
* own implementations like block-sha1 and ppc-sha1, so we list
33+
* own implementations like block-sha1, so we list
3634
* the default for OpenSSL compatible SHA-1 implementations here.
3735
*/
3836
#define platform_SHA_CTX SHA_CTX

ppc/sha1.c

Lines changed: 0 additions & 72 deletions
This file was deleted.

ppc/sha1.h

Lines changed: 0 additions & 25 deletions
This file was deleted.

0 commit comments

Comments
 (0)