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

Check whether atomics can link rather than just compile. #9190

Closed
wants to merge 1 commit into from

Conversation

katzdm
Copy link
Contributor

@katzdm katzdm commented Jul 21, 2022

Some build toolchains support C11 atomics (i.e., _Atomic types), but will not link the associated atomics runtime unless a flag is passed. In such an environment, linking an application with libcurl.a can fail due to undefined symbols for atomic load/store functions.

I encountered this behavior when upgrading curl to 7.84.0 and attempting to build with Solaris Studio 12.6. Solaris provides the flag -xatomic=[gcc | studio], allowing users to link to one of two atomics runtime implementations. However, if the user does not provide this flag, then neither runtime is linked. This led to builds failing in CI, because:

  1. The test-program from m4/curl-functions.m4 compiled.
  2. The HAVE_ATOMICS macro was given a definition.
  3. Curl's thread-safe global initialization was enabled.
  4. Linker errors for undefined atomic load/store functions.

My suggested fix is to:

  1. Modify the program that tests for _Atomic support to assign to the variable after initializing it. For toolchains that implement atomic loads/stores by linking external libraries, this ensures that such a function will be required.
  2. Check that the program can not only be compiled, but also linked.

@bagder bagder added the build label Jul 21, 2022
bagder
bagder approved these changes Jul 21, 2022
@bagder
Copy link
Member

bagder commented Jul 21, 2022

Thanks!

@bagder bagder closed this in e7511f8 Jul 21, 2022
@katzdm katzdm deleted the check-link-for-atomics branch July 21, 2022 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants