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

Append pthread to platform libs if cmake supplied Threads is not good enough #897

Merged
merged 1 commit into from
Mar 31, 2022

Conversation

SergeyRyabinin
Copy link
Contributor

Issue #, if available:
original PR created from a fork branch: 895
aws-c-common has an explicit dependency on pthread's pthread_mutexattr_settype here in mutex.c .
However, aws-c-common cmake script does not instruct cmake to explicitly link with pthread.

Appending fsanitize=address to CMAKE_CXX_FLAGS results in GLIBC library to provide some pthread API but not all. Therefore, find_package(Threads REQUIRED) starts to "find" fake pthread APIs in GLIBC and stops to set CMAKE_THREAD_LIBS_INIT (which is typically expected to contain Threads lib implementation (which is typically pthread)). In addition, if user sets OpenSSL as a crypto lib implementation, aws-lc is not being built too (aws-lc has a hard set lining dependency on pthread, aws-lc hides the issue being described/fixed in this pull request).

The end result is that aws-c-common fails to link (in case of fsanitize= and OpenSSL crypto) because pthread_mutexattr_settype symbol is not found.

Description of changes:
Link explicitly with pthread unless cmake found Threads library provides necessary symbol (i.e. still provide user with a theoretical ability to use custom Threads lib).

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@SergeyRyabinin SergeyRyabinin marked this pull request as ready for review March 31, 2022 21:03
@SergeyRyabinin SergeyRyabinin merged commit 68f28f8 into main Mar 31, 2022
@SergeyRyabinin SergeyRyabinin deleted the srPthread branch March 31, 2022 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants