Closed Bug 1608327 Opened 2 years ago Closed 2 years ago

Two problems with NEON-specific code in freebl

Categories

(NSS :: Build, defect)

3.49
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: glandium, Assigned: glandium, NeedInfo)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

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

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.

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.50

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

Working on a fixup

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED

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?

a point release including both fixes would be very helpfull.

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

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

Flags: needinfo?(mh+mozilla)
You need to log in before you can comment on or make changes to this bug.