package/libnss: fix build failure on arm32 arch not armv7

NSS assumes that every arm32 build is an armv7, but this is not, so
let's add a patch to remove -march=armv7 flag.

Fixes:
http://autobuild.buildroot.net/results/464/464044fda2850123339de6c8071374e380636ee0/

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
---
 ...w-build-gcm-arm32-neon-on-march-armv.patch | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 package/libnss/0002-Bug-1624864-Allow-build-gcm-arm32-neon-on-march-armv.patch

Forgot to mention, patch is pending upstream:
https://bugzilla.mozilla.org/show_bug.cgi?id=1624864

On 3/25/20 3:31 PM, Giulio Benetti wrote:
> NSS assumes that every arm32 build is an armv7, but this is not, so
> let's add a patch to remove -march=armv7 flag.
> 
> Fixes:
> http://autobuild.buildroot.net/results/464/464044fda2850123339de6c8071374e380636ee0/
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> ---
>   ...w-build-gcm-arm32-neon-on-march-armv.patch | 28 +++++++++++++++++++
>   1 file changed, 28 insertions(+)
>   create mode 100644 package/libnss/0002-Bug-1624864-Allow-build-gcm-arm32-neon-on-march-armv.patch
> 
> diff --git a/package/libnss/0002-Bug-1624864-Allow-build-gcm-arm32-neon-on-march-armv.patch b/package/libnss/0002-Bug-1624864-Allow-build-gcm-arm32-neon-on-march-armv.patch
> new file mode 100644
> index 0000000000..f23a067833
> --- /dev/null
> +++ b/package/libnss/0002-Bug-1624864-Allow-build-gcm-arm32-neon-on-march-armv.patch
> @@ -0,0 +1,28 @@
> +From 345503215135466a0008ef1b7546b65e5705d0df Mon Sep 17 00:00:00 2001
> +From: Giulio Benetti <giulio.benetti@benettiengineering.com>
> +Date: Wed, 25 Mar 2020 15:02:20 +0100
> +Subject: [PATCH] Bug 1624864 - Allow build gcm-arm32-neon on -march != armv7
> +
> +Don't assume every arm32 support armv7 instruction set.
> +
> +Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> +---
> + nss/lib/freebl/Makefile | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/nss/lib/freebl/Makefile b/nss/lib/freebl/Makefile
> +index dc502f309..c7eb9c205 100644
> +--- a/nss/lib/freebl/Makefile
> ++++ b/nss/lib/freebl/Makefile
> +@@ -781,7 +781,7 @@ ifeq ($(CPU_ARCH),arm)
> + USES_SOFTFLOAT_ABI := $(shell $(CC) -o - -E -dM - $(CFLAGS) < /dev/null | grep __SOFTFP__ > /dev/null && echo 1)
> + $(OBJDIR)/$(PROG_PREFIX)aes-armv8$(OBJ_SUFFIX): CFLAGS += -march=armv8-a -mfpu=crypto-neon-fp-armv8$(if $(USES_SOFTFLOAT_ABI), -mfloat-abi=softfp)
> + ifndef NSS_DISABLE_GCM_ARM32_NEON
> +-$(OBJDIR)/$(PROG_PREFIX)gcm-arm32-neon$(OBJ_SUFFIX): CFLAGS += -march=armv7 -mfpu=neon$(if $(USES_SOFTFLOAT_ABI), -mfloat-abi=softfp)
> ++$(OBJDIR)/$(PROG_PREFIX)gcm-arm32-neon$(OBJ_SUFFIX): CFLAGS += -mfpu=neon$(if $(USES_SOFTFLOAT_ABI), -mfloat-abi=softfp)
> + endif
> + endif
> + ifeq ($(CPU_ARCH),aarch64)
> +--
> +2.20.1
> +
>

On Wed, 25 Mar 2020 15:31:08 +0100
Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:

> +diff --git a/nss/lib/freebl/Makefile b/nss/lib/freebl/Makefile
> +index dc502f309..c7eb9c205 100644
> +--- a/nss/lib/freebl/Makefile
> ++++ b/nss/lib/freebl/Makefile
> +@@ -781,7 +781,7 @@ ifeq ($(CPU_ARCH),arm)
> + USES_SOFTFLOAT_ABI := $(shell $(CC) -o - -E -dM - $(CFLAGS) < /dev/null | grep __SOFTFP__ > /dev/null && echo 1)
> + $(OBJDIR)/$(PROG_PREFIX)aes-armv8$(OBJ_SUFFIX): CFLAGS += -march=armv8-a -mfpu=crypto-neon-fp-armv8$(if $(USES_SOFTFLOAT_ABI), -mfloat-abi=softfp)
> + ifndef NSS_DISABLE_GCM_ARM32_NEON
> +-$(OBJDIR)/$(PROG_PREFIX)gcm-arm32-neon$(OBJ_SUFFIX): CFLAGS += -march=armv7 -mfpu=neon$(if $(USES_SOFTFLOAT_ABI), -mfloat-abi=softfp)
> ++$(OBJDIR)/$(PROG_PREFIX)gcm-arm32-neon$(OBJ_SUFFIX): CFLAGS += -mfpu=neon$(if $(USES_SOFTFLOAT_ABI), -mfloat-abi=softfp)

But only ARMv7 can have NEON support support. So removing just
-march=armv7 doesn't make sense. Actually, the issue is probably that
they should be using -march=armv7-a, because -march=armv7 is the common
subset of ARMv7-A, ARMv7-M and ARMv7-R, which I suppose doesn't include
NEON.

So while the patch work, its description is misleading at best, and
having -march=armv7-a here is probably the right fix.

Best regards,

Thomas

Hi Thomas,

On 3/25/20 3:45 PM, Thomas Petazzoni wrote:
> On Wed, 25 Mar 2020 15:31:08 +0100
> Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:
> 
>> +diff --git a/nss/lib/freebl/Makefile b/nss/lib/freebl/Makefile
>> +index dc502f309..c7eb9c205 100644
>> +--- a/nss/lib/freebl/Makefile
>> ++++ b/nss/lib/freebl/Makefile
>> +@@ -781,7 +781,7 @@ ifeq ($(CPU_ARCH),arm)
>> + USES_SOFTFLOAT_ABI := $(shell $(CC) -o - -E -dM - $(CFLAGS) < /dev/null | grep __SOFTFP__ > /dev/null && echo 1)
>> + $(OBJDIR)/$(PROG_PREFIX)aes-armv8$(OBJ_SUFFIX): CFLAGS += -march=armv8-a -mfpu=crypto-neon-fp-armv8$(if $(USES_SOFTFLOAT_ABI), -mfloat-abi=softfp)
>> + ifndef NSS_DISABLE_GCM_ARM32_NEON
>> +-$(OBJDIR)/$(PROG_PREFIX)gcm-arm32-neon$(OBJ_SUFFIX): CFLAGS += -march=armv7 -mfpu=neon$(if $(USES_SOFTFLOAT_ABI), -mfloat-abi=softfp)
>> ++$(OBJDIR)/$(PROG_PREFIX)gcm-arm32-neon$(OBJ_SUFFIX): CFLAGS += -mfpu=neon$(if $(USES_SOFTFLOAT_ABI), -mfloat-abi=softfp)
> 
> But only ARMv7 can have NEON support support. So removing just
> -march=armv7 doesn't make sense. Actually, the issue is probably that
> they should be using -march=armv7-a, because -march=armv7 is the common
> subset of ARMv7-A, ARMv7-M and ARMv7-R, which I suppose doesn't include
> NEON.
> 
> So while the patch work, its description is misleading at best,

yes, sorry

> and
> having -march=armv7-a here is probably the right fix.

But build failure we have here:
https://gitlab.com/buildroot.org/buildroot/-/jobs/483712746
is on cortex-A53 in 32-bit mode, so NEON is present and we are on arm,
but it is an armv8, not armv7, so we can't pass -march=armv7 to gcc.

Also they have added -march=armv7 to fix an armv6 build failure:
https://bugzilla.mozilla.org/show_bug.cgi?id=1612177
but how can that work on armv6? Probably it builds because -march=armv7 
emits NEON macro in some way, but it shouldn't run correctly.

So basically with a better description I think my patch is correct.
The problem here is the case on armv8(32-bits mode) with -march=armv7

Another way is to avoid building gcm-arm32-neon.c on armv8 in 32 bits mode.

What do you think?

Kind regards

Hi Thomas,

just sent a v2 for this with upstreamed patch:
https://patchwork.ozlabs.org/patch/1266204/

Best regards