Skip to content

Commit ae4596e

Browse files
author
Yonghong Song
committed
[BPF] Report Undefined Behavior from IR
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 discussion [2], various approaches are discussed, e.g., improving compiler to detect undefined behavior due to uninitialized variables, trying to use ubsan (-fsanitize=undefined), and making -ftrivial-auto-var-init=zero as the bpf default flags. I tried [3] with -ftrivial-auto-var-init=zero and eventually we decided no-go since first it may introduce performance regression and second the prog may still be wrong if the prog expects a non-zero value. The ubsan apprach seems not working as well since it involves runtime callback func ([4]). The approach here is not to do complicate compiler analysis to detect whether where is undef behavior which may impact final codegen. Rather, we relies on compiler to do its normal transformation and at later IR passes stage, a BPF backend pass is inserted to check whether undef behavior is in IR or not. Note that if undef behavior indeed impacts codes, the compiler will discard those related codes with simple 'undef' or 'unreachable'. For example, for the case [1], before SCCPPass, the IR looks like ``` define dso_local i32 @repro(ptr noundef %0) #0 section "classifier" { %2 = alloca %struct.ipv6_opt_hdr, align 8 %3 = tail call i64 (ptr, i32, ...) inttoptr (i64 6 to ptr)(ptr noundef nonnull @repro.____fmt, i32 noundef 6) #2 %4 = tail call ptr asm sideeffect "$0 = *(u32 *)($1 + $2)", "=r,r,i"(ptr %0, i64 76) #2, !srcloc !3 %5 = ptrtoint ptr %4 to i64 %6 = trunc i64 %5 to i32 %7 = tail call i64 (ptr, i32, ...) inttoptr (i64 6 to ptr)(ptr noundef nonnull @icmp6_ndisc_validate.____fmt, i32 noundef 23) #2 call void @llvm.lifetime.start.p0(i64 2, ptr nonnull %2) #2 %8 = getelementptr inbounds nuw i8, ptr %2, i64 1 switch i8 undef, label %51 [ i8 59, label %56 i8 44, label %57 i8 0, label %9 i8 43, label %9 i8 51, label %9 i8 60, label %9 ] 9: ; preds = %1, %1, %1, %1 %10 = sub i32 40, %6 ... ``` Note that 'undef' is used for switch key due to one of early pass LoopFullUnrollPass. After SCCPPass: ``` efine dso_local i32 @repro(ptr noundef %0) #0 section "classifier" { %2 = alloca %struct.ipv6_opt_hdr, align 8 %3 = tail call i64 (ptr, i32, ...) inttoptr (i64 6 to ptr)(ptr noundef nonnull @repro.____fmt, i32 noundef 6) #2 %4 = tail call ptr asm sideeffect "$0 = *(u32 *)($1 + $2)", "=r,r,i"(ptr %0, i64 76) #2, !srcloc !3 %5 = ptrtoint ptr %4 to i64 %6 = trunc i64 %5 to i32 %7 = tail call i64 (ptr, i32, ...) inttoptr (i64 6 to ptr)(ptr noundef nonnull @icmp6_ndisc_validate.____fmt, i32 noundef 23) #2 call void @llvm.lifetime.start.p0(i64 2, ptr nonnull %2) #2 %8 = getelementptr inbounds nuw i8, ptr %2, i64 1 unreachable } ``` For another example, ``` $ cat t.c int foo() { int i[2]; return i[1]; } ``` Before SROAPass pass, ``` define dso_local i32 @foo() #0 { %1 = alloca [2 x i32], align 4 call void @llvm.lifetime.start.p0(i64 8, ptr %1) #2 %2 = getelementptr inbounds [2 x i32], ptr %1, i64 0, i64 1 %3 = load i32, ptr %2, align 4, !tbaa !3 call void @llvm.lifetime.end.p0(i64 8, ptr %1) #2 ret i32 %3 } ``` After SROAPass pass, ``` define dso_local i32 @foo() #0 { ret i32 undef } ``` Besides the above two test cases, the following three patterns are also covered: - It is possible llvm may generate codes where a default branch to 'unreachable' location. Ignore such 'unreachable' instances. See [5] or some comments in [2]. - Handle pattern like __bpf_unreachable (defined in bpf_helpers.h). - Functions with naked attribute will have 'unreachable' at the end of function. Ignore such functions. Tested with bpf selftests and there are no warnings issued. [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 [3] #125601 [4] https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/ubsan/ubsan_interface.inc [5] https://lore.kernel.org/lkml/[email protected]/
1 parent 9679735 commit ae4596e

File tree

6 files changed

+309
-0
lines changed

6 files changed

+309
-0
lines changed

‎llvm/lib/Target/BPF/BPF.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ class BPFTargetMachine;
2222
class InstructionSelector;
2323
class PassRegistry;
2424

25+
ModulePass *createBPFCheckUndefIR();
2526
ModulePass *createBPFCheckAndAdjustIR();
2627

2728
FunctionPass *createBPFISelDag(BPFTargetMachine &TM);
@@ -34,6 +35,7 @@ InstructionSelector *createBPFInstructionSelector(const BPFTargetMachine &,
3435
const BPFSubtarget &,
3536
const BPFRegisterBankInfo &);
3637

38+
void initializeBPFCheckUndefIRPass(PassRegistry&);
3739
void initializeBPFCheckAndAdjustIRPass(PassRegistry&);
3840
void initializeBPFDAGToDAGISelLegacyPass(PassRegistry &);
3941
void initializeBPFMIPeepholePass(PassRegistry &);

‎llvm/lib/Target/BPF/BPFCheckUndefIR.cpp

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
//===---------------- BPFAdjustOpt.cpp - Adjust Optimization --------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
//
9+
// Check 'undef' and 'unreachable' IRs and issue proper warnings.
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
#include "BPF.h"
14+
#include "llvm/IR/Constants.h"
15+
#include "llvm/IR/Instruction.h"
16+
#include "llvm/IR/Module.h"
17+
#include "llvm/IR/Type.h"
18+
#include "llvm/IR/Value.h"
19+
#include "llvm/Pass.h"
20+
21+
#define DEBUG_TYPE "bpf-check-undef-ir"
22+
23+
using namespace llvm;
24+
25+
namespace {
26+
27+
class BPFCheckUndefIR final : public ModulePass {
28+
bool runOnModule(Module &F) override;
29+
30+
public:
31+
static char ID;
32+
BPFCheckUndefIR() : ModulePass(ID) {}
33+
34+
private:
35+
void BPFCheckUndefIRImpl(Function &F);
36+
void BPFCheckInst(Function &F, BasicBlock &BB, Instruction &I);
37+
void HandleReturnInsn(Function &F, ReturnInst *I);
38+
void HandleUnreachableInsn(Function &F, BasicBlock &BB, Instruction &I);
39+
};
40+
} // End anonymous namespace
41+
42+
char BPFCheckUndefIR::ID = 0;
43+
INITIALIZE_PASS(BPFCheckUndefIR, DEBUG_TYPE, "BPF Check Undef IRs", false,
44+
false)
45+
46+
ModulePass *llvm::createBPFCheckUndefIR() { return new BPFCheckUndefIR(); }
47+
48+
void BPFCheckUndefIR::HandleReturnInsn(Function &F, ReturnInst *I) {
49+
Value *RetValue = I->getReturnValue();
50+
// PoisonValue is a special UndefValue where compiler intentionally to
51+
// poisons a value since it shouldn't be used.
52+
if (!RetValue || isa<PoisonValue>(RetValue) || !isa<UndefValue>(RetValue))
53+
return;
54+
55+
dbgs() << "WARNING: return undefined value in func " << F.getName()
56+
<< ", due to uninitialized variable?\n";
57+
}
58+
59+
void BPFCheckUndefIR::HandleUnreachableInsn(Function &F, BasicBlock &BB,
60+
Instruction &I) {
61+
// LLVM may create a switch statement with default to a 'unreachable' basic
62+
// block. Do not warn for such cases.
63+
unsigned NumNoSwitches = 0, NumSwitches = 0;
64+
for (BasicBlock *Pred : predecessors(&BB)) {
65+
const Instruction *Term = Pred->getTerminator();
66+
if (Term && Term->getOpcode() == Instruction::Switch) {
67+
NumSwitches++;
68+
continue;
69+
}
70+
NumNoSwitches++;
71+
}
72+
if (NumSwitches > 0 && NumNoSwitches == 0)
73+
return;
74+
75+
// If the previous insn is no return, do not warn for such cases.
76+
// One example is __bpf_unreachable from libbpf bpf_headers.h.
77+
Instruction *PrevI = I.getPrevNonDebugInstruction();
78+
if (PrevI) {
79+
auto *CI = dyn_cast<CallInst>(PrevI);
80+
if (CI && CI->doesNotReturn())
81+
return;
82+
}
83+
84+
dbgs() << "WARNING: unreachable in func " << F.getName()
85+
<< ", due to uninitialized variable?\n";
86+
}
87+
88+
void BPFCheckUndefIR::BPFCheckInst(Function &F, BasicBlock &BB,
89+
Instruction &I) {
90+
switch (I.getOpcode()) {
91+
case Instruction::Ret:
92+
HandleReturnInsn(F, cast<ReturnInst>(&I));
93+
break;
94+
case Instruction::Unreachable:
95+
HandleUnreachableInsn(F, BB, I);
96+
break;
97+
default:
98+
break;
99+
}
100+
}
101+
102+
void BPFCheckUndefIR::BPFCheckUndefIRImpl(Function &F) {
103+
// A 'unreachable' will be added to the end of naked function.
104+
// Let ignore these naked functions.
105+
if (F.hasFnAttribute(Attribute::Naked))
106+
return;
107+
108+
for (auto &BB : F) {
109+
for (auto &I : BB)
110+
BPFCheckInst(F, BB, I);
111+
}
112+
}
113+
114+
bool BPFCheckUndefIR::runOnModule(Module &M) {
115+
for (Function &F : M)
116+
BPFCheckUndefIRImpl(F);
117+
return false;
118+
}

‎llvm/lib/Target/BPF/BPFTargetMachine.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeBPFTarget() {
4545

4646
PassRegistry &PR = *PassRegistry::getPassRegistry();
4747
initializeGlobalISel(PR);
48+
initializeBPFCheckUndefIRPass(PR);
4849
initializeBPFCheckAndAdjustIRPass(PR);
4950
initializeBPFMIPeepholePass(PR);
5051
initializeBPFDAGToDAGISelLegacyPass(PR);
@@ -143,6 +144,7 @@ void BPFTargetMachine::registerPassBuilderCallbacks(PassBuilder &PB) {
143144
}
144145

145146
void BPFPassConfig::addIRPasses() {
147+
addPass(createBPFCheckUndefIR());
146148
addPass(createAtomicExpandLegacyPass());
147149
addPass(createBPFCheckAndAdjustIR());
148150

‎llvm/lib/Target/BPF/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ add_llvm_target(BPFCodeGen
2626
BPFAsmPrinter.cpp
2727
BPFASpaceCastSimplifyPass.cpp
2828
BPFCheckAndAdjustIR.cpp
29+
BPFCheckUndefIR.cpp
2930
BPFFrameLowering.cpp
3031
BPFInstrInfo.cpp
3132
BPFIRPeephole.cpp

‎llvm/test/CodeGen/BPF/undef-sccp.ll

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
; RUN: opt -S -passes='sccp' %s -o %t1
2+
; RUN: opt --bpf-check-undef-ir -S -mtriple=bpf-pc-linux %t1 >& %t2
3+
; RUN: cat %t2 | FileCheck -check-prefixes=CHECK %s
4+
5+
%union.v6addr = type { %struct.anon.1 }
6+
%struct.anon.1 = type { i64, i64 }
7+
%union.macaddr = type { %struct.anon }
8+
%struct.anon = type { i32, i16 }
9+
%struct.icmp6hdr = type { i8, i8, i16, %union.anon }
10+
%union.anon = type { [1 x i32] }
11+
%struct.ipv6_opt_hdr = type { i8, i8 }
12+
13+
@repro.____fmt = internal constant [6 x i8] c"Start\00", align 1
14+
@repro.____fmt.1 = internal constant [4 x i8] c"End\00", align 1
15+
@__packed = dso_local global %union.v6addr zeroinitializer, align 8
16+
@icmp6_ndisc_validate.____fmt = internal constant [23 x i8] c"pre ipv6_hdrlen_offset\00", align 1
17+
@icmp6_ndisc_validate.____fmt.2 = internal constant [24 x i8] c"post ipv6_hdrlen_offset\00", align 1
18+
@icmp6_ndisc_validate.____fmt.3 = internal constant [5 x i8] c"KO 1\00", align 1
19+
@icmp6_ndisc_validate.____fmt.4 = internal constant [5 x i8] c"KO 2\00", align 1
20+
@icmp6_ndisc_validate.____fmt.5 = internal constant [5 x i8] c"ACK \00", align 1
21+
@ipv6_hdrlen_offset.____fmt = internal constant [17 x i8] c"OKOK %d, len: %d\00", align 1
22+
@ipv6_hdrlen_offset.____fmt.6 = internal constant [18 x i8] c"KO INVALID EXTHDR\00", align 1
23+
@llvm.compiler.used = appending global [1 x ptr] [ptr @repro], section "llvm.metadata"
24+
25+
define dso_local i32 @repro(ptr noundef %0) section "classifier" {
26+
%2 = alloca %struct.ipv6_opt_hdr, align 8
27+
%3 = tail call i64 (ptr, i32, ...) inttoptr (i64 6 to ptr)(ptr noundef nonnull @repro.____fmt, i32 noundef 6)
28+
%4 = tail call ptr asm sideeffect "$0 = *(u32 *)($1 + $2)", "=r,r,i"(ptr %0, i64 76)
29+
%5 = ptrtoint ptr %4 to i64
30+
%6 = trunc i64 %5 to i32
31+
%7 = tail call i64 (ptr, i32, ...) inttoptr (i64 6 to ptr)(ptr noundef nonnull @icmp6_ndisc_validate.____fmt, i32 noundef 23)
32+
call void @llvm.lifetime.start.p0(i64 2, ptr nonnull %2)
33+
%8 = getelementptr inbounds nuw i8, ptr %2, i64 1
34+
switch i8 undef, label %51 [
35+
i8 59, label %56
36+
i8 44, label %57
37+
i8 0, label %9
38+
i8 43, label %9
39+
i8 51, label %9
40+
i8 60, label %9
41+
]
42+
43+
; CHECK: unreachable in func repro, due to uninitialized variable?
44+
; CHECK: %8 = getelementptr inbounds nuw i8, ptr %2, i64 1
45+
; CHECK-NEXT: unreachable
46+
47+
9: ; preds = %1, %1, %1, %1
48+
%10 = sub i32 40, %6
49+
%11 = call i64 inttoptr (i64 26 to ptr)(ptr noundef %0, i32 noundef %10, ptr noundef nonnull %2, i32 noundef 2)
50+
%12 = icmp slt i64 %11, 0
51+
br i1 %12, label %57, label %13
52+
53+
13: ; preds = %9
54+
%14 = load i8, ptr %8, align 1
55+
%15 = zext i8 %14 to i32
56+
%16 = shl nuw nsw i32 %15, 3
57+
%17 = add nuw nsw i32 48, %16
58+
%18 = load i8, ptr %2, align 1
59+
switch i8 %18, label %51 [
60+
i8 59, label %56
61+
i8 44, label %57
62+
i8 0, label %19
63+
i8 43, label %19
64+
i8 51, label %19
65+
i8 60, label %19
66+
]
67+
68+
19: ; preds = %13, %13, %13, %13
69+
%20 = sub i32 %17, %6
70+
%21 = call i64 inttoptr (i64 26 to ptr)(ptr noundef %0, i32 noundef %20, ptr noundef nonnull %2, i32 noundef 2)
71+
%22 = icmp slt i64 %21, 0
72+
br i1 %22, label %57, label %23
73+
74+
23: ; preds = %19
75+
%24 = icmp eq i8 %18, 51
76+
%25 = load i8, ptr %8, align 1
77+
%26 = zext i8 %25 to i32
78+
%27 = select i1 %24, i32 2, i32 3
79+
%28 = shl nuw nsw i32 %26, %27
80+
%29 = add nuw nsw i32 %17, 8
81+
%30 = add nuw nsw i32 %29, %28
82+
%31 = load i8, ptr %2, align 1
83+
switch i8 %31, label %51 [
84+
i8 59, label %56
85+
i8 44, label %57
86+
i8 0, label %32
87+
i8 43, label %32
88+
i8 51, label %32
89+
i8 60, label %32
90+
]
91+
92+
32: ; preds = %23, %23, %23, %23
93+
%33 = sub i32 %30, %6
94+
%34 = call i64 inttoptr (i64 26 to ptr)(ptr noundef %0, i32 noundef %33, ptr noundef nonnull %2, i32 noundef 2)
95+
%35 = icmp slt i64 %34, 0
96+
br i1 %35, label %57, label %36
97+
98+
36: ; preds = %32
99+
%37 = icmp eq i8 %31, 51
100+
%38 = load i8, ptr %8, align 1
101+
%39 = zext i8 %38 to i32
102+
%40 = select i1 %37, i32 2, i32 3
103+
%41 = shl nuw nsw i32 %39, %40
104+
%42 = add nuw nsw i32 %30, 8
105+
%43 = add nuw nsw i32 %42, %41
106+
%44 = load i8, ptr %2, align 1
107+
switch i8 %44, label %51 [
108+
i8 59, label %56
109+
i8 44, label %57
110+
i8 0, label %45
111+
i8 43, label %45
112+
i8 51, label %45
113+
i8 60, label %45
114+
]
115+
116+
45: ; preds = %36, %36, %36, %36
117+
%46 = sub i32 %43, %6
118+
%47 = call i64 inttoptr (i64 26 to ptr)(ptr noundef %0, i32 noundef %46, ptr noundef nonnull %2, i32 noundef 2)
119+
%48 = icmp slt i64 %47, 0
120+
br i1 %48, label %57, label %49
121+
122+
49: ; preds = %45
123+
%50 = call i64 (ptr, i32, ...) inttoptr (i64 6 to ptr)(ptr noundef nonnull @ipv6_hdrlen_offset.____fmt.6, i32 noundef 18)
124+
br label %59
125+
126+
51: ; preds = %36, %23, %13, %1
127+
%52 = phi i8 [ undef, %1 ], [ %18, %13 ], [ %31, %23 ], [ %44, %36 ]
128+
%53 = phi i32 [ 40, %1 ], [ %17, %13 ], [ %30, %23 ], [ %43, %36 ]
129+
%54 = call i64 (ptr, i32, ...) inttoptr (i64 6 to ptr)(ptr noundef nonnull @ipv6_hdrlen_offset.____fmt, i32 noundef 17, i32 noundef 0, i32 noundef %53)
130+
%55 = icmp ne i8 %52, 58
131+
br label %59
132+
133+
56: ; preds = %36, %23, %13, %1
134+
br label %59
135+
136+
57: ; preds = %45, %36, %32, %23, %19, %13, %1, %9
137+
%58 = phi i32 [ -134, %9 ], [ -157, %1 ], [ -157, %13 ], [ -134, %19 ], [ -157, %23 ], [ -134, %32 ], [ -157, %36 ], [ -134, %45 ]
138+
br label %59
139+
140+
59: ; preds = %57, %56, %51, %49
141+
%60 = phi i1 [ %55, %51 ], [ undef, %49 ], [ undef, %56 ], [ undef, %57 ]
142+
%61 = phi i32 [ %53, %51 ], [ -156, %49 ], [ -156, %56 ], [ %58, %57 ]
143+
call void @llvm.lifetime.end.p0(i64 2, ptr nonnull %2)
144+
%62 = call i64 (ptr, i32, ...) inttoptr (i64 6 to ptr)(ptr noundef nonnull @icmp6_ndisc_validate.____fmt.2, i32 noundef 24)
145+
%63 = icmp slt i32 %61, 0
146+
%64 = or i1 %63, %60
147+
br i1 %64, label %65, label %67
148+
149+
65: ; preds = %59
150+
%66 = call i64 (ptr, i32, ...) inttoptr (i64 6 to ptr)(ptr noundef nonnull @icmp6_ndisc_validate.____fmt.3, i32 noundef 5)
151+
br label %77
152+
153+
67: ; preds = %59
154+
%68 = call ptr asm sideeffect "$0 = *(u32 *)($1 + $2)", "=r,r,i"(ptr %0, i64 76)
155+
%69 = zext nneg i32 %61 to i64
156+
%70 = getelementptr inbounds nuw i8, ptr %68, i64 %69
157+
%71 = load i8, ptr %70, align 4
158+
%72 = icmp eq i8 %71, -121
159+
br i1 %72, label %75, label %73
160+
161+
73: ; preds = %67
162+
%74 = call i64 (ptr, i32, ...) inttoptr (i64 6 to ptr)(ptr noundef nonnull @icmp6_ndisc_validate.____fmt.4, i32 noundef 5)
163+
br label %77
164+
165+
75: ; preds = %67
166+
%76 = call i64 (ptr, i32, ...) inttoptr (i64 6 to ptr)(ptr noundef nonnull @icmp6_ndisc_validate.____fmt.5, i32 noundef 5)
167+
br label %77
168+
169+
77: ; preds = %65, %73, %75
170+
%78 = call i64 (ptr, i32, ...) inttoptr (i64 6 to ptr)(ptr noundef nonnull @repro.____fmt.1, i32 noundef 4)
171+
ret i32 0
172+
}

‎llvm/test/CodeGen/BPF/undef-sroa.ll

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
; RUN: opt -S -passes='sroa' %s -o %t1
2+
; RUN: opt --bpf-check-undef-ir -S -mtriple=bpf-pc-linux %t1 >& %t2
3+
; RUN: cat %t2 | FileCheck -check-prefixes=CHECK %s
4+
5+
define dso_local i32 @foo() {
6+
%1 = alloca [2 x i32], align 4
7+
call void @llvm.lifetime.start.p0(i64 8, ptr %1)
8+
%2 = getelementptr inbounds [2 x i32], ptr %1, i64 0, i64 1
9+
%3 = load i32, ptr %2, align 4
10+
call void @llvm.lifetime.end.p0(i64 8, ptr %1)
11+
ret i32 %3
12+
}
13+
; CHECK: return undefined value in func foo, due to uninitialized variable?
14+
; CHECK: ret i32 undef

0 commit comments

Comments
 (0)