-
Notifications
You must be signed in to change notification settings - Fork 30.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Require openssl-no-asm option explicitly if older assembler version is found. #19944
Comments
+1 consistent with #19943 if we accept that one too. I'm going with @bnoordhuis on this one that you should opt in. Your notes about versions had me wondering if our infra even supports this and our Linux release builders didn't until today! So nightlies, and the last 10.0.0 test build for Linux (@ https://nodejs.org/download/test/v10.0.0-test20180410edd9cc466a/) would have been built without asm support if it was falling back to no-asm silently. I also still need to make sure our macOS machines are fully updated with xcode too. It would have been nice to get build failures because |
When we make opt-in, we should consider the future upgrade to OpenSSL-1.1.1 which needs higher version so as not to be semver-major update. Following to the requirements in 1.1.1, asm support is #### OpenSSL asm support
OpenSSL requires the following asssembler version for use of asm
support.
* gas (GNU assembler) version 2.26 or higher
* xcode version 5.0 or higher
* llvm version 3.3 or higher
* nasm version 2.11.8 or higher in Windows
Otherwise, build with `--openssl-no-asm` in configure.
*Note:* This requirements come from the forthcoming OpenSSL-1.1.1. See
https://www.openssl.org/docs/man1.1.1/man3/OPENSSL_ia32cap.html for
details. At the current ,the following CI would be failed due to the older assembler version.
|
ppcle, s390x how use higher versions for Node 10.x and higher. aix we'll have to check why it reports as no_asm. |
as is on the machine but reports: ^C-bash-4.3$ as --version |
@gireeshpunathil can you take a look at the requirements for as in openssl for AIX to figure out if we need an additional or newer version of the as tool or if the detection logic needs a tweek. |
sure |
That's almost certainly not the GNU assembler and looks like it is the AIX assembler. Node.js' Lines 679 to 701 in 0857790
on one of IBM's internal AIX systems, the equivalent
which doesn't match the regexp so we end up with Does openssl support AIX's assembler? cc @nodejs/platform-aix |
I found that OpenSSL built on s390 does not check assembler version.
and then give me build sources in tar ball. |
And also please give me a stdout logs of |
cc @nodejs/platform-s390 @nodejs/build |
@jBarz @mmalecki can you help @shigeki get what he needs? @shigeki we can also get you access to one of the linuxOne machines in the community CI as well. I've opened the issue to request access for you here: nodejs/build#1246 |
What does "check assembler version" mean here? Are you suggesting the sha512-s390x.pl check for |
Other architectures have checks assembler version such as node/deps/openssl/openssl/crypto/poly1305/asm/poly1305-x86_64.pl Lines 57 to 74 in 8b012e1
|
PR-URL: #20226 Fixes: #19944 Refs: #20217 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Richard Lau <[email protected]>
The current openssl checks assembler version only x86_64 or ia32 target arch for use of AES-NI, AVX and AVX2. This requires --openssl-no-asm option during configure when an older assembler version is found only on x86_64 or ia32. PR-URL: #20226 Fixes: #19944 Refs: #20217 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Richard Lau <[email protected]>
PR-URL: #20226 Fixes: #19944 Refs: #20217 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Richard Lau <[email protected]>
PR-URL: #20226 Fixes: #19944 Refs: #20217 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Richard Lau <[email protected]>
The current openssl checks assembler version only x86_64 or ia32 target arch for use of AES-NI, AVX and AVX2. This requires --openssl-no-asm option during configure when an older assembler version is found only on x86_64 or ia32. PR-URL: #20226 Fixes: #19944 Refs: #20217 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Richard Lau <[email protected]>
Currently openssl asm support requires assembler versions as described in BUILDING.md and automatically falls back to no-asm build with warning.
Should we stop building then require and opt-in to have --openssl-no-asm option in that case?
CC @nodejs/build
Ref #19918
The text was updated successfully, but these errors were encountered: