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

Jump to bottom

Add -latomic only for architectures where needed #15640

Closed
wants to merge 2 commits into from
Closed

Add -latomic only for architectures where needed #15640

wants to merge 2 commits into from

Conversation

t8m
Copy link
Member

@t8m t8m commented Jun 7, 2021

Apparently adding -latomic everywhere does not work with older gcc toolchains. We add it only on architectures where it is needed. It still means that on these targets you will need a toolchain which is new enough to include the libatomic.

It would be better to somehow detect that linking with -latomic is needed however the autodetection of features is fairly hard to do correctly for all possible cases and I do not feel like we should attempt to do that.

Another alternative would be to simply document that if you see something like missing __atomic_fetch_or_8() when linking you should reconfigure with -latomic added to the config command line.

@t8m
Copy link
Member Author

t8m commented Jun 7, 2021

@slontis does this help with what you're seeing with the missing libatomic on x86?

@t8m t8m added approval: review pending This pull request needs review by a committer branch: master Merge to master branch labels Jun 7, 2021
@mattcaswell
Copy link
Member

Does this cover the android cases that we had of this?

@t8m
Copy link
Member Author

t8m commented Jun 7, 2021

Does this cover the android cases that we had of this?

Apparently with the current toolchain it is not needed for android:
#14083 (comment)

@slontis
Copy link
Member

slontis commented Jun 7, 2021 •
edited

@slontis does this help with what you're seeing with the missing libatomic on x86?

I installed the lib so it was no longer a problem.. Thanks for adding though.

@slontis
Copy link
Member

slontis commented Jun 7, 2021

Error is valid.

@paulidale
Copy link
Contributor

Configuring without threads is another way to not need atomic operations 😃

@t8m
Copy link
Member Author

t8m commented Jun 7, 2021

Error is valid.

Fixed.

@t8m
Copy link
Member Author

t8m commented Jun 11, 2021

Ping for review

@paulidale paulidale 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 Jun 11, 2021
@openssl-machine openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Jun 12, 2021
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@paulidale
Copy link
Contributor

Merged.

@paulidale paulidale closed this Jun 13, 2021
openssl-machine pushed a commit that referenced this pull request Jun 13, 2021
@t8m @paulidale
Add -latomic only for architectures where needed
cdf2986 
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #15640)
devnexen pushed a commit to devnexen/openssl that referenced this pull request Jul 7, 2021
@t8m @devnexen
Add -latomic only for architectures where needed
71b23a9 
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from openssl#15640)
@DirtYiCE DirtYiCE mentioned this pull request Sep 9, 2021
Closed
halstead pushed a commit to openembedded/openembedded-core that referenced this pull request Dec 7, 2021
@abrodkin @rpurdie
openssl: Use linux-latomic target for ARC
7cc555a 
Some atomic ops for 32-bit ARC processors are implemented in GCC's libatomic.
For example those dealing with 64-bit data (e.g. __atomic_load_8()) as well as
some others. That said it's required to add "-latomic" for successful linkage.

Otherwise error messages like this happen on OpenSSL building for ARC:
------------------------------->8------------------------------
| ...ld: libcrypto.a(libcrypto-lib-threads_pthread.o): in function `CRYPTO_atomic_or':
| .../openssl-3.0.0/crypto/threads_pthread.c:219: undefined reference to `__atomic_is_lock_free'
| ...ld: .../openssl-3.0.0/crypto/threads_pthread.c:219: undefined reference to `__atomic_is_lock_free'
| ...ld: .../openssl-3.0.0/crypto/threads_pthread.c:220: undefined reference to `__atomic_fetch_or_8'
------------------------------->8------------------------------

Fix that by using a special target, which does exactly what's needed.
See [1] and [2] for more details on the matter.

[1] openssl/openssl@cdf2986
[2] openssl/openssl#15640

Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
kraj pushed a commit to YoeDistro/poky-old that referenced this pull request Dec 7, 2021
@abrodkin @rpurdie
openssl: Use linux-latomic target for ARC
25a8631 
Some atomic ops for 32-bit ARC processors are implemented in GCC's libatomic.
For example those dealing with 64-bit data (e.g. __atomic_load_8()) as well as
some others. That said it's required to add "-latomic" for successful linkage.

Otherwise error messages like this happen on OpenSSL building for ARC:
------------------------------->8------------------------------
| ...ld: libcrypto.a(libcrypto-lib-threads_pthread.o): in function `CRYPTO_atomic_or':
| .../openssl-3.0.0/crypto/threads_pthread.c:219: undefined reference to `__atomic_is_lock_free'
| ...ld: .../openssl-3.0.0/crypto/threads_pthread.c:219: undefined reference to `__atomic_is_lock_free'
| ...ld: .../openssl-3.0.0/crypto/threads_pthread.c:220: undefined reference to `__atomic_fetch_or_8'
------------------------------->8------------------------------

Fix that by using a special target, which does exactly what's needed.
See [1] and [2] for more details on the matter.

[1] openssl/openssl@cdf2986
[2] openssl/openssl#15640

(From OE-Core rev: 7cc555a9fc15678437736ea864da2a2e2eba9cb0)

Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
kraj pushed a commit to YoeDistro/poky-old that referenced this pull request Dec 8, 2021
@abrodkin @rpurdie
openssl: Use linux-latomic target for ARC
d2f6773 
Some atomic ops for 32-bit ARC processors are implemented in GCC's libatomic.
For example those dealing with 64-bit data (e.g. __atomic_load_8()) as well as
some others. That said it's required to add "-latomic" for successful linkage.

Otherwise error messages like this happen on OpenSSL building for ARC:
------------------------------->8------------------------------
| ...ld: libcrypto.a(libcrypto-lib-threads_pthread.o): in function `CRYPTO_atomic_or':
| .../openssl-3.0.0/crypto/threads_pthread.c:219: undefined reference to `__atomic_is_lock_free'
| ...ld: .../openssl-3.0.0/crypto/threads_pthread.c:219: undefined reference to `__atomic_is_lock_free'
| ...ld: .../openssl-3.0.0/crypto/threads_pthread.c:220: undefined reference to `__atomic_fetch_or_8'
------------------------------->8------------------------------

Fix that by using a special target, which does exactly what's needed.
See [1] and [2] for more details on the matter.

[1] openssl/openssl@cdf2986
[2] openssl/openssl#15640

(From OE-Core rev: 7cc555a9fc15678437736ea864da2a2e2eba9cb0)

Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
halstead pushed a commit to openembedded/openembedded-core that referenced this pull request Dec 8, 2021
@abrodkin @rpurdie
openssl: Use linux-latomic target for ARC
f48227a 
Some atomic ops for 32-bit ARC processors are implemented in GCC's libatomic.
For example those dealing with 64-bit data (e.g. __atomic_load_8()) as well as
some others. That said it's required to add "-latomic" for successful linkage.

Otherwise error messages like this happen on OpenSSL building for ARC:
------------------------------->8------------------------------
| ...ld: libcrypto.a(libcrypto-lib-threads_pthread.o): in function `CRYPTO_atomic_or':
| .../openssl-3.0.0/crypto/threads_pthread.c:219: undefined reference to `__atomic_is_lock_free'
| ...ld: .../openssl-3.0.0/crypto/threads_pthread.c:219: undefined reference to `__atomic_is_lock_free'
| ...ld: .../openssl-3.0.0/crypto/threads_pthread.c:220: undefined reference to `__atomic_fetch_or_8'
------------------------------->8------------------------------

Fix that by using a special target, which does exactly what's needed.
See [1] and [2] for more details on the matter.

[1] openssl/openssl@cdf2986
[2] openssl/openssl#15640

Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
kraj pushed a commit to YoeDistro/poky-old that referenced this pull request Dec 8, 2021
@abrodkin @rpurdie
openssl: Use linux-latomic target for ARC
22eb7f2 
Some atomic ops for 32-bit ARC processors are implemented in GCC's libatomic.
For example those dealing with 64-bit data (e.g. __atomic_load_8()) as well as
some others. That said it's required to add "-latomic" for successful linkage.

Otherwise error messages like this happen on OpenSSL building for ARC:
------------------------------->8------------------------------
| ...ld: libcrypto.a(libcrypto-lib-threads_pthread.o): in function `CRYPTO_atomic_or':
| .../openssl-3.0.0/crypto/threads_pthread.c:219: undefined reference to `__atomic_is_lock_free'
| ...ld: .../openssl-3.0.0/crypto/threads_pthread.c:219: undefined reference to `__atomic_is_lock_free'
| ...ld: .../openssl-3.0.0/crypto/threads_pthread.c:220: undefined reference to `__atomic_fetch_or_8'
------------------------------->8------------------------------

Fix that by using a special target, which does exactly what's needed.
See [1] and [2] for more details on the matter.

[1] openssl/openssl@cdf2986
[2] openssl/openssl#15640

(From OE-Core rev: f48227a192022c604f8c2ea4fe973c6664861101)

Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
MarcelRaad added a commit to MarcelRaad/openssl that referenced this pull request Mar 8, 2022
@MarcelRaad
android-x86 target: Add -latomic
0800848 
Fixes openssl#14083 again after being
broken by openssl#15640.

CLA: trivial
@MarcelRaad MarcelRaad mentioned this pull request Mar 8, 2022
Closed
openssl-machine pushed a commit that referenced this pull request Mar 9, 2022
@MarcelRaad @t8m
android-x86 target: Add -latomic
b420e24 
Fixes #14083 again after being
broken by #15640.

CLA: trivial

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #17833)
openssl-machine pushed a commit that referenced this pull request Mar 9, 2022
@MarcelRaad @t8m
android-x86 target: Add -latomic
28c1ac6 
Fixes #14083 again after being
broken by #15640.

CLA: trivial

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #17833)

(cherry picked from commit b420e24)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paulidale paulidale paulidale approved these changes

Assignees
No one assigned
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@t8m @mattcaswell @slontis @paulidale @openssl-machine