Skip to content

Comments

dladdr() implementation for AIX for master (attempt 2)#5668

Closed
makr wants to merge 7 commits intoopenssl:masterfrom
makr:openssl#5485-master2
Closed

dladdr() implementation for AIX for master (attempt 2)#5668
makr wants to merge 7 commits intoopenssl:masterfrom
makr:openssl#5485-master2

Conversation

@makr
Copy link

@makr makr commented Mar 19, 2018

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
  • tests are added or updated

Matthias Kraft and others added 7 commits March 19, 2018 13:42
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.
@makr makr changed the title Openssl#5485 master2 dladdr() implementation for AIX for master (attempt 2) Mar 19, 2018
@richsalz richsalz added branch: master Applies to master branch approval: review pending This pull request needs review by a committer labels Mar 19, 2018
@levitte levitte added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Mar 19, 2018
@levitte
Copy link
Member

levitte commented Mar 19, 2018

Feel free to merge, @richsalz

@dot-asm
Copy link
Contributor

dot-asm commented Mar 19, 2018

There is related question in #5626 (comment).

@richsalz
Copy link
Contributor

I did not get around to merging before the freeze, sadly.

@mattcaswell
Copy link
Member

I did not get around to merging before the freeze, sadly.

This is a bug fix so it can go in when the repo reopens.

@mattcaswell mattcaswell added this to the 1.1.1 milestone Mar 20, 2018
@richsalz
Copy link
Contributor

merged to master, thanks!

@richsalz richsalz closed this Mar 21, 2018
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)
} else if (strcmp(p, "-just_crypto") == 0) {
test_type = JUST_CRYPTO;
} else if (strcmp(p, "-dso_ref") == 0) {
test_type = JUST_CRYPTO;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look right. Should that be test_type = DSO_REFTEST;?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it should. Will you make the PR? I've very little time today...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: done This pull request has the required number of approvals branch: master Applies to master branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1.1.0g: test_shlibload fails on AIX

5 participants