Skip to content

Commit

Permalink
compiler-rt: fix build on ARMv6
Browse files Browse the repository at this point in the history
compiler-rt has accumulated several regressions that prevent it from building
on ARMv6. It is important to note that there are two major versions of ARMv6:
base ARMv6 and ARMv6K. ARMv6K includes several important new instructions,
such as non-word size atomic operations (ldrexd, strexd, etc.) and the yield
instruction. Most ARMv6 CPUs actually implement ARMv6K, including all those used
in Raspberry Pis, but nixpkgs' "raspberryPi" platform targets base ARMv6.

compiler-rt versions 8-14 fail to build on ARMv6 and ARMv6K. compiler-rt 15 (not
yet in nixpkgs) builds on ARMv6K but not ARMv6. This patch fixes versions 9-14
on both ARMv6 variants. The patches don't apply cleanly to version 8, and I
figured it wasn't worth carrying another version of the patches for such an old
version.

A total of five patches are required to get compiler-rt building on ARMv6:
* armv6-mcr-dmb.patch: use `mcr` to provide the equivalent of `dmb` on ARMv6.
  Included in LLVM 15.
* armv6-sync-ops-no-thumb.patch: prevent certain atomic operation functions
  from using Thumb mode. Included in LLVM 15.
* armv6-no-ldrexd-strexd.patch: don't use ldrexd or strexd, which are not
  available in base ARMv6. Submitted upstream by me.
* armv6-scudo-no-yield.patch: use nop instead of yield on ARMv6 in standalone
  scudo. Required by versions >=13, since they enable standalone scudo.
  Submitted upstream by me.
* armv6-scudo-libatomic.patch: link standlone scudo to libatomic on ARMv6 (and
  any other platforms that need it). Not yet submitted because the backport is
  a bit different from the upstream version and I need to test it.
  • Loading branch information
lopsided98 committed Mar 8, 2023
1 parent 6e10265 commit 98ebcd2
Show file tree
Hide file tree
Showing 12 changed files with 416 additions and 1 deletion.
4 changes: 4 additions & 0 deletions pkgs/development/compilers/llvm/10/compiler-rt/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ stdenv.mkDerivation {
./gnu-install-dirs.patch
../../common/compiler-rt/libsanitizer-no-cyclades-11.patch
./X86-support-extension.patch # backported from LLVM 11
# Fix build on armv6l
../../common/compiler-rt/armv6-mcr-dmb.patch
../../common/compiler-rt/armv6-sync-ops-no-thumb.patch
../../common/compiler-rt/armv6-no-ldrexd-strexd.patch
] ++ lib.optional stdenv.hostPlatform.isAarch32 ./armv7l.patch;

# TSAN requires XPC on Darwin, which we have no public/free source files for. We can depend on the Apple frameworks
Expand Down
4 changes: 4 additions & 0 deletions pkgs/development/compilers/llvm/11/compiler-rt/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ stdenv.mkDerivation {
../../common/compiler-rt/libsanitizer-no-cyclades-11.patch
../../common/compiler-rt/darwin-plistbuddy-workaround.patch
./armv7l.patch
# Fix build on armv6l
../../common/compiler-rt/armv6-mcr-dmb.patch
../../common/compiler-rt/armv6-sync-ops-no-thumb.patch
../../common/compiler-rt/armv6-no-ldrexd-strexd.patch
];

preConfigure = lib.optionalString stdenv.hostPlatform.isDarwin ''
Expand Down
4 changes: 4 additions & 0 deletions pkgs/development/compilers/llvm/12/compiler-rt/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ stdenv.mkDerivation {
./normalize-var.patch
../../common/compiler-rt/darwin-plistbuddy-workaround.patch
./armv7l.patch
# Fix build on armv6l
../../common/compiler-rt/armv6-mcr-dmb.patch
../../common/compiler-rt/armv6-sync-ops-no-thumb.patch
../../common/compiler-rt/armv6-no-ldrexd-strexd.patch
];

# TSAN requires XPC on Darwin, which we have no public/free source files for. We can depend on the Apple frameworks
Expand Down
6 changes: 6 additions & 0 deletions pkgs/development/compilers/llvm/13/compiler-rt/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ stdenv.mkDerivation {
./darwin-targetconditionals.patch
../../common/compiler-rt/darwin-plistbuddy-workaround.patch
./armv7l.patch
# Fix build on armv6l
../../common/compiler-rt/armv6-mcr-dmb.patch
../../common/compiler-rt/armv6-sync-ops-no-thumb.patch
../../common/compiler-rt/armv6-no-ldrexd-strexd.patch
../../common/compiler-rt/armv6-scudo-no-yield.patch
../../common/compiler-rt/armv6-scudo-libatomic.patch
];

# TSAN requires XPC on Darwin, which we have no public/free source files for. We can depend on the Apple frameworks
Expand Down
6 changes: 6 additions & 0 deletions pkgs/development/compilers/llvm/14/compiler-rt/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,12 @@ stdenv.mkDerivation {
./darwin-targetconditionals.patch
../../common/compiler-rt/darwin-plistbuddy-workaround.patch
./armv7l.patch
# Fix build on armv6l
../../common/compiler-rt/armv6-mcr-dmb.patch
../../common/compiler-rt/armv6-sync-ops-no-thumb.patch
../../common/compiler-rt/armv6-no-ldrexd-strexd.patch
../../common/compiler-rt/armv6-scudo-no-yield.patch
../../common/compiler-rt/armv6-scudo-libatomic.patch
];

# TSAN requires XPC on Darwin, which we have no public/free source files for. We can depend on the Apple frameworks
Expand Down
4 changes: 4 additions & 0 deletions pkgs/development/compilers/llvm/9/compiler-rt/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ stdenv.mkDerivation {
./codesign.patch # Revert compiler-rt commit that makes codesign mandatory
./gnu-install-dirs.patch
../../common/compiler-rt/libsanitizer-no-cyclades-9.patch
# Fix build on armv6l
../../common/compiler-rt/armv6-mcr-dmb.patch
../../common/compiler-rt/armv6-sync-ops-no-thumb.patch
../../common/compiler-rt/armv6-no-ldrexd-strexd.patch
] ++ lib.optional stdenv.hostPlatform.isAarch32 ./armv7l.patch;

# TSAN requires XPC on Darwin, which we have no public/free source files for. We can depend on the Apple frameworks
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
From a11d1cc41c725ec6dee58f75e4a852a658dd7543 Mon Sep 17 00:00:00 2001
From: Khem Raj <raj.khem@gmail.com>
Date: Thu, 10 Mar 2022 19:30:00 -0800
Subject: [PATCH] [builtins] Use mcr for dmb instruction on armv6

At present compiler-rt cross compiles for armv6 ( -march=armv6 ) but includes
dmb instructions which are only available in armv7+ this causes SIGILL on
clang+compiler-rt compiled components on rpi0w platforms.

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D99282
---
compiler-rt/lib/builtins/arm/sync-ops.h | 8 ++++----
compiler-rt/lib/builtins/assembly.h | 8 ++++++++
2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/lib/builtins/arm/sync-ops.h b/lib/builtins/arm/sync-ops.h
index c9623249e5d20..7a26170741ad2 100644
--- a/lib/builtins/arm/sync-ops.h
+++ b/lib/builtins/arm/sync-ops.h
@@ -19,14 +19,14 @@
.thumb; \
.syntax unified; \
DEFINE_COMPILERRT_THUMB_FUNCTION(__sync_fetch_and_##op) \
- dmb; \
+ DMB; \
mov r12, r0; \
LOCAL_LABEL(tryatomic_##op) : ldrex r0, [r12]; \
op(r2, r0, r1); \
strex r3, r2, [r12]; \
cmp r3, #0; \
bne LOCAL_LABEL(tryatomic_##op); \
- dmb; \
+ DMB; \
bx lr

#define SYNC_OP_8(op) \
@@ -35,14 +35,14 @@
.syntax unified; \
DEFINE_COMPILERRT_THUMB_FUNCTION(__sync_fetch_and_##op) \
push {r4, r5, r6, lr}; \
- dmb; \
+ DMB; \
mov r12, r0; \
LOCAL_LABEL(tryatomic_##op) : ldrexd r0, r1, [r12]; \
op(r4, r5, r0, r1, r2, r3); \
strexd r6, r4, r5, [r12]; \
cmp r6, #0; \
bne LOCAL_LABEL(tryatomic_##op); \
- dmb; \
+ DMB; \
pop { r4, r5, r6, pc }

#define MINMAX_4(rD, rN, rM, cmp_kind) \
diff --git a/lib/builtins/assembly.h b/lib/builtins/assembly.h
index 69a3d8620f924..06aa18162e3b4 100644
--- a/lib/builtins/assembly.h
+++ b/lib/builtins/assembly.h
@@ -189,6 +189,14 @@
JMP(ip)
#endif

+#if __ARM_ARCH >= 7
+#define DMB dmb
+#elif __ARM_ARCH >= 6
+#define DMB mcr p15, #0, r0, c7, c10, #5
+#else
+#error only supported on ARMv6+
+#endif
+
#if defined(USE_THUMB_2)
#define WIDE(op) op.w
#else

Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
From 4fe3f21bf8b20c766877d2251d61118d0ff36688 Mon Sep 17 00:00:00 2001
From: Ben Wolsieffer <benwolsieffer@gmail.com>
Date: Wed, 7 Dec 2022 14:56:51 -0500
Subject: [PATCH] [compiler-rt][builtins] Do not use ldrexd or strexd on ARMv6

The ldrexd and strexd instructions are not available on base ARMv6, and were
only added in ARMv6K (see [1]). This patch solves this problem once and for all
using the __ARM_FEATURE_LDREX macro (see [2]) defined in the ARM C Language
Extensions (ACLE). Although this macro is technically deprecated in the ACLE,
it allows compiler-rt to reliably detect whether ldrexd and strexd are supported
without complicated conditionals to detect different ARM architecture variants.

[1] https://developer.arm.com/documentation/dht0008/a/ch01s02s01
[2] https://arm-software.github.io/acle/main/acle.html#ldrexstrex

Differential Revision: https://reviews.llvm.org/D139585
---
compiler-rt/lib/builtins/arm/sync_fetch_and_add_8.S | 2 +-
compiler-rt/lib/builtins/arm/sync_fetch_and_and_8.S | 2 +-
compiler-rt/lib/builtins/arm/sync_fetch_and_max_8.S | 2 +-
compiler-rt/lib/builtins/arm/sync_fetch_and_min_8.S | 2 +-
compiler-rt/lib/builtins/arm/sync_fetch_and_nand_8.S | 2 +-
compiler-rt/lib/builtins/arm/sync_fetch_and_or_8.S | 2 +-
compiler-rt/lib/builtins/arm/sync_fetch_and_sub_8.S | 2 +-
compiler-rt/lib/builtins/arm/sync_fetch_and_umax_8.S | 2 +-
compiler-rt/lib/builtins/arm/sync_fetch_and_umin_8.S | 2 +-
compiler-rt/lib/builtins/arm/sync_fetch_and_xor_8.S | 2 +-
10 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/lib/builtins/arm/sync_fetch_and_add_8.S b/lib/builtins/arm/sync_fetch_and_add_8.S
index 18bdd875b8b7..bee6f7ba0f34 100644
--- a/lib/builtins/arm/sync_fetch_and_add_8.S
+++ b/lib/builtins/arm/sync_fetch_and_add_8.S
@@ -13,7 +13,7 @@

#include "sync-ops.h"

-#if __ARM_ARCH_PROFILE != 'M'
+#if __ARM_FEATURE_LDREX & 8
#define add_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI) \
adds rD_LO, rN_LO, rM_LO ; \
adc rD_HI, rN_HI, rM_HI
diff --git a/lib/builtins/arm/sync_fetch_and_and_8.S b/lib/builtins/arm/sync_fetch_and_and_8.S
index 3716eff809d5..b4e77a54edf6 100644
--- a/lib/builtins/arm/sync_fetch_and_and_8.S
+++ b/lib/builtins/arm/sync_fetch_and_and_8.S
@@ -13,7 +13,7 @@

#include "sync-ops.h"

-#if __ARM_ARCH_PROFILE != 'M'
+#if __ARM_FEATURE_LDREX & 8
#define and_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI) \
and rD_LO, rN_LO, rM_LO ; \
and rD_HI, rN_HI, rM_HI
diff --git a/lib/builtins/arm/sync_fetch_and_max_8.S b/lib/builtins/arm/sync_fetch_and_max_8.S
index 06115ab55246..1813274cc649 100644
--- a/lib/builtins/arm/sync_fetch_and_max_8.S
+++ b/lib/builtins/arm/sync_fetch_and_max_8.S
@@ -13,7 +13,7 @@

#include "sync-ops.h"

-#if __ARM_ARCH_PROFILE != 'M'
+#if __ARM_FEATURE_LDREX & 8
#define max_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI) MINMAX_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI, gt)

SYNC_OP_8(max_8)
diff --git a/lib/builtins/arm/sync_fetch_and_min_8.S b/lib/builtins/arm/sync_fetch_and_min_8.S
index 4f3e299d95cc..fa8f3477757b 100644
--- a/lib/builtins/arm/sync_fetch_and_min_8.S
+++ b/lib/builtins/arm/sync_fetch_and_min_8.S
@@ -13,7 +13,7 @@

#include "sync-ops.h"

-#if __ARM_ARCH_PROFILE != 'M'
+#if __ARM_FEATURE_LDREX & 8
#define min_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI) MINMAX_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI, lt)

SYNC_OP_8(min_8)
diff --git a/lib/builtins/arm/sync_fetch_and_nand_8.S b/lib/builtins/arm/sync_fetch_and_nand_8.S
index 425c94474af7..fb27219ee200 100644
--- a/lib/builtins/arm/sync_fetch_and_nand_8.S
+++ b/lib/builtins/arm/sync_fetch_and_nand_8.S
@@ -13,7 +13,7 @@

#include "sync-ops.h"

-#if __ARM_ARCH_PROFILE != 'M'
+#if __ARM_FEATURE_LDREX & 8
#define nand_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI) \
bic rD_LO, rN_LO, rM_LO ; \
bic rD_HI, rN_HI, rM_HI
diff --git a/lib/builtins/arm/sync_fetch_and_or_8.S b/lib/builtins/arm/sync_fetch_and_or_8.S
index 4f18dcf84df9..3b077c8737b1 100644
--- a/lib/builtins/arm/sync_fetch_and_or_8.S
+++ b/lib/builtins/arm/sync_fetch_and_or_8.S
@@ -13,7 +13,7 @@

#include "sync-ops.h"

-#if __ARM_ARCH_PROFILE != 'M'
+#if __ARM_FEATURE_LDREX & 8
#define or_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI) \
orr rD_LO, rN_LO, rM_LO ; \
orr rD_HI, rN_HI, rM_HI
diff --git a/lib/builtins/arm/sync_fetch_and_sub_8.S b/lib/builtins/arm/sync_fetch_and_sub_8.S
index 25a4a1076555..c171607eabd8 100644
--- a/lib/builtins/arm/sync_fetch_and_sub_8.S
+++ b/lib/builtins/arm/sync_fetch_and_sub_8.S
@@ -13,7 +13,7 @@

#include "sync-ops.h"

-#if __ARM_ARCH_PROFILE != 'M'
+#if __ARM_FEATURE_LDREX & 8
#define sub_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI) \
subs rD_LO, rN_LO, rM_LO ; \
sbc rD_HI, rN_HI, rM_HI
diff --git a/lib/builtins/arm/sync_fetch_and_umax_8.S b/lib/builtins/arm/sync_fetch_and_umax_8.S
index aa5213ff1def..d1224f758049 100644
--- a/lib/builtins/arm/sync_fetch_and_umax_8.S
+++ b/lib/builtins/arm/sync_fetch_and_umax_8.S
@@ -13,7 +13,7 @@

#include "sync-ops.h"

-#if __ARM_ARCH_PROFILE != 'M'
+#if __ARM_FEATURE_LDREX & 8
#define umax_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI) MINMAX_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI, hi)

SYNC_OP_8(umax_8)
diff --git a/lib/builtins/arm/sync_fetch_and_umin_8.S b/lib/builtins/arm/sync_fetch_and_umin_8.S
index 8b40541ab47d..595444e6d053 100644
--- a/lib/builtins/arm/sync_fetch_and_umin_8.S
+++ b/lib/builtins/arm/sync_fetch_and_umin_8.S
@@ -13,7 +13,7 @@

#include "sync-ops.h"

-#if __ARM_ARCH_PROFILE != 'M'
+#if __ARM_FEATURE_LDREX & 8
#define umin_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI) MINMAX_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI, lo)

SYNC_OP_8(umin_8)
diff --git a/lib/builtins/arm/sync_fetch_and_xor_8.S b/lib/builtins/arm/sync_fetch_and_xor_8.S
index 7436eb1d4cae..9fc3d85cef75 100644
--- a/lib/builtins/arm/sync_fetch_and_xor_8.S
+++ b/lib/builtins/arm/sync_fetch_and_xor_8.S
@@ -13,7 +13,7 @@

#include "sync-ops.h"

-#if __ARM_ARCH_PROFILE != 'M'
+#if __ARM_FEATURE_LDREX & 8
#define xor_8(rD_LO, rD_HI, rN_LO, rN_HI, rM_LO, rM_HI) \
eor rD_LO, rN_LO, rM_LO ; \
eor rD_HI, rN_HI, rM_HI
--
2.38.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
From a56bb19a9dc303a50ef12d83cd24c2395bf81076 Mon Sep 17 00:00:00 2001
From: Ben Wolsieffer <benwolsieffer@gmail.com>
Date: Wed, 7 Dec 2022 21:25:46 -0500
Subject: [PATCH] [scudo][standalone] Use CheckAtomic to decide to link to
libatomic

Standalone scudo uses the atomic operation builtin functions, which require
linking to libatomic on some platforms. Currently, this is done in an ad-hoc
manner. MIPS platforms always link to libatomic, and the tests are always linked
to it as well. libatomic is required on base ARMv6 (but not ARMv6K), but it is
currently not linked, causing the build to fail.

This patch replaces this ad-hoc logic with the CheckAtomic CMake module already
used in other parts of LLVM. The CheckAtomic module checks whether std::atomic
requires libatomic, which is not strictly the same as checking the atomic
builtins, but should have the same results as far as I know. If this is
problematic, a custom version of CheckAtomic could be used to specifically test
the builtins.
---
compiler-rt/lib/scudo/standalone/CMakeLists.txt | 7 +++++++
compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt | 4 +---
2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/lib/scudo/standalone/CMakeLists.txt b/lib/scudo/standalone/CMakeLists.txt
index ae5c354768c8..eb27374ca520 100644
--- a/lib/scudo/standalone/CMakeLists.txt
+++ b/lib/scudo/standalone/CMakeLists.txt
@@ -1,5 +1,8 @@
add_compiler_rt_component(scudo_standalone)

+include(DetermineGCCCompatible)
+include(CheckAtomic)
+
include_directories(../.. include)

set(SCUDO_CFLAGS)
@@ -34,6 +37,10 @@ list(APPEND SCUDO_LINK_FLAGS -Wl,-z,defs,-z,now,-z,relro)

list(APPEND SCUDO_LINK_FLAGS -ffunction-sections -fdata-sections -Wl,--gc-sections)

+if(HAVE_CXX_ATOMICS_WITH_LIB OR HAVE_CXX_ATOMICS64_WITH_LIB)
+ list(APPEND SCUDO_LINK_FLAGS -latomic)
+endif()
+
# We don't use the C++ standard library, so avoid including it by mistake.
append_list_if(COMPILER_RT_HAS_NOSTDLIBXX_FLAG -nostdlib++ SCUDO_LINK_FLAGS)

diff --git a/lib/scudo/standalone/tests/CMakeLists.txt b/lib/scudo/standalone/tests/CMakeLists.txt
index 6d0936cbb5c1..70a5a7e959c1 100644
--- a/lib/scudo/standalone/tests/CMakeLists.txt
+++ b/lib/scudo/standalone/tests/CMakeLists.txt
@@ -38,9 +38,7 @@ set(LINK_FLAGS
${SANITIZER_TEST_CXX_LIBRARIES}
)
list(APPEND LINK_FLAGS -pthread)
-# Linking against libatomic is required with some compilers
-check_library_exists(atomic __atomic_load_8 "" COMPILER_RT_HAS_LIBATOMIC)
-if (COMPILER_RT_HAS_LIBATOMIC)
+if(HAVE_CXX_ATOMICS_WITH_LIB OR HAVE_CXX_ATOMICS64_WITH_LIB)
list(APPEND LINK_FLAGS -latomic)
endif()

--
2.38.1

Loading

0 comments on commit 98ebcd2

Please sign in to comment.