Fix for compiling on AIX using IBM XLC compiler

[MrShark]

Building on AIX using XLC gives an error during make test:
Error: Compiled for incorrect endianness
This also breaks in git from 2.13.2

Add a #ifdef guard based on macros defined at [2] and [3].

Should perhaps xlc should should be changed to or combined with _AIX
based on the behavour of GCC on AIX or XL C on Linux.

1. git/git@6b851e5
2. https://www.ibm.com/support/knowledgecenter/SSGH2K_13.1.3/com.ibm.xlc1313.aix.doc/compiler_ref/macros_platform.html
3. https://www.ibm.com/support/knowledgecenter/SSGH2K_13.1.3/com.ibm.xlc1313.aix.doc/compiler_ref/xlmacros.html

[jrn]

More context: https://public-inbox.org/git/CAEoyyNJPZXUKi3W=Gs=3v8i4YnTL47G9oaHDoV5pvCLx78jUVQ@mail.gmail.com/

[jrn]

As I discussed in more detail at https://public-inbox.org/git/20180509183008.GL10348@aiede.svl.corp.google.com/, I think the intent behind this defined(__powerpc) is closer to defined(_BIG_ENDIAN).

[kencu]

Before committing this, please see also: #40.

I think perhaps testing for _BIG_ENDIAN might be what we're after? I used this patch on MacPorts https://github.com/kencu/macports-ports/blob/0261b9a80b537020f07b58831c2b5eb44404a27a/devel/git/files/patch-sha1dc-older-apple-gcc-versions.diff but perhaps it is too machine-specific as well.

[shumow]

@kencu I am more inclined to this change here, as it continues the solution that we have been using.

@MrShark Do you have an opinion on Ken's comment?

[kencu]

OK -- but I was thinking that then no sooner would that be committed than we'd need another one right after like:

(defined(__ppc__) && defined(__APPLE__))

and I was hoping we might cover off all of them with a

defined(_BIG_ENDIAN)

instead, as that would seem to include everyone in one fell swoop without causing any troubles that I can forsee...

[MrShark]

I have no big options about the solutions, any is fine with me. But I can confirm that a patch like below also works for AIX with xlc:

diff --git a/lib/sha1.c b/lib/sha1.c
index 25eded1..b30988e 100644
--- a/lib/sha1.c
+++ b/lib/sha1.c
@@ -84,7 +84,7 @@
 /* Not under GCC-alike or glibc or *BSD or newlib */
 #elif (defined(__ARMEB__) || defined(__THUMBEB__) || defined(__AARCH64EB__) || \
        defined(__MIPSEB__) || defined(__MIPSEB) || defined(_MIPSEB) || \
-       defined(__sparc))
+       defined(__sparc) || defined(_BIG_ENDIAN))
 /*
  * Should define Big Endian for a whitelist of known processors. See
  * https://sourceforge.net/p/predef/wiki/Endianness/ and

[cr-marcstevens]

Yes, you're probably right. Endianness detection remains ugly stuff.
The problem indeed is that even for little endian machines a BIG_ENDIAN macro may be defined, and vice versa.
Although I don't know if we tried this logic: if defined(BIG_ENDIAN) && !defined(LITTLE_ENDIAN) to avoid that problem.

I was still hoping there might be a more general and simpler solution than whitelisting platforms,
but what we have works and adding another whitelisting should cause no problems if it is specific enough.
Alright, so let's go for this approach that doesn't cause problems for other people.

[cr-marcstevens]

BTW the original issue mentioned that this broke git.
Didn't git have its own endianness detection that overrules our own?

[kencu]

Apparently not - at least, it broke git on some of my machines. On certain system versions, however, Apple machines use a different SHA1 method (Apple Common Crypto) so they weren't affected.

Would you like me to make up another PR for the Apple tests?

[shumow]

Would you like me to make up another PR for the Apple tests?
@kencu Yes please.

[aixtools]

I 'discovered' this independently - while trying to build the latest git.

As this is 'for AIX' and AIX is only BigEndian the simple define to check is simply '_AIX'.

When it is "Linux on Power" I would suggest looking in /proc/... somewhere. I do not have a system handy, but there is file/directory with the current operational mode (as POWER can run in either mode - iirc the 'boot' process tells the processor which endian mode it needs.

[aixtools]

This is not "AIX" related, but POWER related. Maybe the title should/cound be changed to "Fix when compiling on POWER".

a) POWER7 and earlier are Big Endian only
b) POWER8 and later support both modes

With Debian (on POWER6) I could not find anything useful in /proc/{powerppc|ppc64|ppc64el|ppc64le}/{rtas|lparcfg}

However the output of lscpu is useful:

root@x074:/usr/bin# lscpu | grep -i endian
Byte Order:            Big Endian

Again, AIX is always Big Endian.

Looking at a gcc compiler I have for "defines" it looks again asif _AIX is the thing to test:

root@x068:[/data/httpd/gcc]gcc -dM -E - < /dev/null | grep AIX
#define _AIX 1
#define _AIX32 1
#define _AIX41 1
#define _AIX43 1
#define _AIX51 1
#define _AIX52 1
#define _AIX53 1

While xlc, in /etc/vacx.cfg.{53|61|71|72} has:

michael@x071:[/home/michael]/usr/bin/grep -p DEFLT: /etc/vac* | grep options\
        options   = -D_AIX,-D_AIX32,-D_AIX41,-D_AIX43,-D_AIX50,-D_AIX51,-D_AIX52,-D_AIX53,-D_IBMR2,-D_POWER
        options   = -D_AIX,-D_AIX32,-D_AIX41,-D_AIX43,-D_AIX50,-D_AIX51,-D_AIX52,-D_AIX53,-D_AIX61,-D_IBMR2,-D_POWER
        options   = -D_AIX,-D_AIX32,-D_AIX41,-D_AIX43,-D_AIX50,-D_AIX51,-D_AIX52,-D_AIX53,-D_AIX61,-D_AIX71,-D_IBMR2,-D_POWER
        options   = -D_AIX,-D_AIX32,-D_AIX41,-D_AIX43,-D_AIX50,-D_AIX51,-D_AIX52,-D_AIX53,-D_AIX61,-D_AIX71,-D_AIX72,-D_IBMR2,-D_POWER

[kencu]

This PR is now redundant given #45, so it might as well be closed.

I may open up another PR at some point for the Apple PPC systems, but that is fixed now with a MacPorts patch anyway so not in any way particularly urgent.

[avar]

@kencu : Do you have a link to that MacPorts patch?

[kencu]

Sure - sorry, was away for a bit. Here's the patch we have on MacPorts: https://github.com/macports/macports-ports/blob/0c8d458a59b44e344f2a91cc41c047d4d4867388/devel/git/files/patch-sha1dc-older-apple-gcc-versions.diff

[MrShark]

Cleaning up among my old pull requests, I no longer have access to AIX systems to test this out on and since it seems soved in #45.