Skip to content

Commit c78aba3

Browse files
author
Yonghong Song
committed
[BPF] Handle unreachable with a kfunc call
Currently, middle-end generates 'unreachable' insn if the compiler feels the code is indeed unreachable or the code becomes invalid due to some optimizaiton (e.g. code optimization with uninitialized variables). Right now BPF backend ignores 'unreachable' insn during selectiondag lowering. For cases where 'unreachable' is due to invalid code transformation, such a signal will be missed. Later on, users needs some effort to debug it which impacts developer productivity. This patch enabled selectiondag lowering for 'unreachable' insn. Previous attempt ([1]) tries to have a backend IR pass to filter out 'unreachable' insns in a number of cases. But such pattern matching may misalign with future middle-end optimization with 'unreachable' insns. This patch takes a different approach. The 'unreachable' insn is lowered with special encoding in bpf object file and verifier will do proper verification for the bpf prog. More specifically, the 'unreachable' insn is replaced by a '__bpf_unreachable' function. This function will be a kfunc (in ".ksyms" section) with a weak attribute, but does not have definition. The function is also present in prog btf. This way, the extern '__bpf_unreachable' can be handled properly in libbpf func poison_kfunc_call(). The name '__bpf_unreachable' is chosen to satisfy reserved identifier requirement. I tested this patch on bpf selftests and all tests are passed. I also tried original example in [2] and the error is properly detected by verifier: func#0 @0 last insn is not an exit or jmp In another internal sched-ext bpf prog, with the patch we have bpf code: Disassembly of section .text: 0000000000000000 <scx_storage_init_single>: ; { 0: bc 13 00 00 00 00 00 00 w3 = w1 1: b4 01 00 00 00 00 00 00 w1 = 0x0 ; const u32 zero = 0; ... 0000000000003a80 <create_dom>: ; { 1872: bc 16 00 00 00 00 00 00 w6 = w1 ; bpf_printk("dom_id %d", dom_id); 1873: 18 01 00 00 3f 00 00 00 00 00 00 00 00 00 00 00 r1 = 0x3f ll 0000000000003a88: R_BPF_64_64 .rodata 1875: b4 02 00 00 0a 00 00 00 w2 = 0xa 1876: bc 63 00 00 00 00 00 00 w3 = w6 1877: 85 00 00 00 06 00 00 00 call 0x6 ; ret = scx_bpf_create_dsq(dom_id, 0); 1878: bc 61 00 00 00 00 00 00 w1 = w6 1879: b4 02 00 00 00 00 00 00 w2 = 0x0 1880: 85 10 00 00 ff ff ff ff call -0x1 0000000000003ac0: R_BPF_64_32 scx_bpf_create_dsq ; domc->node_cpumask = node_data[node_id]; 1881: 85 10 00 00 ff ff ff ff call -0x1 0000000000003ac8: R_BPF_64_32 __bpf_unreachable <END> The verifier can easily report the error too. A bpf flag `-bpf-disable-trap-unreachable` is introduced to disable trapping unreachable. [1] #126858 [2] https://github.com/msune/clang_bpf/blob/main/Makefile#L3
1 parent 6375a85 commit c78aba3

File tree

6 files changed

+139
-4
lines changed

6 files changed

+139
-4
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,8 @@ class BPFTargetMachine;
2222
class InstructionSelector;
2323
class PassRegistry;
2424

25+
static const char *BPF_UNREACHABLE = "__bpf_unreachable";
26+
2527
ModulePass *createBPFCheckAndAdjustIR();
2628

2729
FunctionPass *createBPFISelDag(BPFTargetMachine &TM);

llvm/lib/Target/BPF/BPFISelLowering.cpp

Lines changed: 58 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@
2121
#include "llvm/CodeGen/MachineRegisterInfo.h"
2222
#include "llvm/CodeGen/TargetLoweringObjectFileImpl.h"
2323
#include "llvm/CodeGen/ValueTypes.h"
24+
#include "llvm/IR/DIBuilder.h"
2425
#include "llvm/IR/DiagnosticInfo.h"
2526
#include "llvm/IR/DiagnosticPrinter.h"
27+
#include "llvm/IR/Module.h"
2628
#include "llvm/Support/Debug.h"
2729
#include "llvm/Support/ErrorHandling.h"
2830
#include "llvm/Support/MathExtras.h"
@@ -68,6 +70,8 @@ BPFTargetLowering::BPFTargetLowering(const TargetMachine &TM,
6870
setOperationAction(ISD::BRIND, MVT::Other, Expand);
6971
setOperationAction(ISD::BRCOND, MVT::Other, Expand);
7072

73+
setOperationAction(ISD::TRAP, MVT::Other, Custom);
74+
7175
setOperationAction({ISD::GlobalAddress, ISD::ConstantPool}, MVT::i64, Custom);
7276

7377
setOperationAction(ISD::DYNAMIC_STACKALLOC, MVT::i64, Custom);
@@ -326,6 +330,8 @@ SDValue BPFTargetLowering::LowerOperation(SDValue Op, SelectionDAG &DAG) const {
326330
case ISD::ATOMIC_LOAD:
327331
case ISD::ATOMIC_STORE:
328332
return LowerATOMIC_LOAD_STORE(Op, DAG);
333+
case ISD::TRAP:
334+
return LowerTRAP(Op, DAG);
329335
}
330336
}
331337

@@ -521,10 +527,12 @@ SDValue BPFTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
521527
Callee = DAG.getTargetGlobalAddress(G->getGlobal(), CLI.DL, PtrVT,
522528
G->getOffset(), 0);
523529
} else if (ExternalSymbolSDNode *E = dyn_cast<ExternalSymbolSDNode>(Callee)) {
524-
Callee = DAG.getTargetExternalSymbol(E->getSymbol(), PtrVT, 0);
525-
fail(CLI.DL, DAG,
526-
Twine("A call to built-in function '" + StringRef(E->getSymbol()) +
527-
"' is not supported."));
530+
if (StringRef(E->getSymbol()) != BPF_UNREACHABLE) {
531+
Callee = DAG.getTargetExternalSymbol(E->getSymbol(), PtrVT, 0);
532+
fail(CLI.DL, DAG,
533+
Twine("A call to built-in function '" + StringRef(E->getSymbol()) +
534+
"' is not supported."));
535+
}
528536
}
529537

530538
// Returns a chain & a flag for retval copy to use.
@@ -726,6 +734,52 @@ SDValue BPFTargetLowering::LowerATOMIC_LOAD_STORE(SDValue Op,
726734
return Op;
727735
}
728736

737+
static Function *createBPFUnreachable(Module *M) {
738+
if (auto *Fn = M->getFunction(BPF_UNREACHABLE))
739+
return Fn;
740+
741+
FunctionType *FT = FunctionType::get(Type::getVoidTy(M->getContext()), false);
742+
Function *NewF = Function::Create(FT, GlobalValue::ExternalWeakLinkage,
743+
BPF_UNREACHABLE, M);
744+
NewF->setDSOLocal(true);
745+
NewF->setCallingConv(CallingConv::C);
746+
NewF->setSection(".ksyms");
747+
748+
if (M->debug_compile_units().empty())
749+
return NewF;
750+
751+
DIBuilder DBuilder(*M);
752+
DITypeRefArray ParamTypes =
753+
DBuilder.getOrCreateTypeArray({nullptr /*void return*/});
754+
DISubroutineType *FuncType = DBuilder.createSubroutineType(ParamTypes);
755+
DICompileUnit *CU = *M->debug_compile_units_begin();
756+
DISubprogram *SP = DBuilder.createFunction(
757+
CU, BPF_UNREACHABLE, BPF_UNREACHABLE, nullptr, 0, FuncType, 0,
758+
DINode::FlagZero, DISubprogram::SPFlagZero);
759+
NewF->setSubprogram(SP);
760+
return NewF;
761+
}
762+
763+
SDValue BPFTargetLowering::LowerTRAP(SDValue Op, SelectionDAG &DAG) const {
764+
MachineFunction &MF = DAG.getMachineFunction();
765+
TargetLowering::CallLoweringInfo CLI(DAG);
766+
SmallVector<SDValue> InVals;
767+
SDNode *N = Op.getNode();
768+
SDLoc DL(N);
769+
770+
Function *Fn = createBPFUnreachable(MF.getFunction().getParent());
771+
auto PtrVT = getPointerTy(MF.getDataLayout());
772+
CLI.Callee = DAG.getTargetGlobalAddress(Fn, DL, PtrVT);
773+
CLI.Chain = N->getOperand(0);
774+
CLI.IsTailCall = false;
775+
CLI.CallConv = CallingConv::C;
776+
CLI.IsVarArg = false;
777+
CLI.DL = DL;
778+
CLI.NoMerge = false;
779+
CLI.DoesNotReturn = true;
780+
return LowerCall(CLI, InVals);
781+
}
782+
729783
const char *BPFTargetLowering::getTargetNodeName(unsigned Opcode) const {
730784
switch ((BPFISD::NodeType)Opcode) {
731785
case BPFISD::FIRST_NUMBER:

llvm/lib/Target/BPF/BPFISelLowering.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ class BPFTargetLowering : public TargetLowering {
8080
SDValue LowerATOMIC_LOAD_STORE(SDValue Op, SelectionDAG &DAG) const;
8181
SDValue LowerConstantPool(SDValue Op, SelectionDAG &DAG) const;
8282
SDValue LowerGlobalAddress(SDValue Op, SelectionDAG &DAG) const;
83+
SDValue LowerTRAP(SDValue Op, SelectionDAG &DAG) const;
8384

8485
template <class NodeTy>
8586
SDValue getAddr(NodeTy *N, SelectionDAG &DAG, unsigned Flags = 0) const;

llvm/lib/Target/BPF/BPFMIPeephole.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,7 @@ struct BPFMIPreEmitPeephole : public MachineFunctionPass {
320320
bool adjustBranch();
321321
bool insertMissingCallerSavedSpills();
322322
bool removeMayGotoZero();
323+
bool addExitAfterUnreachable();
323324

324325
public:
325326

@@ -336,6 +337,7 @@ struct BPFMIPreEmitPeephole : public MachineFunctionPass {
336337
Changed = adjustBranch() || Changed;
337338
Changed |= insertMissingCallerSavedSpills();
338339
Changed |= removeMayGotoZero();
340+
Changed |= addExitAfterUnreachable();
339341
return Changed;
340342
}
341343
};
@@ -734,6 +736,20 @@ bool BPFMIPreEmitPeephole::removeMayGotoZero() {
734736
return Changed;
735737
}
736738

739+
// If the last insn in a funciton is 'JAL &bpf_unreachable', let us add an
740+
// 'exit' insn after that insn. This will ensure no fallthrough at the last
741+
// insn, making kernel verification easier.
742+
bool BPFMIPreEmitPeephole::addExitAfterUnreachable() {
743+
MachineBasicBlock &MBB = MF->back();
744+
MachineInstr &MI = MBB.back();
745+
if (MI.getOpcode() != BPF::JAL || !MI.getOperand(0).isGlobal() ||
746+
MI.getOperand(0).getGlobal()->getName() != BPF_UNREACHABLE)
747+
return false;
748+
749+
BuildMI(&MBB, MI.getDebugLoc(), TII->get(BPF::RET));
750+
return true;
751+
}
752+
737753
} // end default namespace
738754

739755
INITIALIZE_PASS(BPFMIPreEmitPeephole, "bpf-mi-pemit-peephole",

llvm/lib/Target/BPF/BPFTargetMachine.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ static cl::
3737
opt<bool> DisableMIPeephole("disable-bpf-peephole", cl::Hidden,
3838
cl::desc("Disable machine peepholes for BPF"));
3939

40+
static cl::opt<bool>
41+
DisableCheckUnreachable("bpf-disable-trap-unreachable", cl::Hidden,
42+
cl::desc("Disable Trap Unreachable for BPF"));
43+
4044
extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeBPFTarget() {
4145
// Register the target.
4246
RegisterTargetMachine<BPFTargetMachine> X(getTheBPFleTarget());
@@ -77,6 +81,11 @@ BPFTargetMachine::BPFTargetMachine(const Target &T, const Triple &TT,
7781
getEffectiveCodeModel(CM, CodeModel::Small), OL),
7882
TLOF(std::make_unique<TargetLoweringObjectFileELF>()),
7983
Subtarget(TT, std::string(CPU), std::string(FS), *this) {
84+
if (!DisableCheckUnreachable) {
85+
this->Options.TrapUnreachable = true;
86+
this->Options.NoTrapAfterNoreturn = true;
87+
}
88+
8089
initAsmInfo();
8190

8291
BPFMCAsmInfo *MAI =
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
; RUN: llc -mtriple=bpfel -mcpu=v3 -filetype=obj -o %t1 %s
2+
; RUN: llvm-objcopy --dump-section='.BTF'=%t2 %t1
3+
; RUN: %python %p/print_btf.py %t2 | FileCheck -check-prefixes=CHECK-BTF %s
4+
; RUN: llc -mtriple=bpfel -mcpu=v3 < %s | FileCheck -check-prefixes=CHECK %s
5+
6+
define void @foo() {
7+
entry:
8+
tail call void @bar()
9+
unreachable
10+
}
11+
12+
; CHECK: foo:
13+
; CHECK-NEXT: .Lfunc_begin0:
14+
; CHECK-NEXT: .cfi_startproc
15+
; CHECK-NEXT: # %bb.0:
16+
; CHECK-NEXT: call bar
17+
; CHECK-NEXT: call __bpf_unreachable
18+
; CHECK-NEXT: exit
19+
; CHECK-NEXT: .Lfunc_end0:
20+
21+
define void @buz() #0 {
22+
entry:
23+
tail call void asm sideeffect "r0 = r1; exit;", ""()
24+
unreachable
25+
}
26+
27+
; CHECK: buz:
28+
; CHECK-NEXT: .Lfunc_begin1:
29+
; CHECK-NEXT: .cfi_startproc
30+
; CHECK-NEXT: # %bb.0:
31+
; CHECK-NEXT: #APP
32+
; CHECK-NEXT: r0 = r1
33+
; CHECK-NEXT: exit
34+
; CHECK-EMPTY:
35+
; CHECK-NEXT: #NO_APP
36+
; CHECK-NEXT: .Lfunc_end1:
37+
38+
; CHECK-BTF: [1] FUNC_PROTO '(anon)' ret_type_id=0 vlen=0
39+
; CHECK-BTF: [2] FUNC '__bpf_unreachable' type_id=1 linkage=extern
40+
; CHECK-BTF: [3] DATASEC '.ksyms' size=0 vlen=1
41+
; CHECK-BTF: type_id=2 offset=0 size=0
42+
43+
declare dso_local void @bar()
44+
45+
attributes #0 = { naked }
46+
47+
!llvm.dbg.cu = !{!0}
48+
!llvm.module.flags = !{!2, !3}
49+
50+
!0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, emissionKind: FullDebug)
51+
!1 = !DIFile(filename: "test.c", directory: "/some/dir")
52+
!2 = !{i32 7, !"Dwarf Version", i32 5}
53+
!3 = !{i32 2, !"Debug Info Version", i32 3}

0 commit comments

Comments
 (0)