dladdr() implementation for AIX for master (attempt 2)#5668
Closed
makr wants to merge 7 commits intoopenssl:masterfrom
Closed
dladdr() implementation for AIX for master (attempt 2)#5668makr wants to merge 7 commits intoopenssl:masterfrom
makr wants to merge 7 commits intoopenssl:masterfrom
Conversation
As AIX is missing a dladdr() implementation it is currently uncertain our exit()-handlers can still be called when the application exits. After dlclose() the whole library might have been unloaded already. The TODOs mark the place and the way for a stripped down dladdr()-implementation using AIX' own loadquery()-function. Signed-off-by: Matthias Kraft <[email protected]>
Implemented a stripped down (and so far untested) dladdr()-implementation using AIX' own loadquery()-function. Following the SGI example in the same code, the DL_info only has the dli_fname member. As the scope of dlfcn_pathbyaddr() is the filename, this implementation does not consider archive members, which can be dlopen()ed in AIX. It is also leaking memory as it provides a copy of ldinfo_filename. Signed-off-by: Matthias Kraft <[email protected]>
Fixed thinko in calculation of next ld_info entry. Changed to large fixed size ld_info buffer, as the ld_info entries are of dynamic size. The 80k will still hold 32 entries in a worst case scenario. The implementation can only lookup function symbols, no data symbols. Added PIC-flag to aix*-cc build targets. The marked printf's will be removed when the test case is implemented.
Added DATA segment checking to catch ptrgl virtual addresses. Avoid memleaks with every AIX/dladdr() call. Removed debug-fprintf()s. Added test case for DSO_dsobyaddr(), which will eventually call dladdr(). Removed unecessary AIX ifdefs again.
Fixed indentation, moved trailing operators to leading and removed unecessary braces. Tried to sqeeze everything in 80 cpl. Removed extra errno declaration. Changed a void* into char* for portable pointer arithmetics. Addressed a Travis issue complaining about another void*-cast. Reconfigured the test code to be empty on Windows, as the implementation there is completely different and would always produce an error. This is addressing the Appveyor issue.
Although it deviates from the actual prototype of DSO_dsobyaddr(), this is now ISO C compliant and gcc -Wpedantic accepts the code.
Removed architecture dependent comment, not really belonging here. More style guide conformance edits, partly indent suggested.
richsalz
approved these changes
Mar 19, 2018
levitte
approved these changes
Mar 19, 2018
Member
|
Feel free to merge, @richsalz |
2 tasks
Contributor
|
There is related question in #5626 (comment). |
Contributor
|
I did not get around to merging before the freeze, sadly. |
Member
This is a bug fix so it can go in when the repo reopens. |
Contributor
|
merged to master, thanks! |
levitte
pushed a commit
that referenced
this pull request
Mar 21, 2018
Although it deviates from the actual prototype of DSO_dsobyaddr(), this is now ISO C compliant and gcc -Wpedantic accepts the code. Added DATA segment checking to catch ptrgl virtual addresses. Avoid memleaks with every AIX/dladdr() call. Removed debug-fprintf()s. Added test case for DSO_dsobyaddr(), which will eventually call dladdr(). Removed unecessary AIX ifdefs again. The implementation can only lookup function symbols, no data symbols. Added PIC-flag to aix*-cc build targets. As AIX is missing a dladdr() implementation it is currently uncertain our exit()-handlers can still be called when the application exits. After dlclose() the whole library might have been unloaded already. Signed-off-by: Matthias Kraft <[email protected]> Reviewed-by: Richard Levitte <[email protected]> Reviewed-by: Rich Salz <[email protected]> (Merged from #5668)
makr
pushed a commit
to makr/openssl
that referenced
this pull request
Apr 12, 2018
The ongoing discussion about casting or not in PR openssl#5626 had me compiling again with above mentioned flags. Indeed the compiler had to say something about it and I did these changes to silence it again.
makr
pushed a commit
to makr/openssl
that referenced
this pull request
Apr 13, 2018
makr
pushed a commit
to makr/openssl
that referenced
this pull request
Apr 13, 2018
The ongoing discussion about casting or not in PR openssl#5626 had me compiling again with above mentioned flags. Indeed the compiler had to say something about it and I did these changes to silence it again.
makr
pushed a commit
to makr/openssl
that referenced
this pull request
Apr 13, 2018
levitte
pushed a commit
that referenced
this pull request
Apr 14, 2018
The ongoing discussion about casting or not in PR #5626 had me compiling again with above mentioned flags. Indeed the compiler had to say something about it and I did these changes to silence it again. Reviewed-by: Bernd Edlinger <[email protected]> Reviewed-by: Rich Salz <[email protected]> Reviewed-by: Richard Levitte <[email protected]> (Merged from #5943)
mattcaswell
reviewed
Nov 15, 2018
| } else if (strcmp(p, "-just_crypto") == 0) { | ||
| test_type = JUST_CRYPTO; | ||
| } else if (strcmp(p, "-dso_ref") == 0) { | ||
| test_type = JUST_CRYPTO; |
Member
There was a problem hiding this comment.
This doesn't look right. Should that be test_type = DSO_REFTEST;?
Member
There was a problem hiding this comment.
Yes it should. Will you make the PR? I've very little time today...
Member
There was a problem hiding this comment.
I've incorporated the fix in a larger PR I'm working on. If someone gets to it in the meantime then that's fine too.
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
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.
AIX is missing a dladdr() implementation, while OpenSSL is relying on it to retrieve a reference to its own shared object. This pull request now provides such an implementation. OpenSSL will now longer
crash in exit() on AIX after a dlopen()/dlclose().
This is following PR #5626 as implementation for master and replaces PR #5655.
Fixes #5485
Checklist