From 8135e49e9b65827e4bbcb8801e327aea29c6dfc2 Mon Sep 17 00:00:00 2001 From: Yonghong Song Date: Thu, 20 Sep 2018 22:24:27 +0000 Subject: [PATCH] bpf: check illegal usage of XADD insn return value Currently, BPF has XADD (locked add) insn support and the asm looks like: lock *(u32 *)(r1 + 0) += r2 lock *(u64 *)(r1 + 0) += r2 The instruction itself does not have a return value. At the source code level, users often use __sync_fetch_and_add() which eventually translates to XADD. The return value of __sync_fetch_and_add() is supposed to be the old value in the xadd memory location. Since BPF::XADD insn does not support such a return value, this patch added a PreEmit phase to check such a usage. If such an illegal usage pattern is detected, a fatal error will be reported like line 4: Invalid usage of the XADD return value if compiled with -g, or Invalid usage of the XADD return value if compiled without -g. Signed-off-by: Yonghong Song Acked-by: Alexei Starovoitov git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@342692 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Target/BPF/BPF.h | 2 + lib/Target/BPF/BPFMIChecking.cpp | 96 +++++++++++++++++++++++++++++++++++++ lib/Target/BPF/BPFTargetMachine.cpp | 1 + lib/Target/BPF/CMakeLists.txt | 1 + test/CodeGen/BPF/xadd.ll | 59 +++++++++++++++++++++++ 5 files changed, 159 insertions(+) create mode 100644 lib/Target/BPF/BPFMIChecking.cpp create mode 100644 test/CodeGen/BPF/xadd.ll diff --git a/lib/Target/BPF/BPF.h b/lib/Target/BPF/BPF.h index 76d3e1ca5f6..9749e369c2c 100644 --- a/lib/Target/BPF/BPF.h +++ b/lib/Target/BPF/BPF.h @@ -19,9 +19,11 @@ class BPFTargetMachine; FunctionPass *createBPFISelDag(BPFTargetMachine &TM); FunctionPass *createBPFMIPeepholePass(); FunctionPass *createBPFMIPreEmitPeepholePass(); +FunctionPass *createBPFMIPreEmitCheckingPass(); void initializeBPFMIPeepholePass(PassRegistry&); void initializeBPFMIPreEmitPeepholePass(PassRegistry&); +void initializeBPFMIPreEmitCheckingPass(PassRegistry&); } #endif diff --git a/lib/Target/BPF/BPFMIChecking.cpp b/lib/Target/BPF/BPFMIChecking.cpp new file mode 100644 index 00000000000..0a311378e77 --- /dev/null +++ b/lib/Target/BPF/BPFMIChecking.cpp @@ -0,0 +1,96 @@ +//===-------------- BPFMIChecking.cpp - MI Checking Legality -------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This pass performs checking to signal errors for certain illegal usages at +// MachineInstruction layer. Specially, the result of XADD{32,64} insn should +// not be used. The pass is done at the PreEmit pass right before the +// machine code is emitted at which point the register liveness information +// is still available. +// +//===----------------------------------------------------------------------===// + +#include "BPF.h" +#include "BPFInstrInfo.h" +#include "BPFTargetMachine.h" +#include "llvm/CodeGen/MachineInstrBuilder.h" +#include "llvm/CodeGen/MachineRegisterInfo.h" + +using namespace llvm; + +#define DEBUG_TYPE "bpf-mi-checking" + +namespace { + +struct BPFMIPreEmitChecking : public MachineFunctionPass { + + static char ID; + MachineFunction *MF; + const TargetRegisterInfo *TRI; + + BPFMIPreEmitChecking() : MachineFunctionPass(ID) { + initializeBPFMIPreEmitCheckingPass(*PassRegistry::getPassRegistry()); + } + +private: + // Initialize class variables. + void initialize(MachineFunction &MFParm); + + void checkingIllegalXADD(void); + +public: + + // Main entry point for this pass. + bool runOnMachineFunction(MachineFunction &MF) override { + if (!skipFunction(MF.getFunction())) { + initialize(MF); + checkingIllegalXADD(); + } + return false; + } +}; + +// Initialize class variables. +void BPFMIPreEmitChecking::initialize(MachineFunction &MFParm) { + MF = &MFParm; + TRI = MF->getSubtarget().getRegisterInfo(); + LLVM_DEBUG(dbgs() << "*** BPF PreEmit checking pass ***\n\n"); +} + +void BPFMIPreEmitChecking::checkingIllegalXADD(void) { + for (MachineBasicBlock &MBB : *MF) { + for (MachineInstr &MI : MBB) { + if (MI.getOpcode() != BPF::XADD32 && MI.getOpcode() != BPF::XADD64) + continue; + + LLVM_DEBUG(MI.dump()); + if (!MI.allDefsAreDead()) { + DebugLoc Empty; + const DebugLoc &DL = MI.getDebugLoc(); + if (DL != Empty) + report_fatal_error("line " + std::to_string(DL.getLine()) + + ": Invalid usage of the XADD return value", false); + else + report_fatal_error("Invalid usage of the XADD return value", false); + } + } + } + + return; +} + +} // end default namespace + +INITIALIZE_PASS(BPFMIPreEmitChecking, "bpf-mi-pemit-checking", + "BPF PreEmit Checking", false, false) + +char BPFMIPreEmitChecking::ID = 0; +FunctionPass* llvm::createBPFMIPreEmitCheckingPass() +{ + return new BPFMIPreEmitChecking(); +} diff --git a/lib/Target/BPF/BPFTargetMachine.cpp b/lib/Target/BPF/BPFTargetMachine.cpp index 84d89bff74f..ffcb1302533 100644 --- a/lib/Target/BPF/BPFTargetMachine.cpp +++ b/lib/Target/BPF/BPFTargetMachine.cpp @@ -115,6 +115,7 @@ void BPFPassConfig::addMachineSSAOptimization() { void BPFPassConfig::addPreEmitPass() { const BPFSubtarget *Subtarget = getBPFTargetMachine().getSubtargetImpl(); + addPass(createBPFMIPreEmitCheckingPass()); if (getOptLevel() != CodeGenOpt::None) if (Subtarget->getHasAlu32() && !DisableMIPeephole) addPass(createBPFMIPreEmitPeepholePass()); diff --git a/lib/Target/BPF/CMakeLists.txt b/lib/Target/BPF/CMakeLists.txt index ee01b4b7b80..5e2ae533ea6 100644 --- a/lib/Target/BPF/CMakeLists.txt +++ b/lib/Target/BPF/CMakeLists.txt @@ -24,6 +24,7 @@ add_llvm_target(BPFCodeGen BPFSubtarget.cpp BPFTargetMachine.cpp BPFMIPeephole.cpp + BPFMIChecking.cpp ) add_subdirectory(AsmParser) diff --git a/test/CodeGen/BPF/xadd.ll b/test/CodeGen/BPF/xadd.ll new file mode 100644 index 00000000000..c5f2620179e --- /dev/null +++ b/test/CodeGen/BPF/xadd.ll @@ -0,0 +1,59 @@ +; RUN: not llc -march=bpfel < %s 2>&1 | FileCheck %s +; RUN: not llc -march=bpfeb < %s 2>&1 | FileCheck %s + +; This file is generated with the source command and source +; $ clang -target bpf -O2 -g -S -emit-llvm t.c +; $ cat t.c +; int test(int *ptr) { +; int r; +; __sync_fetch_and_add(ptr, 4); +; r = __sync_fetch_and_add(ptr, 6); +; return r; +; } + +; ModuleID = 't.c' +source_filename = "t.c" +target datalayout = "e-m:e-p:64:64-i64:64-n32:64-S128" +target triple = "bpf" + +; Function Attrs: nounwind +define dso_local i32 @test(i32* nocapture %ptr) local_unnamed_addr #0 !dbg !7 { +entry: + call void @llvm.dbg.value(metadata i32* %ptr, metadata !13, metadata !DIExpression()), !dbg !15 + %0 = atomicrmw add i32* %ptr, i32 4 seq_cst, !dbg !16 + %1 = atomicrmw add i32* %ptr, i32 6 seq_cst, !dbg !17 +; CHECK: line 4: Invalid usage of the XADD return value + call void @llvm.dbg.value(metadata i32 %1, metadata !14, metadata !DIExpression()), !dbg !18 + ret i32 %1, !dbg !19 +} + +; Function Attrs: nounwind readnone speculatable +declare void @llvm.dbg.value(metadata, metadata, metadata) #1 + +attributes #0 = { nounwind "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" } +attributes #1 = { nounwind readnone speculatable } + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!3, !4, !5} +!llvm.ident = !{!6} + +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 8.0.0 (trunk 342605) (llvm/trunk 342612)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None) +!1 = !DIFile(filename: "t.c", directory: "/home/yhs/work/tests/llvm/sync/test1") +!2 = !{} +!3 = !{i32 2, !"Dwarf Version", i32 4} +!4 = !{i32 2, !"Debug Info Version", i32 3} +!5 = !{i32 1, !"wchar_size", i32 4} +!6 = !{!"clang version 8.0.0 (trunk 342605) (llvm/trunk 342612)"} +!7 = distinct !DISubprogram(name: "test", scope: !1, file: !1, line: 1, type: !8, isLocal: false, isDefinition: true, scopeLine: 1, flags: DIFlagPrototyped, isOptimized: true, unit: !0, retainedNodes: !12) +!8 = !DISubroutineType(types: !9) +!9 = !{!10, !11} +!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) +!11 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !10, size: 64) +!12 = !{!13, !14} +!13 = !DILocalVariable(name: "ptr", arg: 1, scope: !7, file: !1, line: 1, type: !11) +!14 = !DILocalVariable(name: "r", scope: !7, file: !1, line: 2, type: !10) +!15 = !DILocation(line: 1, column: 15, scope: !7) +!16 = !DILocation(line: 3, column: 4, scope: !7) +!17 = !DILocation(line: 4, column: 8, scope: !7) +!18 = !DILocation(line: 2, column: 8, scope: !7) +!19 = !DILocation(line: 5, column: 4, scope: !7) -- 2.11.0