diff --git a/clang/test/CodeGen/bpf-preserve-attr-index-bitfield-warn.c b/clang/test/CodeGen/bpf-preserve-attr-index-bitfield-warn.c new file mode 100644 --- /dev/null +++ b/clang/test/CodeGen/bpf-preserve-attr-index-bitfield-warn.c @@ -0,0 +1,161 @@ +// Check that diagnostic message is printed when bitfields in a +// structure marked with preserve_access_index are accessed directly. + +// REQUIRES: bpf-registered-target +// RUN: %clang -target bpf -g -S -o /dev/null 2>&1 %s \ +// RUN: | FileCheck %s + +// The below macro definitions are made to be similar to +// tools/lib/bpf/bpf_core_read.h in the Linux Kernel tree +// (but not identical to avoid licensing issues). + +extern void mutate(void* what, unsigned info); +extern int some_int(unsigned info); + +enum info { + OFFSET = 0, + SIZE = 1, + EXISTS = 2, + SIGNED = 3, + LSHIFT = 4, + RSHIFT = 5, +}; + +#define INFO(field, info) __builtin_preserve_field_info(field, info) + +#define BPF_CORE_READ_BITFIELD_PROBED(s, field) ({ \ + unsigned long long val = 0; \ + mutate(&val, INFO((s)->field, SIZE)); \ + val <<= some_int(INFO((s)->field, LSHIFT)); \ + val >> some_int(INFO((s)->field, RSHIFT)); \ + val; }) + +#define BPF_CORE_READ_BITFIELD(s, field) ({ \ + const void *p = (const void *)s + INFO((s)->field, OFFSET); \ + unsigned long long val; \ + val = *(const unsigned long long *)p; \ + val <<= some_int(INFO((s)->field, LSHIFT)); \ + val >> some_int(INFO((s)->field, RSHIFT)); \ + val; }) + +struct plain_struct { + int a:1; + int b; + int c:1; + int d:1; +} __attribute__((preserve_access_index)); + +typedef struct plain_struct plain_struct_alias; + +struct plain_union { + int a:1; + int b; +} __attribute__((preserve_access_index)); + +struct anon_struct { + struct { + int a:1; + int b; + }; +} __attribute__((preserve_access_index)); + +struct anon_union { + union { + int a:1; + int b; + }; +} __attribute__((preserve_access_index)); + +struct nested_struct { + struct plain_struct ps; + struct plain_struct *ps2; +} __attribute__((preserve_access_index)); + +struct plain_struct_no_pai { + int aa:1; + int bb; +}; + +struct nested_struct2 { + struct plain_struct_no_pai ps; +} __attribute__((preserve_access_index)); + +extern void consume_int(int); + +void plain_struct_fn(struct plain_struct *ctx) { + consume_int(ctx->a); consume_int(ctx->c); +// CHECK: {{.*}}.c:[[#@LINE-1]]:{{.*}}: warning: Direct access to a bitfield 'a' +// CHECK: {{.*}}.c:[[#@LINE-2]]:{{.*}}: warning: Direct access to a bitfield 'c' + consume_int(ctx->b); +// CHECK-NOT: {{.*}} 'b' + consume_int(ctx->d); +// CHECK: {{.*}}.c:[[#@LINE-1]]:{{.*}}: warning: Direct access to a bitfield 'd' + consume_int(BPF_CORE_READ_BITFIELD(ctx, a)); + consume_int(BPF_CORE_READ_BITFIELD_PROBED(ctx, a)); +// CHECK-NOT: {{.*}} 'a' +} + +void plain_struct_alias_fn(plain_struct_alias *ctx) { + consume_int(ctx->a); +// CHECK: {{.*}}.c:[[#@LINE-1]]:{{.*}}: warning: Direct access to a bitfield 'a' + consume_int(ctx->b); +// CHECK-NOT: {{.*}} 'b' + consume_int(BPF_CORE_READ_BITFIELD(ctx, a)); + consume_int(BPF_CORE_READ_BITFIELD_PROBED(ctx, a)); +// CHECK-NOT: {{.*}} 'a' +} + +void plain_union_fn(struct plain_union *ctx) { + consume_int(ctx->a); +// CHECK: {{.*}}.c:[[#@LINE-1]]:{{.*}}: warning: Direct access to a bitfield 'a' + consume_int(ctx->b); +// CHECK-NOT: {{.*}} 'b' + consume_int(BPF_CORE_READ_BITFIELD(ctx, a)); + consume_int(BPF_CORE_READ_BITFIELD_PROBED(ctx, a)); +// CHECK-NOT: {{.*}} 'a' +} + +void anon_struct_fn(struct anon_struct *ctx) { + consume_int(ctx->a); +// CHECK: {{.*}}.c:[[#@LINE-1]]:{{.*}}: warning: Direct access to a bitfield 'a' + consume_int(ctx->b); +// CHECK-NOT: {{.*}} 'b' + consume_int(BPF_CORE_READ_BITFIELD(ctx, a)); + consume_int(BPF_CORE_READ_BITFIELD_PROBED(ctx, a)); +// CHECK-NOT: {{.*}} 'a' +} + +void anon_union_fn(struct anon_union *ctx) { + consume_int(ctx->a); +// CHECK: {{.*}}.c:[[#@LINE-1]]:{{.*}}: warning: Direct access to a bitfield 'a' + consume_int(ctx->b); +// CHECK-NOT: {{.*}} 'b' + consume_int(BPF_CORE_READ_BITFIELD(ctx, a)); + consume_int(BPF_CORE_READ_BITFIELD_PROBED(ctx, a)); +// CHECK-NOT: {{.*}} 'a' +} + +void nested_struct_fn(struct nested_struct *ctx) { + consume_int(ctx->ps.b); +// CHECK-NOT: {{.*}} 'b' + consume_int(BPF_CORE_READ_BITFIELD(&ctx->ps, a)); + consume_int(BPF_CORE_READ_BITFIELD_PROBED(&ctx->ps, a)); +// CHECK-NOT: {{.*}} 'a' + consume_int(ctx->ps.a); +// CHECK: {{.*}}.c:[[#@LINE-1]]:{{.*}}: warning: Direct access to a bitfield 'a' + + consume_int(ctx->ps2->a); +// CHECK: {{.*}}.c:[[#@LINE-1]]:{{.*}}: warning: Direct access to a bitfield 'a' + consume_int(ctx->ps2->b); +// CHECK-NOT: {{.*}} 'b' + consume_int(BPF_CORE_READ_BITFIELD(ctx->ps2, a)); + consume_int(BPF_CORE_READ_BITFIELD_PROBED(ctx->ps2, a)); +// CHECK-NOT: {{.*}} 'a' +} + +void nested_struct_fn2(struct nested_struct2 *ctx) { + consume_int(ctx->ps.aa); +// CHECK-NOT: {{.*}} 'aa' + consume_int(ctx->ps.bb); +// CHECK-NOT: {{.*}} 'bb' +} diff --git a/llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp b/llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp --- a/llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp +++ b/llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp @@ -79,6 +79,7 @@ #include "BPFTargetMachine.h" #include "llvm/BinaryFormat/Dwarf.h" #include "llvm/IR/DebugInfoMetadata.h" +#include "llvm/IR/DiagnosticInfo.h" #include "llvm/IR/GlobalVariable.h" #include "llvm/IR/Instruction.h" #include "llvm/IR/Instructions.h" @@ -90,6 +91,7 @@ #include "llvm/IR/Value.h" #include "llvm/Pass.h" #include "llvm/Transforms/Utils/BasicBlockUtils.h" +#include #include #define DEBUG_TYPE "bpf-abstract-member-access" @@ -185,6 +187,8 @@ std::string &AccessKey, bool &IsInt32Ret); uint64_t getConstant(const Value *IndexValue); bool transformGEPChain(CallInst *Call, CallInfo &CInfo); + void doSemanticChecks(); + DIDerivedType *isBitFieldAccess(CallInfo &CInfo); }; std::map BPFAbstractMemberAccess::GEPGlobals; @@ -207,6 +211,36 @@ : FunctionPass(ID), TM(TM) {} }; +// Base class for diagnostic messages output by +// BPFAbstractMemberAccess::doSemanticChecks. In order to report these +// messages in the source location order these have to be deferred. +struct DiagnosticMessage { + const DiagnosticLocation Loc; + DiagnosticMessage(const DiagnosticLocation &Loc) : Loc(Loc) {} + virtual void report(LLVMContext &C) = 0; + virtual ~DiagnosticMessage() {} +}; + +struct SortMsgByLineNumber { + bool operator()(const std::unique_ptr &A, + const std::unique_ptr &B) { + + auto &LocA = A->Loc; + auto &LocB = B->Loc; + // Currently reported for a single function, thus skipping + // file name comparison. + return LocA.getLine() > LocB.getLine() || + (LocA.getLine() == LocB.getLine() && + LocA.getColumn() > LocB.getColumn()); + } +}; + +// A queue of diagnostic messages sorted by location. +using DiagnosticMessageQueue = std::priority_queue + , + std::vector>, + SortMsgByLineNumber>; + } // End anonymous namespace char BPFAbstractMemberAccessLegacyPass::ID = 0; @@ -1199,6 +1233,90 @@ return true; } +namespace { + +class DirectBitfieldAccessMessage : public DiagnosticMessage { + DIDerivedType *DTy; + Instruction *User; +public: + DirectBitfieldAccessMessage(DIDerivedType *DTy, Instruction *User) + : DiagnosticMessage(User->getDebugLoc()), DTy(DTy), User(User) {} + void report(LLVMContext &C) override { + auto Msg = DiagnosticInfoUnsupported + (*User->getFunction(), + Twine("Direct access to a bitfield ") + .concat("'").concat(DTy->getName()).concat("'") + .concat(" of a structure with preserve_access_index attribute " + "is not supported, consider using BPF_CORE_READ_BITFIELD " + "or BPF_CORE_READ_BITFIELD_PROBED macro, see bpf_core_read.h"), + User->getDebugLoc(), + DS_Warning); + C.diagnose(Msg); + } +}; + +} // Anonymous namespace + +DIDerivedType *BPFAbstractMemberAccess::isBitFieldAccess(CallInfo &CI) { + if (CI.Kind != BPFPreserveStructAI) + return nullptr; + if (!CI.Metadata) + return nullptr; + auto *DTy = dyn_cast(CI.Metadata); + if (!DTy) + return nullptr; + auto *CTy = dyn_cast(stripQualifiers(DTy)); + if (!CTy) + return nullptr; + assert(CI.AccessIndex < CTy->getElements().size() && + "isBitFieldAccess: Out of bounds access to CTy->getElements()"); + auto *Elem = CTy->getElements()[CI.AccessIndex]; + auto *ElemTy = dyn_cast(Elem); + if (ElemTy && ElemTy->getFlags() & DINode::DIFlags::FlagBitField) + return ElemTy; + return nullptr; +} + +// So far only one semantic check: bitfield usage. +void BPFAbstractMemberAccess::doSemanticChecks() { + // Iteration over BaseAICalls not necessarily follows source code + // order, so use a priority queue sorting by location to defer the + // diagnostic messages. + DiagnosticMessageQueue MsgQueue; + auto Enqueue= [&](DiagnosticMessage *Msg) { + MsgQueue.push(std::unique_ptr(Msg)); + }; + for (auto &[Call, CInfo] : BaseAICalls) { + // Verify that direct LoadInst is not used for CORE bitfields. + // CORE bitfields should be accessed using BPF_CORE_READ_BITFIELD + // and BPF_CORE_READ_BITFIELD_PROBED macro. + // These macro generate a specific pattern (for a bitfield ctx->field): + // + // BPF_CORE_READ_BITFIELD(ctx, field) is expanded as: + // + // ({ const void *p = (void*) ctx + __builtin_preserve_field_info(...); + // unsigned long long val; + // ... val = *()p; ... + // val; + // }) + // + // Note that ptr->field is not referenced, instead an address is + // computed starting from ctx. + // + // BPF_CORE_READ_BITFIELD_PROBED is a bit different but similar. + if (auto *ElemTy = isBitFieldAccess(CInfo)) + for (auto *User: Call->users()) + if (isa(User)) + Enqueue(new DirectBitfieldAccessMessage + (ElemTy, cast(User))); + } + auto &C = M->getContext(); + while (!MsgQueue.empty()) { + MsgQueue.top()->report(C); + MsgQueue.pop(); + } +} + bool BPFAbstractMemberAccess::doTransformation(Function &F) { bool Transformed = false; @@ -1207,6 +1325,8 @@ // patterns similar to GEP. collectAICallChains(F); + doSemanticChecks(); + for (auto &C : BaseAICalls) Transformed = transformGEPChain(C.first, C.second) || Transformed;