Add atomic and bswap functions to libcompiler_rt
rS364753
Actions

Description

Add atomic and bswap functions to libcompiler_rt

There have been several mentions on our mailing lists about missing
atomic functions in our system libraries (e.g. atomic_load_8 and
friends), and recently I saw bswapdi2 and __bswapsi2 mentioned too.

To address this, add implementations for the functions from compiler-rt
to the system compiler support libraries, e.g. libcompiler_rt.a and and
libgcc_s.so.

This also needs a small fixup in compiler-rt's atomic.c, to ensure that
32-bit mips can build correctly.

Bump __FreeBSD_version to make it easier for port maintainers to detect
when these functions were added.

MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D26159

Details

Provenance

dim	Authored on

Differential Revision

D26159: Add atomic and bswap functions to libcompiler_rt

Parents

rS364752: Added a syzkaller reproducer.

Branches

Unknown

Tags

Unknown

Event Timeline

dim committed rS364753: Add atomic and bswap functions to libcompiler_rt.Aug 25 2020, 6:49 AM

dim added an edge: D26159: Add atomic and bswap functions to libcompiler_rt.

arichardson added a subscriber: arichardson.Aug 25 2020, 6:53 AM

arichardson added inline comments.

/head/lib/libcompiler_rt/Makefile.inc
129	This should not be needed, since clang will emit a libcall if this warning is emitted. And emitting the libcall inside the libcall implementation is not ideal.

dim added inline comments.Aug 25 2020, 8:02 AM

/head/lib/libcompiler_rt/Makefile.inc

129

Unfortunately it *is* needed, otherwise the build dies in both lib/libcompiler_rt and lib/libgcc_s with:

contrib/llvm-project/compiler-rt/lib/builtins/atomic.c:179:3: error: large atomic operation may incur significant performance penalty [-Werror,-Watomic-alignment]
  LOCK_FREE_CASES();
  ^
contrib/llvm-project/compiler-rt/lib/builtins/atomic.c:161:9: note: expanded from macro 'LOCK_FREE_CASES'
        LOCK_FREE_ACTION(uint64_t);                                            \
        ^
contrib/llvm-project/compiler-rt/lib/builtins/atomic.c:177:21: note: expanded from macro 'LOCK_FREE_ACTION'
  *((type *)dest) = __c11_atomic_load((_Atomic(type) *)src, model);            \
                    ^

We obviously expect that this may incur a penalty, so the warning is superfluous here, and must be suppressed.

arichardson added inline comments.Aug 25 2020, 8:04 AM

/head/lib/libcompiler_rt/Makefile.inc
129	However, this means that the condition was not constant-folded to 1/0 and therefore __atomic_is_lock_free() is being called. And that function is not implemented anywhere so we get linker errors. I've posted an upstream review to fix this: https://reviews.llvm.org/D86510

dim added inline comments.Aug 25 2020, 10:15 AM

/head/lib/libcompiler_rt/Makefile.inc
129	Hmm you may be right, I think I got this warning before adding an additional `#if defined(__mips__)` in front of the `#define IS_LOCK_FREE_8 0` line in `contrib/llvm-project/compiler-rt/lib/builtins/atomic.c`! And indeed I then got a link error because `___atomic_is_lock_free` was being called there, and not available. Now that that is solved, I'm doing another mips buildworld run with the -Wno- flag removed, to see if the warning is gone now, as we'd expect.