https://github.com/statham-arm created
https://github.com/llvm/llvm-project/pull/150419
Depending on the particular version of the AArch32 architecture, load/store
exclusive operations might be available for various subset of 8, 16, 32, and
64-bit quantities. Sema knew nothing about this and was accepting all four
sizes, leading to a compiler crash at isel time if you used a size not
available on the target architecture.
Now the Sema checking stage emits a more sensible diagnostic, pointing at the
location in the code.
In order to allow Sema to query the set of supported sizes, I've moved the enum
of LDREX_x sizes out of its Arm-specific header into `TargetInfo.h`.
Also, in order to allow the diagnostic to specify the correct list of supported
sizes, I've filled it with `%select{}`. (The alternative was to make separate
error messages for each different list of sizes.)
>From fa0ce89d172979533bb8a54922d53fab6c0bfea0 Mon Sep 17 00:00:00 2001
From: Simon Tatham <simon.tat...@arm.com>
Date: Thu, 24 Jul 2025 13:38:43 +0100
Subject: [PATCH] [Clang][ARM][Sema] Reject bad sizes of __builtin_arm_ldrex
Depending on the particular version of the AArch32 architecture,
load/store exclusive operations might be available for various subset
of 8, 16, 32, and 64-bit quantities. Sema knew nothing about this and
was accepting all four sizes, leading to a compiler crash at isel time
if you used a size not available on the target architecture.
Now the Sema checking stage emits a more sensible diagnostic, pointing
at the location in the code.
In order to allow Sema to query the set of supported sizes, I've moved
the enum of LDREX_x sizes out of its Arm-specific header into
`TargetInfo.h`.
Also, in order to allow the diagnostic to specify the correct list of
supported sizes, I've filled it with `%select{}`. (The alternative was
to make separate error messages for each different list of sizes.)
---
.../clang/Basic/DiagnosticSemaKinds.td | 24 ++++++-
clang/include/clang/Basic/TargetInfo.h | 11 ++++
clang/include/clang/Sema/SemaARM.h | 4 +-
clang/lib/Basic/Targets/ARM.cpp | 10 +--
clang/lib/Basic/Targets/ARM.h | 9 +--
clang/lib/Sema/SemaARM.cpp | 66 +++++++++++++++----
clang/test/Sema/builtins-arm-exclusive-124.c | 26 ++++++++
clang/test/Sema/builtins-arm-exclusive-4.c | 22 +++++++
clang/test/Sema/builtins-arm-exclusive-none.c | 22 +++++++
clang/test/Sema/builtins-arm-exclusive.c | 8 +++
10 files changed, 175 insertions(+), 27 deletions(-)
create mode 100644 clang/test/Sema/builtins-arm-exclusive-124.c
create mode 100644 clang/test/Sema/builtins-arm-exclusive-4.c
create mode 100644 clang/test/Sema/builtins-arm-exclusive-none.c
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index b2ea65ae111be..d8b1e312c1228 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9329,8 +9329,28 @@ def err_atomic_builtin_pointer_size : Error<
"address argument to atomic builtin must be a pointer to 1,2,4,8 or 16 byte "
"type (%0 invalid)">;
def err_atomic_exclusive_builtin_pointer_size : Error<
- "address argument to load or store exclusive builtin must be a pointer to"
- " 1,2,4 or 8 byte type (%0 invalid)">;
+ "address argument to load or store exclusive builtin must be a pointer to "
+ // Because the range of legal sizes for load/store exclusive varies with the
+ // Arm architecture version, this error message wants to be able to specify
+ // various different subsets of the sizes 1, 2, 4, 8. Rather than make a
+ // separate diagnostic for each subset, I've arranged here that _this_ error
+ // can display any combination of the sizes. For each size there are two
+ // %select parameters: the first chooses whether you need a "," or " or " to
+ // separate the number from a previous one (or neither), and the second
+ // parameter indicates whether to display the number itself.
+ //
+ // (The very first of these parameters isn't really necessary, since you
+ // never want to start with "," or " or " before the first number in the
+ // list, but it keeps it simple to make it look exactly like the other cases,
+ // and also allows a loop constructing this diagnostic to handle every case
+ // exactly the same.)
+ "%select{|,| or }1%select{|1}2"
+ "%select{|,| or }3%select{|2}4"
+ "%select{|,| or }5%select{|4}6"
+ "%select{|,| or }7%select{|8}8"
+ " byte type (%0 invalid)">;
+def err_atomic_exclusive_builtin_pointer_size_none : Error<
+ "load and store exclusive builtins are not available on this architecture">;
def err_atomic_builtin_ext_int_size : Error<
"atomic memory operand must have a power-of-two size">;
def err_atomic_builtin_bit_int_prohibit : Error<
diff --git a/clang/include/clang/Basic/TargetInfo.h
b/clang/include/clang/Basic/TargetInfo.h
index 8b66c73474704..7e84c6a9ce0b5 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -1071,6 +1071,17 @@ class TargetInfo : public TransferrableTargetInfo,
/// as Custom Datapath.
uint32_t getARMCDECoprocMask() const { return ARMCDECoprocMask; }
+ /// For ARM targets returns a mask defining which data sizes are suitable for
+ /// __builtin_arm_ldrex and __builtin_arm_strex.
+ enum {
+ ARM_LDREX_B = (1 << 0), /// byte (8-bit)
+ ARM_LDREX_H = (1 << 1), /// half (16-bit)
+ ARM_LDREX_W = (1 << 2), /// word (32-bit)
+ ARM_LDREX_D = (1 << 3), /// double (64-bit)
+ };
+
+ virtual unsigned getARMLDREXMask() const { return 0; }
+
/// Returns whether the passed in string is a valid clobber in an
/// inline asm statement.
///
diff --git a/clang/include/clang/Sema/SemaARM.h
b/clang/include/clang/Sema/SemaARM.h
index 788a7abf5f9c1..0f7838bdac70f 100644
--- a/clang/include/clang/Sema/SemaARM.h
+++ b/clang/include/clang/Sema/SemaARM.h
@@ -44,8 +44,8 @@ class SemaARM : public SemaBase {
bool CheckImmediateArg(CallExpr *TheCall, unsigned CheckTy, unsigned ArgIdx,
unsigned EltBitWidth, unsigned VecBitWidth);
- bool CheckARMBuiltinExclusiveCall(unsigned BuiltinID, CallExpr *TheCall,
- unsigned MaxWidth);
+ bool CheckARMBuiltinExclusiveCall(const TargetInfo &TI, unsigned BuiltinID,
+ CallExpr *TheCall);
bool CheckNeonBuiltinFunctionCall(const TargetInfo &TI, unsigned BuiltinID,
CallExpr *TheCall);
bool PerformNeonImmChecks(
diff --git a/clang/lib/Basic/Targets/ARM.cpp b/clang/lib/Basic/Targets/ARM.cpp
index 29de34bbc4fe4..6bec2fae0fbd0 100644
--- a/clang/lib/Basic/Targets/ARM.cpp
+++ b/clang/lib/Basic/Targets/ARM.cpp
@@ -618,21 +618,21 @@ bool
ARMTargetInfo::handleTargetFeatures(std::vector<std::string> &Features,
LDREX = 0;
else if (ArchKind == llvm::ARM::ArchKind::ARMV6K ||
ArchKind == llvm::ARM::ArchKind::ARMV6KZ)
- LDREX = LDREX_D | LDREX_W | LDREX_H | LDREX_B;
+ LDREX = ARM_LDREX_D | ARM_LDREX_W | ARM_LDREX_H | ARM_LDREX_B;
else
- LDREX = LDREX_W;
+ LDREX = ARM_LDREX_W;
break;
case 7:
case 8:
if (ArchProfile == llvm::ARM::ProfileKind::M)
- LDREX = LDREX_W | LDREX_H | LDREX_B;
+ LDREX = ARM_LDREX_W | ARM_LDREX_H | ARM_LDREX_B;
else
- LDREX = LDREX_D | LDREX_W | LDREX_H | LDREX_B;
+ LDREX = ARM_LDREX_D | ARM_LDREX_W | ARM_LDREX_H | ARM_LDREX_B;
break;
case 9:
assert(ArchProfile != llvm::ARM::ProfileKind::M &&
"No Armv9-M architectures defined");
- LDREX = LDREX_D | LDREX_W | LDREX_H | LDREX_B;
+ LDREX = ARM_LDREX_D | ARM_LDREX_W | ARM_LDREX_H | ARM_LDREX_B;
}
if (!(FPU & NeonFPU) && FPMath == FP_Neon) {
diff --git a/clang/lib/Basic/Targets/ARM.h b/clang/lib/Basic/Targets/ARM.h
index 1719217ca25b7..43c4718f4735b 100644
--- a/clang/lib/Basic/Targets/ARM.h
+++ b/clang/lib/Basic/Targets/ARM.h
@@ -98,13 +98,6 @@ class LLVM_LIBRARY_VISIBILITY ARMTargetInfo : public
TargetInfo {
LLVM_PREFERRED_TYPE(bool)
unsigned HasBTI : 1;
- enum {
- LDREX_B = (1 << 0), /// byte (8-bit)
- LDREX_H = (1 << 1), /// half (16-bit)
- LDREX_W = (1 << 2), /// word (32-bit)
- LDREX_D = (1 << 3), /// double (64-bit)
- };
-
uint32_t LDREX;
// ACLE 6.5.1 Hardware floating point
@@ -225,6 +218,8 @@ class LLVM_LIBRARY_VISIBILITY ARMTargetInfo : public
TargetInfo {
bool hasBitIntType() const override { return true; }
+ unsigned getARMLDREXMask() const override { return LDREX; }
+
const char *getBFloat16Mangling() const override { return "u6__bf16"; };
std::pair<unsigned, unsigned> hardwareInterferenceSizes() const override {
diff --git a/clang/lib/Sema/SemaARM.cpp b/clang/lib/Sema/SemaARM.cpp
index bd603a925d15e..d7cef178045fc 100644
--- a/clang/lib/Sema/SemaARM.cpp
+++ b/clang/lib/Sema/SemaARM.cpp
@@ -846,9 +846,9 @@ bool SemaARM::CheckARMCoprocessorImmediate(const TargetInfo
&TI,
return false;
}
-bool SemaARM::CheckARMBuiltinExclusiveCall(unsigned BuiltinID,
- CallExpr *TheCall,
- unsigned MaxWidth) {
+bool SemaARM::CheckARMBuiltinExclusiveCall(const TargetInfo &TI,
+ unsigned BuiltinID,
+ CallExpr *TheCall) {
assert((BuiltinID == ARM::BI__builtin_arm_ldrex ||
BuiltinID == ARM::BI__builtin_arm_ldaex ||
BuiltinID == ARM::BI__builtin_arm_strex ||
@@ -923,12 +923,56 @@ bool SemaARM::CheckARMBuiltinExclusiveCall(unsigned
BuiltinID,
return true;
}
- // But ARM doesn't have instructions to deal with 128-bit versions.
- if (Context.getTypeSize(ValType) > MaxWidth) {
- assert(MaxWidth == 64 && "Diagnostic unexpectedly inaccurate");
- Diag(DRE->getBeginLoc(), diag::err_atomic_exclusive_builtin_pointer_size)
- << PointerArg->getType() << PointerArg->getSourceRange();
- return true;
+ // Check whether the size of the type can be handled atomically on this
+ // target.
+ if (!TI.getTriple().isAArch64()) {
+ unsigned Mask = TI.getARMLDREXMask();
+ unsigned Bits = Context.getTypeSize(ValType);
+ bool Supported =
+ (llvm::isPowerOf2_64(Bits)) && Bits >= 8 && (Mask & (Bits / 8));
+
+ if (!Supported) {
+ // Emit a diagnostic saying that this size isn't available. If _no_ size
+ // of exclusive access is supported on this target, we emit a diagnostic
+ // with special wording for that case, but otherwise, we emit
+ // err_atomic_exclusive_builtin_pointer_size and loop over `Mask` to
+ // control what subset of sizes it lists as legal.
+ if (Mask) {
+ auto D = Diag(DRE->getBeginLoc(),
+ diag::err_atomic_exclusive_builtin_pointer_size)
+ << PointerArg->getType();
+ bool Started = false;
+ for (unsigned Size = 1; Size <= 8; Size <<= 1) {
+ // For each of the sizes 1,2,4,8, pass two integers into the
+ // diagnostic. The first selects a separator from the previous
+ // number: 0 for no separator at all, 1 for a comma, 2 for " or "
+ // which appears before the final number in a list of more than one.
+ // The second integer just indicates whether we print this size in
+ // the message at all.
+ if (!(Mask & Size)) {
+ // This size isn't one of the supported ones, so emit no separator
+ // text and don't print the size itself.
+ D << 0 << 0;
+ } else {
+ // This size is supported, so print it, and an appropriate
+ // separator.
+ Mask &= ~Size;
+ if (!Started)
+ D << 0; // No separator if this is the first size we've printed
+ else if (Mask)
+ D << 1; // "," if there's still another size to come
+ else
+ D << 2; // " or " if the size we're about to print is the last
+ D << 1; // print the size itself
+ Started = true;
+ }
+ }
+ } else {
+ Diag(DRE->getBeginLoc(),
+ diag::err_atomic_exclusive_builtin_pointer_size_none)
+ << PointerArg->getSourceRange();
+ }
+ }
}
switch (ValType.getObjCLifetime()) {
@@ -972,7 +1016,7 @@ bool SemaARM::CheckARMBuiltinFunctionCall(const TargetInfo
&TI,
BuiltinID == ARM::BI__builtin_arm_ldaex ||
BuiltinID == ARM::BI__builtin_arm_strex ||
BuiltinID == ARM::BI__builtin_arm_stlex) {
- return CheckARMBuiltinExclusiveCall(BuiltinID, TheCall, 64);
+ return CheckARMBuiltinExclusiveCall(TI, BuiltinID, TheCall);
}
if (BuiltinID == ARM::BI__builtin_arm_prefetch) {
@@ -1053,7 +1097,7 @@ bool SemaARM::CheckAArch64BuiltinFunctionCall(const
TargetInfo &TI,
BuiltinID == AArch64::BI__builtin_arm_ldaex ||
BuiltinID == AArch64::BI__builtin_arm_strex ||
BuiltinID == AArch64::BI__builtin_arm_stlex) {
- return CheckARMBuiltinExclusiveCall(BuiltinID, TheCall, 128);
+ return CheckARMBuiltinExclusiveCall(TI, BuiltinID, TheCall);
}
if (BuiltinID == AArch64::BI__builtin_arm_prefetch) {
diff --git a/clang/test/Sema/builtins-arm-exclusive-124.c
b/clang/test/Sema/builtins-arm-exclusive-124.c
new file mode 100644
index 0000000000000..013ae3f41ee7f
--- /dev/null
+++ b/clang/test/Sema/builtins-arm-exclusive-124.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple armv7m -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple armv8m.main -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple armv8.1m.main -fsyntax-only -verify %s
+
+// All these architecture versions provide 1-, 2- or 4-byte exclusive accesses,
+// but don't have the LDREXD instruction which takes two operand registers and
+// performs an 8-byte exclusive access. So the calls with a pointer to long
+// long are rejected.
+
+int test_ldrex(char *addr) {
+ int sum = 0;
+ sum += __builtin_arm_ldrex(addr);
+ sum += __builtin_arm_ldrex((short *)addr);
+ sum += __builtin_arm_ldrex((int *)addr);
+ sum += __builtin_arm_ldrex((long long *)addr); // expected-error {{address
argument to load or store exclusive builtin must be a pointer to 1,2 or 4 byte
type}}
+ return sum;
+}
+
+int test_strex(char *addr) {
+ int res = 0;
+ res |= __builtin_arm_strex(4, addr);
+ res |= __builtin_arm_strex(42, (short *)addr);
+ res |= __builtin_arm_strex(42, (int *)addr);
+ res |= __builtin_arm_strex(42, (long long *)addr); // expected-error
{{address argument to load or store exclusive builtin must be a pointer to 1,2
or 4 byte type}}
+ return res;
+}
diff --git a/clang/test/Sema/builtins-arm-exclusive-4.c
b/clang/test/Sema/builtins-arm-exclusive-4.c
new file mode 100644
index 0000000000000..68f01f5416616
--- /dev/null
+++ b/clang/test/Sema/builtins-arm-exclusive-4.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple armv6 -fsyntax-only -verify %s
+
+// Armv6 (apart from Armv6-M) provides 4-byte exclusive accesses, but not any
+// other size. So only the calls with a pointer to a 32-bit type are accepted.
+
+int test_ldrex(char *addr) {
+ int sum = 0;
+ sum += __builtin_arm_ldrex(addr); // expected-error {{address argument to
load or store exclusive builtin must be a pointer to 4 byte type}}
+ sum += __builtin_arm_ldrex((short *)addr); // expected-error {{address
argument to load or store exclusive builtin must be a pointer to 4 byte type}}
+ sum += __builtin_arm_ldrex((int *)addr);
+ sum += __builtin_arm_ldrex((long long *)addr); // expected-error {{address
argument to load or store exclusive builtin must be a pointer to 4 byte type}}
+ return sum;
+}
+
+int test_strex(char *addr) {
+ int res = 0;
+ res |= __builtin_arm_strex(4, addr); // expected-error {{address argument to
load or store exclusive builtin must be a pointer to 4 byte type}}
+ res |= __builtin_arm_strex(42, (short *)addr); // expected-error {{address
argument to load or store exclusive builtin must be a pointer to 4 byte type}}
+ res |= __builtin_arm_strex(42, (int *)addr);
+ res |= __builtin_arm_strex(42, (long long *)addr); // expected-error
{{address argument to load or store exclusive builtin must be a pointer to 4
byte type}}
+ return res;
+}
diff --git a/clang/test/Sema/builtins-arm-exclusive-none.c
b/clang/test/Sema/builtins-arm-exclusive-none.c
new file mode 100644
index 0000000000000..76d327f0111c3
--- /dev/null
+++ b/clang/test/Sema/builtins-arm-exclusive-none.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple armv6m -fsyntax-only -verify %s
+
+// Armv6-M does not support exclusive loads/stores at all, so all uses of
+// __builtin_arm_ldrex and __builtin_arm_strex is forbidden.
+
+int test_ldrex(char *addr) {
+ int sum = 0;
+ sum += __builtin_arm_ldrex(addr); // expected-error {{load and store
exclusive builtins are not available on this architecture}}
+ sum += __builtin_arm_ldrex((short *)addr); // expected-error {{load and
store exclusive builtins are not available on this architecture}}
+ sum += __builtin_arm_ldrex((int *)addr); // expected-error {{load and store
exclusive builtins are not available on this architecture}}
+ sum += __builtin_arm_ldrex((long long *)addr); // expected-error {{load and
store exclusive builtins are not available on this architecture}}
+ return sum;
+}
+
+int test_strex(char *addr) {
+ int res = 0;
+ res |= __builtin_arm_strex(4, addr); // expected-error {{load and store
exclusive builtins are not available on this architecture}}
+ res |= __builtin_arm_strex(42, (short *)addr); // expected-error {{load and
store exclusive builtins are not available on this architecture}}
+ res |= __builtin_arm_strex(42, (int *)addr); // expected-error {{load and
store exclusive builtins are not available on this architecture}}
+ res |= __builtin_arm_strex(42, (long long *)addr); // expected-error {{load
and store exclusive builtins are not available on this architecture}}
+ return res;
+}
diff --git a/clang/test/Sema/builtins-arm-exclusive.c
b/clang/test/Sema/builtins-arm-exclusive.c
index 68457d266b991..49aea1506f25c 100644
--- a/clang/test/Sema/builtins-arm-exclusive.c
+++ b/clang/test/Sema/builtins-arm-exclusive.c
@@ -1,5 +1,13 @@
// RUN: %clang_cc1 -triple armv7 -fsyntax-only -verify %s
+// General tests of __builtin_arm_ldrex and __builtin_arm_strex error checking.
+//
+// This test is compiled for Armv7-A, which provides exclusive load/store
+// instructions for 1-, 2-, 4- and 8-byte quantities. Other Arm architecture
+// versions provide subsets of those, requiring different error reporting.
+// Those are tested in builtins-arm-exclusive-124.c, builtins-arm-exclusive-4.c
+// and builtins-arm-exclusive-none.c.
+
struct Simple {
char a, b;
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
