When no SIMD is found, configure must deactivate SIMD support

[claudioandre-br]

When no SIMD is found, configure should behave as if the user had set --disable-simd:

• set that no SIMD is avaiblable;
• define JOHN_NO_SIMD;
• ...

That's not what's happening on Android.

The lines below will include the header even when SIMD is not supported (in my example).

john/src/pseudo_intrinsics.h

Lines 47 to 48 in f351721

	#if __ARM_NEON || __aarch64__
	#include <arm_neon.h>

checking special compiler flags... ARM
checking for NEON... no
checking for ASIMD... no

[magnumripper]

So what's the output of gcc -dM -E -x c /dev/null | grep -E 'NEON|SIMD'? Are the build tools generic ones from third party?

[magnumripper]

I think this is basically a bug in [configuration of] the build tools (although we could add some logic to mitigate the problem). The natural workaround is to just pass --disable-simd to configure.

[magnumripper]

Actually if they configured a default -march=armv8-a+simd I'm not sure we can disable it (unless -march=native turns off the relevant functions). There doesn't seem to be any -no-march= or -march=no-(...)

[magnumripper]

Apparently for some ARM archs you can disable simd with an march such as -march=armv7-a+nosimd.

It doesn't appear to work on my armv8.3-a macbook M1:

$ gcc -dM -E -x c /dev/null -march=armv8.3-a+nosimd | grep -iE 'NEON|SIMD'
#define __AARCH64_SIMD__ 1
#define __ARM_NEON__ 1

[claudioandre-br]

Full log (config.log and Makefile included) when arm32le.h is selected:
arm32le.h.txt

Full log (idem) when arm64le.h is selected (error at line 1179):
arm64le.txt

I can't see what's causing it:

[claudioandre-br]

This is bad:

john/src/arm64le.h

Lines 45 to 52 in 2d585e0

	/*
	* Here we assume that we're on AArch64, which implies we have Advanced SIMD.
	* Tell our originally 32-bit ARM code that we sort of have NEON.
	* Newer gcc does the same for us on its own, but older gcc needs help here.
	*/
	#ifndef __ARM_NEON
	#define __ARM_NEON 1
	#endif

And wrong.

---

[EDITED]

In my example probably configure is doing the right thing and the #ifndef above is surely doing something wrong.

[magnumripper]

Everything looks correct: Neither __ARM_NEON nor __aarch64__ are defined so the header shouldn't be loaded. Does it fail building or does it segfault when run?

[claudioandre-br]

Everything looks correct: Neither __ARM_NEON ...

See my post above. It is being defined by john/src/arm64le.h ITSELF.

---

[EDITED]

The #ifndef is wrong. If some older gcc needs help, we need to add a check and not blindly set __ARM_NEON.

diff --git a/src/arm64le.h b/src/arm64le.h
index a916cc053..258c9a720 100644
--- a/src/arm64le.h
+++ b/src/arm64le.h
@@ -47,9 +47,11 @@
  * Tell our originally 32-bit ARM code that we sort of have NEON.
  * Newer gcc does the same for us on its own, but older gcc needs help here.
  */
+#if __GNUC__ < 6
 #ifndef __ARM_NEON
 #define __ARM_NEON 1
 #endif
+#endif
 /*
  * Give native vsel() a try with DES_BS=3, even though the timings are often
  * such that it might be better to avoid its use, and DES_BS=1 might be better.

[claudioandre-br]

See 142544a

[magnumripper]

Oh, that explains it. But what exact CPU do you have a problem with? I would have thought any 64-bit ARM would support SIMD.

Anyway I see no problem with applying that #if __GNUC__ < 6.

[solardiz]

 #define __ARM_NEON 1

This piece is already within #if !JOHN_NO_SIMD, so a sufficient fix would be to do what @claudioandre-br suggested at first:

When no SIMD is found, configure should behave as if the user had set --disable-simd:

* set that no SIMD is avaiblable;

* define JOHN_NO_SIMD;

I am not sure this is what we should do. Maybe the gcc version check is a better way. But we shouldn't need both.

[claudioandre-br]

Oh, that explains it. But what exact CPU do you have a problem with? I would have thought any 64-bit ARM would support SIMD.

We “need” to use Android Studio (Android NDK) to some extent. So “in the beginning”, at least, it's x86 hardware.

---

Without SIMD, the tests work just fine after a successful build.

[solardiz]

My comment on that hack included:

  * Tell our originally 32-bit ARM code that we sort of have NEON.

which reminds me of the reason why I added this - my other code in core (I think in DES_bs_b.c) only checked for NEON and didn't know AArch64. I see that our current code in jumbo knows both and checks the right macros as appropriate in each place, except that the recently added mbedtls/common.h still only checks __ARM_NEON in one of the places where I think it should be checking defined(__ARM_NEON) || defined(__aarch64__).

So as an option we may completely drop this old hack and instead patch one line in defined(__ARM_NEON) || defined(__aarch64__) as above. I don't know if such patching would be of any help, though, if the Mbed-TLS code maybe does not currently build for any ARM with NEON by versions of gcc below 6:

#if defined(MBEDTLS_AESCE_HAVE_CODE)

/* Compiler version checks. */
#if defined(__clang__)
#   if defined(MBEDTLS_ARCH_IS_ARM32) && (__clang_major__ < 11)
#       error "Minimum version of Clang for MBEDTLS_AESCE_C on 32-bit Arm or Thumb is 11.0."
#   elif defined(MBEDTLS_ARCH_IS_ARM64) && (__clang_major__ < 4)
#       error "Minimum version of Clang for MBEDTLS_AESCE_C on aarch64 is 4.0."
#   endif
#elif defined(__GNUC__)
#   if __GNUC__ < 6
#       error "Minimum version of GCC for MBEDTLS_AESCE_C is 6.0."

In fact, I wonder whether we expose the above problem on older gcc by our exposure of NEON there, because a condition for having MBEDTLS_AESCE_HAVE_CODE defined is defined(MBEDTLS_ARCH_IS_ARMV8_A) && defined(MBEDTLS_HAVE_NEON_INTRINSICS). Otherwise, without MBEDTLS_AESCE_HAVE_CODE, the source file looks like it's intended to compile with gcc older than 6 too - it even has explicit checks for gcc 5 further down the file.

So maybe just drop our hack, and don't patch anything in Mbed-TLS, but do retest that AES-CE is still enabled where we previously tested that it was.

Or alternatively we need to patch Mbed-TLS some more, also adding !defined(__GNUC__) || __GNUC__ >= 6 as a condition for defining MBEDTLS_AESCE_HAVE_CODE.

[magnumripper]

Not sure I understood all you said but FWIW my M1's native gcc defines both, plus __AARCH64_SIMD__:

$ gcc -dM -E -x c /dev/null | grep -Ei 'arch|neon' | sort
#define __AARCH64EL__ 1
#define __AARCH64_CMODEL_SMALL__ 1
#define __AARCH64_SIMD__ 1
#define __ARM64_ARCH_8__ 1
#define __ARM_ARCH 8
#define __ARM_ARCH_8_3__ 1
#define __ARM_ARCH_8_4__ 1
#define __ARM_ARCH_8_5__ 1
#define __ARM_ARCH_ISA_A64 1
#define __ARM_ARCH_PROFILE 'A'
#define __ARM_NEON 1
#define __ARM_NEON_FP 0xE
#define __ARM_NEON__ 1
#define __aarch64__ 1

Using homebrew's gcc instead, I don't get __AARCH64_SIMD__ but the other two, plus an __ARM_NEON_SVE_BRIDGE whatever that means:

$ gcc-14 -dM -E -x c /dev/null | grep -Ei 'arch|neon' | sort
#define __AARCH64EL__ 1
#define __AARCH64_CMODEL_SMALL__ 1
#define __ARM_ARCH 8
#define __ARM_ARCH_8A 1
#define __ARM_ARCH_ISA_A64 1
#define __ARM_ARCH_PROFILE 65
#define __ARM_NEON 1
#define __ARM_NEON_SVE_BRIDGE 1
#define __aarch64__ 1

I tried building 32-bit but then the arch drops to 4T (?) and I can't link

$ gcc -dM -E -x c /dev/null -m32 | grep -Ei 'arch|neon'
#define __ARM_ARCH 4
#define __ARM_ARCH_4T__ 1
#define __ARM_ARCH_ISA_ARM 1
#define __ARM_ARCH_ISA_THUMB 1

Using homebrew's gcc-14, I don't even get that far

gcc-14: error: unrecognized command-line option '-m32'