[clang] [RFC][clang][BPF] Make trivial uninit var value to be 0 (PR #125601)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Feb 8 19:11:23 PST 2025
https://github.com/yonghong-song updated https://github.com/llvm/llvm-project/pull/125601
>From e0d1f90b24e7b7766bed17710b7072433610725c Mon Sep 17 00:00:00 2001
From: Yonghong Song <yonghong.song at linux.dev>
Date: Sun, 2 Feb 2025 08:33:47 -0800
Subject: [PATCH 1/2] [RFC][clang][BPF] Make trivial uninit var value to be 0
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Marc Suñé (Isovalent, part of Cisco) reported an issue where an
uninitialized variable caused generated bpf prog binary code not
working as expected. The reproducer is in [1] where the flags
“-Wall -Werror” are enabled, but there is no warning and compiler
may take advantage of uninit variable to do aggressive optimization.
In the BPF context, the generated buggy code will be load into
the kernel and likely verification will pass. When the bpf program
actually runs, its result may not be expected and user will then need
to check source or asm code to find reasons. Since compiler may
take advantage of uninit variables for aggressive optimization,
users may be confused with final code without realizing this is
due to uninit variables.
The ideal case is for compiler to issue meaning info about
*actual* uninit variable which impacts transformation. Currently
clang (and gcc) may miss such uninit variables, esp. with cross
function boundaries. See discuss in [2].
The llvm/gcc undef sanitizer is another source to detect such
undef issues. For bpf target, this means potential complexity for
verifier and bpf runtime.
Another idea is to make -ftrivial-auto-var-init=0 as the default
for bpf target. This way, when bpf prog produced unexpected results,
users can check/understand asm codes easier since compiler won't
be able to do aggressive optimization due to uninit variables.
This patch makes -ftrivial-auto-var-init=0 as the default for bpf target.
For example, for code,
int foo1() {
int val;
return val;
}
With this patch, the eventual IR bofore lowering is
ret i32 0;
Without this patch, it is
ret i32 undef
With -ftrivial-auto-var-init=0 on by default, two bpf selftests failed and
below the fix:
diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c
index bd8f15229f5c..72a65ee365ef 100644
--- a/tools/testing/selftests/bpf/progs/dynptr_fail.c
+++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c
@@ -413,7 +413,11 @@ int invalid_helper1(void *ctx)
/* A dynptr can't be passed into a helper function at a non-zero offset */
SEC("?raw_tp")
+#if __clang_major__ >= 21
+__failure __msg("Expected an initialized dynptr as arg #2")
+#else
__failure __msg("cannot pass in dynptr at an offset=-8")
+#endif
int invalid_helper2(void *ctx)
{
struct bpf_dynptr ptr;
diff --git a/tools/testing/selftests/bpf/progs/verifier_mtu.c b/tools/testing/selftests/bpf/progs/verifier_mtu.c
index 256956ea1ac5..c987b20b39eb 100644
--- a/tools/testing/selftests/bpf/progs/verifier_mtu.c
+++ b/tools/testing/selftests/bpf/progs/verifier_mtu.c
@@ -8,7 +8,9 @@ SEC("tc/ingress")
__description("uninit/mtu: write rejected")
__success
__caps_unpriv(CAP_BPF|CAP_NET_ADMIN)
+#if __clang_major__ < 21
__failure_unpriv __msg_unpriv("invalid read from stack")
+#endif
int tc_uninit_mtu(struct __sk_buff *ctx)
{
__u32 mtu;
Not sure whether we should introduce a bpf specific flag to disable
-ftrivial-auto-var-init=0 or not. I guess probably not. But the
kernel change still needed.
[1] https://github.com/msune/clang_bpf/blob/main/Makefile#L3
[2] https://discourse.llvm.org/t/detect-undefined-behavior-due-to-uninitialized-variables-in-bpf-programs/84116?u=yonghong-song
---
clang/lib/Basic/Targets/BPF.cpp | 8 +++++++
clang/lib/Basic/Targets/BPF.h | 2 ++
clang/test/CodeGen/bpf-auto-var-init.c | 30 ++++++++++++++++++++++++++
3 files changed, 40 insertions(+)
create mode 100644 clang/test/CodeGen/bpf-auto-var-init.c
diff --git a/clang/lib/Basic/Targets/BPF.cpp b/clang/lib/Basic/Targets/BPF.cpp
index a463de088402014..8db426be307ae23 100644
--- a/clang/lib/Basic/Targets/BPF.cpp
+++ b/clang/lib/Basic/Targets/BPF.cpp
@@ -89,6 +89,14 @@ void BPFTargetInfo::fillValidCPUList(SmallVectorImpl<StringRef> &Values) const {
Values.append(std::begin(ValidCPUNames), std::end(ValidCPUNames));
}
+void BPFTargetInfo::adjust(DiagnosticsEngine &Diags, LangOptions &Opts) {
+ TargetInfo::adjust(Diags, Opts);
+
+ if (Opts.getTrivialAutoVarInit() ==
+ LangOptions::TrivialAutoVarInitKind::Uninitialized)
+ Opts.setTrivialAutoVarInit(LangOptions::TrivialAutoVarInitKind::Zero);
+}
+
llvm::SmallVector<Builtin::InfosShard>
BPFTargetInfo::getTargetBuiltins() const {
return {{&BuiltinStrings, BuiltinInfos}};
diff --git a/clang/lib/Basic/Targets/BPF.h b/clang/lib/Basic/Targets/BPF.h
index d1f68b842348ea1..80f2593823abeb2 100644
--- a/clang/lib/Basic/Targets/BPF.h
+++ b/clang/lib/Basic/Targets/BPF.h
@@ -60,6 +60,8 @@ class LLVM_LIBRARY_VISIBILITY BPFTargetInfo : public TargetInfo {
llvm::SmallVector<Builtin::InfosShard> getTargetBuiltins() const override;
+ void adjust(DiagnosticsEngine &Diags, LangOptions &Opts) override;
+
std::string_view getClobbers() const override { return ""; }
BuiltinVaListKind getBuiltinVaListKind() const override {
diff --git a/clang/test/CodeGen/bpf-auto-var-init.c b/clang/test/CodeGen/bpf-auto-var-init.c
new file mode 100644
index 000000000000000..09023279e1eeeb7
--- /dev/null
+++ b/clang/test/CodeGen/bpf-auto-var-init.c
@@ -0,0 +1,30 @@
+// REQUIRES: bpf-registered-target
+// RUN: %clang_cc1 -triple bpf -O2 -emit-llvm %s -o - | FileCheck %s
+
+int foo1() {
+ int val;
+ return val;
+}
+// CHECK: ret i32 0
+
+int foo2() {
+ int val[4];
+ return val[2];
+}
+// CHECK: ret i32 0
+
+struct val_t {
+ int val;
+};
+
+int foo3() {
+ struct val_t v;
+ return v.val;
+}
+// CHECK: ret i32 0
+
+int foo4() {
+ int val = 5;
+ return val;
+}
+// CHECK: ret i32 5
>From 22a513a3592bd0f1c8eaea512626661b385ed1dd Mon Sep 17 00:00:00 2001
From: Yonghong Song <yonghong.song at linux.dev>
Date: Sat, 8 Feb 2025 18:27:10 -0800
Subject: [PATCH 2/2] Set TrivialAutoVarInitMaxSize to 8
This is to prevent auto init for large data structures, e.g.
for xdp prog, iphdr or tcphdr they all more than 8 bytes.
If auto init is not really needed, auto init might cause
performance regression for those programs.
---
clang/include/clang/Basic/LangOptions.h | 7 +++++++
clang/lib/Basic/Targets/BPF.cpp | 6 +++++-
clang/test/CodeGen/bpf-auto-var-init.c | 6 ------
3 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index f58a719a45a84de..858065c498846fc 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -653,6 +653,13 @@ class LangOptions : public LangOptionsBase {
void set##Name(Type Value) { Name = static_cast<unsigned>(Value); }
#include "clang/Basic/LangOptions.def"
+ unsigned getTrivialAutoVarInitMaxSize() const {
+ return TrivialAutoVarInitMaxSize;
+ }
+ void setTrivialAutoVarInitMaxSize(unsigned max_size) {
+ TrivialAutoVarInitMaxSize = max_size;
+ }
+
/// Are we compiling a module?
bool isCompilingModule() const {
return getCompilingModule() != CMK_None;
diff --git a/clang/lib/Basic/Targets/BPF.cpp b/clang/lib/Basic/Targets/BPF.cpp
index 8db426be307ae23..52efce268642e8b 100644
--- a/clang/lib/Basic/Targets/BPF.cpp
+++ b/clang/lib/Basic/Targets/BPF.cpp
@@ -93,8 +93,12 @@ void BPFTargetInfo::adjust(DiagnosticsEngine &Diags, LangOptions &Opts) {
TargetInfo::adjust(Diags, Opts);
if (Opts.getTrivialAutoVarInit() ==
- LangOptions::TrivialAutoVarInitKind::Uninitialized)
+ LangOptions::TrivialAutoVarInitKind::Uninitialized) {
Opts.setTrivialAutoVarInit(LangOptions::TrivialAutoVarInitKind::Zero);
+ // Set the maximum auto init size to be 8 to avoid potential regression
+ // e.g. for xdp programs where ip/tcp header size is more than 8.
+ Opts.setTrivialAutoVarInitMaxSize(8);
+ }
}
llvm::SmallVector<Builtin::InfosShard>
diff --git a/clang/test/CodeGen/bpf-auto-var-init.c b/clang/test/CodeGen/bpf-auto-var-init.c
index 09023279e1eeeb7..ea9db64c5d9744a 100644
--- a/clang/test/CodeGen/bpf-auto-var-init.c
+++ b/clang/test/CodeGen/bpf-auto-var-init.c
@@ -7,12 +7,6 @@ int foo1() {
}
// CHECK: ret i32 0
-int foo2() {
- int val[4];
- return val[2];
-}
-// CHECK: ret i32 0
-
struct val_t {
int val;
};
More information about the cfe-commits
mailing list