build: require --openssl-no-asm if no suitable assembler

[rvagg]

Also includes a refactor to extract error(), like warn() and make the messages a bit more consistent.

The main behaviour change here is to fail ./configure if you don't have an assembler new enough, which forces you to supply --openssl-no-asm rather than doing it implicitly. The cost of no-asm is that you get binaries that are potentially a lot slower, it's not something you should walk blindly in to. Discussion around this is primarily in #19944

I'd really like to get this in to Node 10.0.0 so we don't have to argue about the semver-major nature of this. I know it's very late but I'd been assuming this was already done since we talked about it. All of these versions are pretty low (e.g. gcc 4.9 comes with binutils 2.25 and we're asking for 2.23), so aside from Windows (where we have it opt-in via vcbuild.bat already) and some of the exotics, I don't believe many (or any?) would be experiencing this since they already need a fairly modern toolchain for the C++ features we're now needing to support.

When we go to OpenSSL 1.1.1 these minimums are going to rise, but, it's my understanding that OpenSSL will only use the maximum that it has so the fall-back isn't as painful and we'll just need to provide a warning that you have suboptimal (but not terrible) binaries. I don't fully understand the $avx conditionals in the OpenSSL code and need @shigeki @bnoordhuis or someone else to help me with this. If this isn't the case then we're in a bit of trouble because gas gets bumped to 2.26 (or maybe even 3.0) and we might need to consider doing it right now!

This PR fails on AIX, we're going to have to put --openssl-no-asm for Node 10+ there until we get a newer assembler.

@shigeki @bnoordhuis @danbev @jasnell @nodejs/build

[shigeki]

I prefer to provide asm supports for older assemblers rather than making configure failure. It can be semver-minor changes.

[rvagg]

@shigeki if/when we have that we could remove this entirely, it's going to be much easier to remove than add back in something like this

[shigeki]

@rvagg Okay. I think it is better to make this limited to x86 architectures because assembler version checks are made in x86_64 and x86.

ohtsu@obu:~/github/openssl/crypto$ git grep -i 'gnu assembler version'
aes/asm/aesni-mb-x86_64.pl:             =~ /GNU assembler version ([2-9]\.[0-9]+)/) {
aes/asm/aesni-sha1-x86_64.pl:           =~ /GNU assembler version ([2-9]\.[0-9]+)/ &&
aes/asm/aesni-sha256-x86_64.pl:         =~ /GNU assembler version ([2-9]\.[0-9]+)/) {
bn/asm/rsaz-avx2.pl:            =~ /GNU assembler version ([2-9]\.[0-9]+)/) {
bn/asm/rsaz-x86_64.pl:          =~ /GNU assembler version ([2-9]\.[0-9]+)/) {
bn/asm/x86_64-mont.pl:          =~ /GNU assembler version ([2-9]\.[0-9]+)/) {
bn/asm/x86_64-mont5.pl:         =~ /GNU assembler version ([2-9]\.[0-9]+)/) {
chacha/asm/chacha-x86.pl:                       =~ /GNU assembler version ([2-9]\.[0-9]+)/ &&
chacha/asm/chacha-x86_64.pl:            =~ /GNU assembler version ([2-9]\.[0-9]+)/) {
ec/asm/ecp_nistz256-avx2.pl:            =~ /GNU assembler version ([2-9]\.[0-9]+)/) {
ec/asm/ecp_nistz256-x86_64.pl:          =~ /GNU assembler version ([2-9]\.[0-9]+)/) {
ec/asm/x25519-x86_64.pl:                =~ /GNU assembler version ([2-9]\.[0-9]+)/) {
modes/asm/aesni-gcm-x86_64.pl:          =~ /GNU assembler version ([2-9]\.[0-9]+)/) {
modes/asm/ghash-x86_64.pl:              =~ /GNU assembler version ([2-9]\.[0-9]+)/) {
poly1305/asm/poly1305-x86.pl:                   =~ /GNU assembler version ([2-9]\.[0-9]+)/) {
poly1305/asm/poly1305-x86_64.pl:                =~ /GNU assembler version ([2-9]\.[0-9]+)/) {
sha/asm/sha1-586.pl:                    =~ /GNU assembler version ([2-9]\.[0-9]+)/ &&
sha/asm/sha1-mb-x86_64.pl:              =~ /GNU assembler version ([2-9]\.[0-9]+)/) {
sha/asm/sha1-x86_64.pl:         =~ /GNU assembler version ([2-9]\.[0-9]+)/) {
sha/asm/sha256-586.pl:                  =~ /GNU assembler version ([2-9]\.[0-9]+)/) {
sha/asm/sha256-mb-x86_64.pl:            =~ /GNU assembler version ([2-9]\.[0-9]+)/) {
sha/asm/sha512-x86_64.pl:               =~ /GNU assembler version ([2-9]\.[0-9]+)/) {

This diff can be workable.

diff --git a/configure b/configure
index 0294e3a..c1170f9 100755
--- a/configure
+++ b/configure
@@ -1106,6 +1106,8 @@ def configure_openssl(o):
       o['defines'] += ['NODE_WITHOUT_NODE_OPTIONS']

   if not options.shared_openssl and not options.openssl_no_asm:
+    is_x86 = 'x64' in variables['target_arch'] or 'ia32' in variables['target_arch']
+
     # supported asm compiler for AVX2. See https://github.com/openssl/openssl/
     # blob/OpenSSL_1_1_0-stable/crypto/modes/asm/aesni-gcm-x86_64.pl#L52-L69
     openssl110_asm_supported = \
@@ -1114,7 +1116,7 @@ def configure_openssl(o):
       ('llvm_version' in variables and float(variables['llvm_version']) >= 3.3) or \
       ('nasm_version' in variables and float(variables['nasm_version']) >= 2.10)

-    if not openssl110_asm_supported:
+    if is_x86 and not openssl110_asm_supported:
       error('''Did not find a new enough assembler, install one or build with
        --openssl-no-asm.
        Please refer to BUILDING.md''')

[shigeki]

Checking CI in https://ci.nodejs.org/job/node-test-commit/17948/ to see if AIX and others not x86 work fine with my additional patch.

[jasnell]

This would need @nodejs/tsc approval to fast track. It would need to land before noon Pacific time today to make it in to 10.0.0.

[shigeki]

Thre results of https://ci.nodejs.org/job/node-test-commit/17948/ are fine except debian8-64 where only one http2 flaky test was failed.
Both AIX and S390 can be built with asm support by using current aix assembler and their CI results are fine. This solves #19944 (comment).

[mhdawson]

I don't think this is going to work on AIX as I believe we are not using gas or one of the other options. I think at this point we should just exclude the check on AIX as its so late in the game.

[mhdawson]

I see the suggestion from @shigeki, that makes sense to me.

[mhdawson]

If we add the check I'm +1 for only doing the check for x86 platforms.

[shigeki]

I submit a new PR of this with my additional commit in #20226.

[shigeki]

Closed by #20226.