deps: rewrite openssl x86asm.pl win32 workaround - #60510
Open
jaanrebane wants to merge 5 commits into
Open
Conversation
Copy link
Copy Markdown
Sorry, something went wrong.
Some sed lines were previously used to change from C-style #ifdef to nasm-style %ifdef in x86asm.pl for 32-bit Windows builds, but this creates problems when the C preprocessor is used before the assembler to build x86 assembly files. OpenSSL is using C preprocessor before nasm and uses #ifdef in this context. The perl line added to update-openssl.sh will work around the ifdef issue in a way that enables building for win32 and other x86. After update-openssl.sh script is run with "regenerate", x86asm.pl will end up with a modified "endbranch" subroutine that allows to use 2 types of ifdef (nasm-style %ifdef for win32 and gcc-style #ifdef for others). Then, after x86asm.pl is run during the node openssl build process, x86 assembly files may change their ifdef and endif lines depending on the system they are built for.
The file deps/openssl/openssl/crypto/perlasm/x86asm.pl that is creating assembly code for x86, now creates nasm-style %ifdef and %endif for win32 and gcc-style #ifdef and #endif for all other x86. A large number of files in this commit are here after re-running the openssl update script: tools/dep_updaters/update-openssl.sh regenerate Assembler files for non-win32 x86 get a %ifdef to #ifdef modification, buildinf.h files only see a DATE change, configdata.pm files only see a RANLIB change.
Using perl in not allowed from https://github.com/nodejs/node/blob/HEAD/doc/README.md#code-blocks although the ifdef syntax change always has been in perl. Until perl is enabled, use text.
Copy link
Copy Markdown
Collaborator
|
Review requested:
|
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
|
This pull request has been marked as stale due to 90 days of inactivity. |
Sorry, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The win32 builds have been using a workaround for building the openssl dependency. This breaks building x86 assembler files for unsupported architectures (32-bit linux-elf etc).
These git commits aim to solve the situation in a more inclusive way by adding an if statement to the perl script x86asm.pl that is used to create the assembler files. This way, win32 builds still get assembler files with %ifdef syntax, which is how Node has been building them so far. Official OpenSSL has #ifdef and not %ifdef in x86asm.pl.
There are actually only 2 necessary modifications:
Other changes in this pull request happen automatically after running tools/dep_updaters/update-openssl.sh regenerate:
The .S files that openssl creates are supposed to go through the C preprocessor before using the assembler, and cpp doesn't understand the %ifdef syntax, it needs #ifdef. Although 32-bit Linux is no longer supported, people still build for x86 linux (and maybe other x86) and the workaround for building for linux-elf has been so far to revert the %ifdef syntax back to original (openssl source) #ifdef.
In any case, if this pull request gets rejected, after Node drops support for all win32 releases, there will be no need to keep the ifdef modification, and after that, node openssl dependency will be building again for linux x86.
References: