Skip to content

Commit e0d1f90

Browse files
author
Yonghong Song
committed
[RFC][clang][BPF] Make trivial uninit var value to be 0
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
1 parent 9d5edc9 commit e0d1f90

File tree

3 files changed

+40
-0
lines changed

3 files changed

+40
-0
lines changed

clang/lib/Basic/Targets/BPF.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,14 @@ void BPFTargetInfo::fillValidCPUList(SmallVectorImpl<StringRef> &Values) const {
8989
Values.append(std::begin(ValidCPUNames), std::end(ValidCPUNames));
9090
}
9191

92+
void BPFTargetInfo::adjust(DiagnosticsEngine &Diags, LangOptions &Opts) {
93+
TargetInfo::adjust(Diags, Opts);
94+
95+
if (Opts.getTrivialAutoVarInit() ==
96+
LangOptions::TrivialAutoVarInitKind::Uninitialized)
97+
Opts.setTrivialAutoVarInit(LangOptions::TrivialAutoVarInitKind::Zero);
98+
}
99+
92100
llvm::SmallVector<Builtin::InfosShard>
93101
BPFTargetInfo::getTargetBuiltins() const {
94102
return {{&BuiltinStrings, BuiltinInfos}};

clang/lib/Basic/Targets/BPF.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ class LLVM_LIBRARY_VISIBILITY BPFTargetInfo : public TargetInfo {
6060

6161
llvm::SmallVector<Builtin::InfosShard> getTargetBuiltins() const override;
6262

63+
void adjust(DiagnosticsEngine &Diags, LangOptions &Opts) override;
64+
6365
std::string_view getClobbers() const override { return ""; }
6466

6567
BuiltinVaListKind getBuiltinVaListKind() const override {
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// REQUIRES: bpf-registered-target
2+
// RUN: %clang_cc1 -triple bpf -O2 -emit-llvm %s -o - | FileCheck %s
3+
4+
int foo1() {
5+
int val;
6+
return val;
7+
}
8+
// CHECK: ret i32 0
9+
10+
int foo2() {
11+
int val[4];
12+
return val[2];
13+
}
14+
// CHECK: ret i32 0
15+
16+
struct val_t {
17+
int val;
18+
};
19+
20+
int foo3() {
21+
struct val_t v;
22+
return v.val;
23+
}
24+
// CHECK: ret i32 0
25+
26+
int foo4() {
27+
int val = 5;
28+
return val;
29+
}
30+
// CHECK: ret i32 5

0 commit comments

Comments
 (0)