1647049 - if firefox-78 is configured via --with-float-abi=hard and internal nss and nspr on armv7 it fails with a fatal error: You must enable crypto instructions to use these intrinsics in: return __builtin_arm_crypto_aesd (__data, __key);

Status: UNCONFIRMED

(NSS :: Build, defect, P3)

Description

[tt_1]

Attachment: firefox-78.0_beta9-failed-build.log.xz

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

I'm testing firefox beta branch on a regular base in a cross compile setup for compile failures, and also do some basic runtime testing if there are no issues. Since my distro couldn't push new nss-3.53 fast enough, which is mandantory for firefox-78.0 branch, I disabled --with-system-nspr and --with-system-nss switches for now until standalone nss got fixed.

I'm using --enable-lto=thin --with-float-abi=hard --with-fpu=neon --with-thumb=yes --with-thumb-interwork=no

Actual results:

the build failures during lto linkage with:

25:16.87 /usr/bin/armv7a-unknown-linux-gnueabihf-gcc -std=gnu99 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -pipe -mthumb -mno-thumb-interwork -mfpu=neon -mfloat-abi=hard -fno-strict-aliasing -ffunction-sections -fdata-sections -fno-math-errno -pthread -pipe -O2 -fomit-frame-pointer -funwind-tables -Wall -Wempty-body -Wignored-qualifiers -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wduplicated-cond -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=coverage-mismatch -Wno-error=free-nonheap-object -Wno-multistatement-macros -Wno-error=class-memaccess -Wno-error=deprecated-copy -Wformat -Wformat-security -Wformat-overflow=2 -fPIC -shared -Wl,-z,defs -Wl,--gc-sections -Wl,-h,libfreeblpriv3.so -o libfreeblpriv3.so /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-78.0_beta9/work/firefox-78.0/ff/security/nss/lib/freebl/freebl_freeblpriv3/libfreeblpriv3_so.list -flto=12 -flifetime-dse=1 -lpthread -Wl,-O1 -Wl,--as-needed -Wl,--no-keep-memory -Wl,-rpath=/usr/lib/firefox,--enable-new-dtags -Wl,--compress-debug-sections=zlib -fuse-ld=gold -mthumb -Wl,-z,noexecstack -Wl,-z,text -Wl,-z,relro -Wl,-z,nocopyreloc -Wl,-Bsymbolic-functions -Wl,--icf=safe -fstack-protector-strong -Wl,-rpath-link,/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-78.0_beta9/work/firefox-78.0/ff/dist/bin -Wl,-rpath-link,/usr/lib -fdiagnostics-color ../../../../../config/external/nspr/pr/libnspr4.so ../../../../../config/external/nspr/libc/libplc4.so ../../../../../config/external/nspr/ds/libplds4.so -Wl,--version-script,out.freebl_hash_vector.def -ldl -lpthread -ldl -lc
25:16.87 /usr/libexec/gcc/armv7a-unknown-linux-gnueabihf/ld.gold: warning: wildcard match appears in both version 'NSSprivate_3.11' and 'NSSprivate_3.16' in script
25:16.87 /usr/lib/gcc/armv7a-unknown-linux-gnueabihf/9.3.0/include/arm_neon.h: In function 'arm_aes_decrypt_cbc_256':
25:16.87 /usr/lib/gcc/armv7a-unknown-linux-gnueabihf/9.3.0/include/arm_neon.h:16920:10: fatal error: You must enable crypto instructions (e.g. include '-mfloat-abi=softfp' '-mfpu=crypto-neon') to use these intrinsics.
25:16.87 16920 | return __builtin_arm_crypto_aesd (__data, __key);
25:16.87 | ^
25:16.87 compilation terminated.
25:16.87 make[5]: *** [/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-78.0_beta9/temp/ccg9hQFC.mk:2: /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-78.0_beta9/temp/libfreeblpriv3.so.zWqkmz.ltrans0.ltrans.o] Error 1

the full build log is attached.

Expected results:

the build should have passed without any problems.

the failure must be somewhere in lto, if I configure without system-nspr and system-nss, but also don't use --enable-lto, the build compiles without any failure and there are no runtime issues.

also reverting nss and nspr back to system makes the bug vanish

Comment 1

[tt_1]

random quote from the build log, to proof that I'm using hardfloat via -mfloat-abi=hard

4:19.48 /usr/bin/armv7a-unknown-linux-gnueabihf-g++ -std=gnu++17 -o Unified_cpp_xpcom_io0.o -c -flto -flifetime-dse=1 -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-78.0_beta9/work/firefox-78.0/ff/dist/stl_wrappers -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-78.0_beta9/work/firefox-78.0/ff/dist/system_wrappers -include /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-78.0_beta9/work/firefox-78.0/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -DNDEBUG=1 -DTRIMMED=1 -DOS_POSIX=1 -DOS_LINUX=1 -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-78.0_beta9/work/firefox-78.0/xpcom/io -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-78.0_beta9/work/firefox-78.0/ff/xpcom/io -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-78.0_beta9/work/firefox-78.0/ff/ipc/ipdl/_ipdlheaders -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-78.0_beta9/work/firefox-78.0/ipc/chromium/src -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-78.0_beta9/work/firefox-78.0/ipc/glue -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-78.0_beta9/work/firefox-78.0/ff/xpcom -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-78.0_beta9/work/firefox-78.0/xpcom/build -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-78.0_beta9/work/firefox-78.0/ff/dist/include -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-78.0_beta9/work/firefox-78.0/ff/dist/include/nspr -I/usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-78.0_beta9/work/firefox-78.0/ff/dist/include/nss -I/usr/armv7a-unknown-linux-gnueabihf/usr/include/pixman-1 -fPIC -DMOZILLA_CLIENT -include /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-78.0_beta9/work/firefox-78.0/ff/mozilla-config.h -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wduplicated-cond -Wimplicit-fallthrough -Wunused-function -Wunused-variable -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=coverage-mismatch -Wno-error=free-nonheap-object -Wno-multistatement-macros -Wno-error=class-memaccess -Wno-error=deprecated-copy -Wformat -Wformat-security -Wformat-overflow=2 -fno-sized-deallocation -fno-aligned-new -pipe -flifetime-dse=1 -Wno-psabi -Wno-class-memaccess -Wno-int-in-bool-context -Wno-multistatement-macros -Wno-maybe-uninitialized -Wno-deprecated-declarations -mthumb -mno-thumb-interwork -mfpu=neon -mfloat-abi=hard -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -O2 -fomit-frame-pointer -funwind-tables -MD -MP -MF .deps/Unified_cpp_xpcom_io0.o.pp -fdiagnostics-color Unified_cpp_xpcom_io0.cpp

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 3

[tt_1]

after some grepping I take an educated guess that the build script believes it is compiling for armv8:

grep -r arm_aes_decrypt_cbc_256

security/nss/lib/freebl/aes-armv8.c:arm_aes_decrypt_cbc_256(AESContext *cx, unsigned char *output,
security/nss/lib/freebl/aes-armv8.h:SECStatus arm_aes_decrypt_cbc_256(AESContext *cx, unsigned char *output,
security/nss/lib/freebl/aes-armv8.h: : arm_aes_decrypt_cbc_256))

which is confirmed by asking for enablement of

-mfpu=crypto-neon

which is armv8 only, according to my research

Comment 4

[tt_1]

I think this area of code is responsible:

https://searchfox.org/mozilla-central/source/security/nss/lib/freebl/Makefile#706

this block is guarded by ifeq ($(CPU_ARCH),arm) , why does this have anything related to armv8 in it? armv8 is Aarch64 only according to my research: http://infocenter.arm.com/help/topic/com.arm.doc.100067_0608_00_en/chr1411547793198.html

Comment 5

[tt_1]

ok, so I dug a bit deeper into this. The whole softfloat detection is a bit wacky

-march armv8-a is clang only, gcc doesn't understand that. And I believe the softfloat detection in this is clang only too, so that might be a good explanation for why this fails with gcc.

But I don't understand how the use of internal nss and nspr ends in a failure during lto linking, while all is fine with both system-nss and system-nspr

Comment 6

[Ricky Stewart]

Linux on ARMv7 is a tier-3 supported platform so we're limited in the amount of support we can provide here.

That Makefile is from NSS which is vendored, non-m-c code. NSS can be built a few ways, including make, but they have a preference for gyp/ninja as well -- I assume your system NSS/NSPR are built using one of these tools.

I'll re-assign the bug to NSS so they can speak to the actual issue here. Can an NSS expert address how/if building for Linux on ARMv7 is supposed to work? If so, how can we get this so that it works in the Firefox build? I assume the files aes-armv8.{c,h} are only meant to be used for ARMv8, and not ARMv7, but there isn't an aes-armv7.{c,h} in-tree, so I don't know what the "easy" solution (if there is one) would be.

Comment 7

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

We do have a fallback codepath for AES, but obviously the .gyp files [0] aren't catching the case well.

We'll look at this closer next week, but we'd definitely take a patch. We don't have an ARM7 environment to test this with.

[0] https://searchfox.org/nss/source/lib/freebl/freebl.gyp

Comment 8

[tt_1]

Ricky Stewart already pointed me into the correct direction, the standalone nss of my distro carries a patch which adds a Makefile - this made me believe that also the internal version of nss is built with a the Makefile from the freebl folder. But all my tinkering with that Makefile didn't made a difference, since ninja is used for the error presenting in this bug.

After reading through nss/source/lib/freebl/freebl.gyp I see things a bit clearer now.

first of all, the gyp file tries to exploit that there's something as a 32bit mode for armv8, but that's clang specific. Using gcc for compiling will make it choke in exactly this way:

25:16.87 /usr/lib/gcc/armv7a-unknown-linux-gnueabihf/9.3.0/include/arm_neon.h:16920:10: fatal error: You must enable crypto instructions (e.g. include '-mfloat-abi=softfp' '-mfpu=crypto-neon') to use these intrinsics.

my suggestion is to stub all of these sections out with some sort of clangonly statement. Maybe cc_is_clang==1 will do the trick, it's already used in the gyp file.

secondly, aes-armv8.h is included into nss/lib/freebl/rijndael.c - without any determination it seems, and causes the second error:

25:16.87 /usr/lib/gcc/armv7a-unknown-linux-gnueabihf/9.3.0/include/arm_neon.h: In function 'arm_aes_decrypt_cbc_256':

I still have to find out more about how to fix that

also it remains a mystery to me, why all this is triggered by --enable-lto=thin ; building firefox with system-nspr + system-nss without the lto features passes just fine.

Comment 9

[tt_1]

Attachment: 0001-firefox-78.0-remove-aarch64-reference.patch

I've never written a patch for firefox from scratch, here's what I believe a good solution to mitigate the problem for users of gcc.

I've stubbed out all problematic parts for gcc via cc_is_clang==1

It should only make a difference for a compiler that is either arm, arm64 or aarch64, and which is gcc

please review the patch, thank you!

p.s: I may split the patch into individual parts for the two different problems it adresses, if it's acceptable.

Comment 10

[tt_1]

Attachment: 0001-firefox-79.0_beta6-remove-aarch64-references.patch

updated patch for firefox-79.0

Comment 11

[Kevin Jacobs [:kjacobs]]

Comment on attachment 9163727 [details] [diff] [review]
0001-firefox-79.0_beta6-remove-aarch64-references.patch

First, thank you for the patch and for looking into this. We don't want to disable this wholesale for gcc, particularly since the problem seems to be limited to armhf with LTO. I'm not sure what you mean by "--march armv8-a is clang only", see: https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html

FWIW, aes-armv8.h is included by rijndael.c when USE_HW_AES is set. This is done optimistically by freebl.gyp. Importantly, it enables a runtime check of hardware capabilities before actually using the armv8 AES code (use_hw_aes). See https://searchfox.org/mozilla-central/source/security/nss/lib/freebl/rijndael.c#924-925.

Have you tried using -mfpu=crypto-neon-fp-armv8? You might also take a look at bug 1608327.