1608327 - Two problems with NEON-specific code in freebl

Status: RESOLVED FIXED

(NSS :: Build, defect)

Description

[Mike Hommey [:glandium]]

Because both problems are related, I'm filing a single bug for both.

The first problem is that bug 1590676 disabled the armv8 AES code entirely in builds where the compiler doesn't have neon enabled, even though the calling code has runtime detection of the necessary CPU features. https://hg.mozilla.org/projects/nss/file/4921046404f197526969a6b79f19c136469e69f8/lib/freebl/rijndael.c#l861

The core problem that bug 1590676 tried to address is that aes-armv8.c can't be built when the compiler defaults to the softfloat float abi. But aes-armv8.c is built explicitly with -mfpu=crypto-neon-fp-armv8, which is incompatible.

However, the softfp float abi is compatible with softfloat, so a better way to fix bug 1590676 is to switch to that float abi when building aes-armv8.c. The tricky part, however, is that the softfp ABI is not compatible with the hardfp ABI, so it's not possible to unconditionally use -mfloat-abi=softfp. It should only be used when the compiler would default to softfloat.

The second problem is that libfreeblpriv3.so fails to link when building with a compiler that defaults to the hardfp ABI but doesn't enable neon by default. The failure is:

OBJS/Linux_SINGLE_SHLIB/gcm-arm32-neon.o: In function `gcm_HashMult_hw':
./nss/lib/freebl/gcm-arm32-neon.c:111: multiple definition of `gcm_HashMult_hw'
OBJS/Linux_SINGLE_SHLIB/gcm.o:./nss/lib/freebl/gcm.c:56: first defined here
OBJS/Linux_SINGLE_SHLIB/gcm-arm32-neon.o: In function `gcm_HashMult_hw':
./nss/lib/freebl/gcm-arm32-neon.c:111: multiple definition of `gcm_HashWrite_hw'
OBJS/Linux_SINGLE_SHLIB/gcm.o:./nss/lib/freebl/gcm.c:407: first defined here
OBJS/Linux_SINGLE_SHLIB/gcm-arm32-neon.o: In function `gcm_HashMult_hw':
./nss/lib/freebl/gcm-arm32-neon.c:111: multiple definition of `gcm_HashInit_hw'
OBJS/Linux_SINGLE_SHLIB/gcm.o:./nss/lib/freebl/gcm.c:407: first defined here
OBJS/Linux_SINGLE_SHLIB/gcm-arm32-neon.o: In function `gcm_HashMult_hw':
./nss/lib/freebl/gcm-arm32-neon.c:111: multiple definition of `gcm_HashZeroX_hw'
OBJS/Linux_SINGLE_SHLIB/gcm.o:./nss/lib/freebl/gcm.c:407: first defined here
collect2: error: ld returned 1 exit status

The problem here is that gcm.c is built with the default compiler flags, and it exposes those gcm_hw functions when neon is disabled, which it is. But gcm-arm32-neon.c is built with -mfpu=neon, which enables neon and also makes it expose gcm_hw functions.

The solution here is that gcm.c should not, in fact have those fallbacks enabled in this case. The calling code already runtime checks whether neon is available: https://hg.mozilla.org/projects/nss/file/4921046404f197526969a6b79f19c136469e69f8/lib/freebl/gcm.c#l105

Comment 1

[Mike Hommey [:glandium]]

Attachment: Bug 1608327 - Fix freebl arm NEON code use on tier3 platforms.

Despite the code having runtime detection of NEON and crypto extensions,
the optimized code using those instructions is disabled at build time on
platforms where the compiler doesn't enable NEON by default of with the
flags it's given for the caller code.

In the case of gcm, this goes as far as causing a build error.

What is needed is for the optimized code to be enabled in every case,
letting the caller code choose whether to use that code based on the
existing runtime checks.

But this can't be simply done either, because those optimized parts of
the code need to be built with NEON enabled, unconditionally, but that
is not compatible with platforms using the softfloat ABI. For those,
we need to use the softfp ABI, which is compatible. However, the softfp
ABI is not compatible with the hardfp ABI, so we also can't
unconditionally use the softfp ABI, so we do so only when the compiler
targets the softfloat ABI, which confusingly enough is advertized via
the SOFTFP define.

Comment 2

[J.C. Jones [:jcj] (he/they)]

https://hg.mozilla.org/projects/nss/rev/dc57fe5d65d4bb9e407cdd7ef3bfffdb5c77ebb8

Comment 3

[J.C. Jones [:jcj] (he/they)]

Build failures on some platforms: gyp: Undefined variable softfp_cflags in /home/worker/nss/lib/freebl/freebl.gyp

Working on a fixup

Comment 4

[J.C. Jones [:jcj] (he/they)]

fixup try push https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=9df3866d445f

Comment 5

[J.C. Jones [:jcj] (he/they)]

https://hg.mozilla.org/projects/nss/rev/05b923624b730eeb976f068cb3ad190f9179bd29

Comment 6

[Edouard Oger [:eoger]]

We're also affected by this problem on application-services: https://github.com/mozilla/application-services/pull/2476, would it be worth making a dot release?

Comment 7

[tt_1]

a point release including both fixes would be very helpfull.

Comment 8

[J.C. Jones [:jcj] (he/they)]

A point release of 3.49, just to confirm? 3.50 is scheduled for 7th of February

Comment 9

[J.C. Jones [:jcj] (he/they)]

NSS_3_49_BRANCH:
https://hg.mozilla.org/projects/nss/rev/817845f169bd9ad72de0bcfc06e7ccdb23596f48
https://hg.mozilla.org/projects/nss/rev/985638c1e9c674fe8ee3a62b56034cecb6297ec2

Comment 10

[tt_1]

hey there, through some very fancy stalking I found this bug, and I believe it's either causing another problem or it hasn't properly solved the initial problem. I already have opened another bug at bug 1647049 , do you think it's possible for you to have a look at it and either confirm or not confirm it's the same problem? thank you