yonghong-song created this revision.
yonghong-song added a reviewer: rsmith.
Herald added a subscriber: dexonsmith.
yonghong-song requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.
Paul Chaignon reported a bpf verifier failure ([1]) due to using
non-ABI register R11 <https://reviews.llvm.org/source/libunwind/>. For the test
case, llvm11 is okay while
llvm12 and later generates verifier unfriendly code.
The failure is related to variable length array size.
The following mimics the variable length array definition
in the test case:
#define AA 40
struct t { char a[20]; };
void foo(void *);
int test() {
const int a = 8;
char tmp[AA + sizeof(struct t) + a];
foo(tmp);
...
}
Paul helped bisect that the following llvm commit is
responsible:
552c6c232872 ("PR44406: Follow behavior of array bound constant
folding in more recent versions of GCC.")
Basically, before the above commit, clang frontend did constant
folding for array size "AA + sizeof(struct t) + a" to be 68,
so used alloca for stack allocation. After the above commit,
clang frontend didn't do constant folding for array size
any more, which results in a VLA and "llvm.stacksave"/"llvm.stackrestore"
is generated.
BPF architecture API does not support stack pointer (sp) register.
The LLVM internally used R11 <https://reviews.llvm.org/source/libunwind/> to
indicate sp register but it should
not be in the final code. Otherwise, kernel verifier will reject it.
This patch attempts to permit array size constant folding
in clang frontend for BPF target. This allows better code and
better developer experience. Otherwise users will need to manually
recalculate the array size whenever some macro or constant value
changes.
[1] https://lore.kernel.org/bpf/20210809151202.GB1012999@Mem/
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D107882
Files:
clang/lib/Sema/SemaType.cpp
clang/test/CodeGen/bpf-vla.c
llvm/include/llvm/ADT/Triple.h
Index: llvm/include/llvm/ADT/Triple.h
===================================================================
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -738,6 +738,11 @@
: PointerWidth == 64;
}
+ /// Tests whether the target is BPF (little and big endian).
+ bool isBPF() const {
+ return getArch() == Triple::bpfel || getArch() == Triple::bpfeb;
+ }
+
/// Tests whether the target is MIPS 32-bit (little and big endian).
bool isMIPS32() const {
return getArch() == Triple::mips || getArch() == Triple::mipsel;
Index: clang/test/CodeGen/bpf-vla.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/bpf-vla.c
@@ -0,0 +1,29 @@
+// REQUIRES: bpf-registered-target
+// RUN: %clang -target bpf -emit-llvm -S -g %s -o - | FileCheck %s
+
+#define AA 40
+struct t {
+ char a[20];
+};
+void foo(void *);
+int test() {
+ const int a = 8;
+ char tmp[AA + sizeof(struct t) + a];
+
+// CHECK: define dso_local i32 @test(
+// CHECK: %tmp = alloca [68 x i8], align 1
+// CHECK-NOT: llvm.stacksave
+
+ foo(tmp);
+ return 0;
+}
+
+int test2(int b) {
+ const int a = 8;
+ char tmp[a + b];
+
+// CHECK: define dso_local i32 @test2(
+// CHECK: llvm.stacksave
+ foo(tmp);
+ return 0;
+}
Index: clang/lib/Sema/SemaType.cpp
===================================================================
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2279,7 +2279,7 @@
/// ExprError().
static ExprResult checkArraySize(Sema &S, Expr *&ArraySize,
llvm::APSInt &SizeVal, unsigned VLADiag,
- bool VLAIsError) {
+ bool VLAIsError, Sema::AllowFoldKind CanFold) {
if (S.getLangOpts().CPlusPlus14 &&
(VLAIsError ||
!ArraySize->getType()->isIntegralOrUnscopedEnumerationType())) {
@@ -2324,7 +2324,7 @@
} Diagnoser(VLADiag, VLAIsError);
ExprResult R =
- S.VerifyIntegerConstantExpression(ArraySize, &SizeVal, Diagnoser);
+ S.VerifyIntegerConstantExpression(ArraySize, &SizeVal, Diagnoser, CanFold);
if (Diagnoser.IsVLA)
return ExprResult();
return R;
@@ -2442,10 +2442,20 @@
// VLAs always produce at least a -Wvla diagnostic, sometimes an error.
unsigned VLADiag;
bool VLAIsError;
+ AllowFoldKind CanFold = Sema::NoFold;
if (getLangOpts().OpenCL) {
// OpenCL v1.2 s6.9.d: variable length arrays are not supported.
VLADiag = diag::err_opencl_vla;
VLAIsError = true;
+ } else if (Context.getTargetInfo().getTriple().isBPF()) {
+ // BPF target does not support variable length array, but
+ // we don't force an error here. We will try to do constant
+ // folding for array size as much as possible. LLVM backend
+ // will do some checking and warn users if variable length
+ // array still remains.
+ VLADiag = diag::warn_vla_used;
+ VLAIsError = false;
+ CanFold = Sema::AllowFold;
} else if (getLangOpts().C99) {
VLADiag = diag::warn_vla_used;
VLAIsError = false;
@@ -2472,7 +2482,7 @@
T = Context.getDependentSizedArrayType(T, ArraySize, ASM, Quals, Brackets);
} else {
ExprResult R =
- checkArraySize(*this, ArraySize, ConstVal, VLADiag, VLAIsError);
+ checkArraySize(*this, ArraySize, ConstVal, VLADiag, VLAIsError, CanFold);
if (R.isInvalid())
return QualType();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
