Page MenuHomePhabricator

[libc++] Link against libatomic when it is found

Authored by ldionne on Jun 4 2020, 12:38 PM.


Group Reviewers
Restricted Project
rGe0184357fc78: [libc++] Link against libatomic when it is found

Before this patch, we tried detecting whether small atomics were available
without linking against libatomic. However, that's not really what we want
to know -- instead, we want to know what's required in order to support
atomics fully, which is to link against libatomic when it's provided.

That is both much simpler, and it doesn't suffer the problem that we would
not link against libatomic when small atomics didn't require it, which
lead to non-lockfree atomics never working.

Furthermore, because we understand that some platforms might not want to
(or be able to) ship non-lockfree atomics, we add that notion to the test
suite, independently of a potential extern library.

After this patch, we therefore:
(1) Link against libatomic when it is provided
(2) Independently detect whether non-lockfree atomics are supported in

the test suite, regardless of whether that means we're linking against
an external library or not (which is an implementation detail).

Diff Detail

Event Timeline

ldionne created this revision.Jun 4 2020, 12:38 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 4 2020, 12:38 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne marked an inline comment as done.
ldionne added a subscriber: phosek.
ldionne added inline comments.

@phosek Is this right for Fuchsia?

I'm not an expert in the details but this looks right to me.

When this goes in, I will be adding non-lock-free atomic tests.

These tests would fail today, meaning we have no coverage today.

griwes added a subscriber: griwes.Jun 4 2020, 2:31 PM

FWIW this looks good to me as well.

jfb added inline comments.Jun 4 2020, 3:06 PM

Here and for the others, int main(int, char**) 😄


I think your filenames need to be quoted properly otherwise spaces in filenames will bork stuff (above and below).


int main(int, char**)

ldionne updated this revision to Diff 268770.Jun 5 2020, 6:27 AM
ldionne marked 4 inline comments as done.

Address JF's comments


Lit will expand filenames like %t and directories like %T, the assumption is that it will quote it properly for shell evaluation.

ldionne accepted this revision as: Restricted Project.Jun 5 2020, 6:28 AM
This revision was not accepted when it landed; it landed in state Needs Review.Jun 5 2020, 6:37 AM
This revision was automatically updated to reflect the committed changes.
jfb added a comment.Jun 5 2020, 8:27 AM

Thanks for this!

On which platform does this commit work ?

On Linux, the test program would still need an explicit -latomic, since indirect dependencies are not used to resolve undefined symbols from the program.
For this to work, it would need PUBLIC atomic instead of PRIVATE atomic, so that libcxx/cmake/Modules/DefineLinkerScript.cmake would add it to the linker script (
It would be even better with AS_NEEDED in the linker script, and -Wl,--as-needed for the lib, in order to only load the library when it is actually used.