Skip to content
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

configure: Add dependent libraries after crypto #2684

Closed
wants to merge 1 commit into from

Conversation

cmeister2
Copy link
Contributor

@cmeister2 cmeister2 commented Jun 26, 2018

The linker is pretty dumb and processes things left to right, keeping a
tally of symbols it hasn't resolved yet. So, we need -ldl to appear
after -lcrypto otherwise the linker won't find the dl functions.


This appears to fix the fuzzer build, but would appreciate an extra set of eyes.

@cmeister2 cmeister2 requested a review from bagder Jun 26, 2018
AC_MSG_CHECKING([OpenSSL linking with -ldl and -lpthread])
LIBS="$CLEANLIBS -ldl -lpthread -lcrypto"
LIBS="$CLEANLIBS -lcrypto -ldl -lpthread"
Copy link
Contributor Author

@cmeister2 cmeister2 Jun 26, 2018

Choose a reason for hiding this comment

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

I'm not entirely sure why this suddenly uses CLEANLIBS, where the one above uses LIBS.

Copy link
Member

@bagder bagder Jun 26, 2018

Choose a reason for hiding this comment

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

I think maybe the previous one should use CLEANLIBS too, as LIBS already contains "-lcrypto" at that point and it adds it again...

Copy link
Contributor Author

@cmeister2 cmeister2 Jun 26, 2018

Choose a reason for hiding this comment

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

Yes. I did wonder about that. Worth trying do you think?

Copy link
Member

@bagder bagder Jun 26, 2018

Choose a reason for hiding this comment

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

It's not the end of the world, but sure, if you fixup that at the same time as you this fix, I approve!

Copy link
Contributor Author

@cmeister2 cmeister2 Jun 26, 2018

Choose a reason for hiding this comment

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

Done.

@bagder bagder added the build label Jun 26, 2018
bagder
bagder approved these changes Jun 26, 2018
The linker is pretty dumb and processes things left to right, keeping a
tally of symbols it hasn't resolved yet. So, we need -ldl to appear
after -lcrypto otherwise the linker won't find the dl functions.
@cmeister2 cmeister2 force-pushed the cmeister2/openssllinkerorder branch from eff0a7b to 1237081 Compare Jun 26, 2018
bagder
bagder approved these changes Jun 26, 2018
@cmeister2 cmeister2 deleted the cmeister2/openssllinkerorder branch Jun 27, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Sep 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants