Skip to content

Backport/gh 7427 #8399

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

Closed
wants to merge 2 commits into from
Closed

Conversation

indutny
Copy link
Member

@indutny indutny commented Sep 3, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

deps

Description of change

Backport of two commits from the master branch.

cc @thealphanerd

Use `msvs_settings.MASM.UseSafeExceptionHandlers` when building OpenSSL
assembly code on Windows. This option appends `/safeseh` to the list of
assembler flags when building `.asm` files on Windows.

Having this option in place, separate rules in `masm_compile.gypi` are
no longer needed.

Fix: nodejs#7426
PR-URL: nodejs#7427
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Bert Belder <[email protected]>
`ml64.exe` doesn't support `/safeseh` option. Do not attempt to use it
if `target_arch=="x64"`.

See: https://msdn.microsoft.com/en-us/library/s0ksfwcf.aspx
PR-URL: nodejs#7759
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Robert Jefe Lindstaedt <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
@nodejs-github-bot nodejs-github-bot added the openssl Issues and PRs related to the OpenSSL dependency. label Sep 3, 2016
@mscdex mscdex added v4.x windows Issues and PRs related to the Windows platform. labels Sep 3, 2016
@jasnell
Copy link
Member

jasnell commented Sep 4, 2016

LGTM if CI is green.

@indutny
Copy link
Member Author

indutny commented Sep 4, 2016

@indutny
Copy link
Member Author

indutny commented Sep 4, 2016

CI is green except some unrelated failures on ARM. Landing.

@indutny
Copy link
Member Author

indutny commented Sep 4, 2016

Ah, actually. @Fishrock123 I'll let you land this one when you'll be ready for it :)

@Fishrock123
Copy link
Contributor

@indutny huh? Why would I be landing this? If it is ready you can land it directly onto 4.x-staging I think, but maybe cc @thealphanerd

@indutny
Copy link
Member Author

indutny commented Sep 4, 2016

Oops, meant @thealphanerd . Sorry!

@MylesBorins
Copy link
Contributor

landed in 7e60292...09099ab

@MylesBorins MylesBorins closed this Sep 4, 2016
@indutny
Copy link
Member Author

indutny commented Sep 4, 2016

Hooray, thanks everyone!

@indutny indutny deleted the backport/gh-7427 branch September 4, 2016 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openssl Issues and PRs related to the OpenSSL dependency. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants