Fixes issues with dynamic loading OpenSSL. Fixes #13903. #13919
Merged
Conversation
This file contains 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
dom96
force-pushed
the
fixes-openssl-android
branch
2 times, most recently
from
1dd31de to
498ebdd
Compare
This fixes at least a couple of issues: * Procs loaded from the DLL being used even when the pointer is nil. * The actual issue (#13903) which appeared to cause stack corruption on Android 7.1.1 with OpenSSL 1.1.1f. The change that fixed this was the move to loading the procs in `sslSym`.
dom96
force-pushed
the
fixes-openssl-android
branch
from
498ebdd to
ed8dfdc
Compare
1 task
should be |
|
likelihood of clashes is really small. |
narimiran
pushed a commit
that referenced
this pull request
Apr 14, 2020
…ckport] This fixes at least a couple of issues: * Procs loaded from the DLL being used even when the pointer is nil. * The actual issue (#13903) which appeared to cause stack corruption on Android 7.1.1 with OpenSSL 1.1.1f. The change that fixed this was the move to loading the procs in `sslSym`. (cherry picked from commit 350ee03)
narimiran
pushed a commit
that referenced
this pull request
Apr 14, 2020
…ckport] This fixes at least a couple of issues: * Procs loaded from the DLL being used even when the pointer is nil. * The actual issue (#13903) which appeared to cause stack corruption on Android 7.1.1 with OpenSSL 1.1.1f. The change that fixed this was the move to loading the procs in `sslSym`. (cherry picked from commit 350ee03)
|
it's not just about clashes, it's also about namespacing. So that you can tell from a symbol which pkg it comes from (nim vs other nimble package) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This fixes at least a couple of issues:
Android 7.1.1 with OpenSSL 1.1.1f. The change that fixed this was the
move to loading the procs in
sslSym.I also added a new define (
-d:noOpenSSLHacks) to force the use of Nim'sdynlibmechanism (and not the rather hacky dynamic approach taken by default).Also it's important to keep in mind that I tested this with 1.0.6 and then forward ported my patch, here is my openssl.nim on 1.0.6 to ease backporting: https://gist.github.com/dom96/bb6a76e0738f18f35a95ba8d6a4ec86b
As a side note, I wanted to add some unit tests for this but apparently wrapper module files are not tested in CI :(