https://github.com/yonghong-song updated https://github.com/llvm/llvm-project/pull/232498
>From 0c660ef3329d73911c1be19f5d640733ea962051 Mon Sep 17 00:00:00 2001 From: Yonghong Song <[email protected]> Date: Mon, 19 Oct 2026 17:40:45 -0700 Subject: [PATCH] [BPF] Support preserve_access_index attribute for enum Currently, an enum value can only be relocated with the form __builtin_preserve_enum_value(*(enum E *)C, BPF_ENUMVAL_VALUE) or the libbpf macro bpf_core_enum_value(enum E, C). It is easy to forget to use it, and since most enum values are stable, a missing relocation is often not noticed until the program runs on a kernel with a different value. This patch allows the preserve_access_index attribute on enum declarations, in addition to struct and union declarations. If -g is specified, each use of an enumerator of such an enum whose value is computed at runtime becomes an ENUM_VALUE relocation, without any builtin. For example: enum cgroup_subsys_id { cpuset_cgrp_id = 0, cpu_cgrp_id = 1, memory_cgrp_id = 4, CGROUP_SUBSYS_COUNT = 10, } __attribute__((preserve_access_index)); struct css_set { void *subsys[CGROUP_SUBSYS_COUNT]; } __attribute__((preserve_access_index)); int is_valid(int id) { return id >= 0 && id < CGROUP_SUBSYS_COUNT; } void *get_mem_css(struct css_set *cset) { return cset->subsys[memory_cgrp_id]; } The generated code is: is_valid: w0 = 0 if w1 s< 0 goto +5 r2 = 10 ll CO-RE <enumval_value> enum cgroup_subsys_id::CGROUP_SUBSYS_COUNT = 10 w0 = 1 if w1 s< w2 goto +1 w0 = 0 exit get_mem_css: r2 = 0 CO-RE <byte_off> struct css_set::subsys (0:0) r1 += r2 r2 = 4 ll CO-RE <enumval_value> enum cgroup_subsys_id::memory_cgrp_id = 4 r2 <<= 32 r2 s>>= 29 r1 += r2 r0 = *(u64 *)(r1 + 0) exit Without the attribute on the enum, memory_cgrp_id is folded into the struct relocation, so the array index 4 is fixed: get_mem_css: r0 = *(u64 *)(r1 + 32) CO-RE <byte_off> struct css_set::subsys[4] (0:0:4) exit The attribute takes effect for - a named enum, e.g., enum cgroup_subsys_id above, - an anonymous enum with a typedef name, e.g., typedef enum { A = 1 } __attribute__((preserve_access_index)) T; - a named enum defined inside a struct or union with the attribute, which inherits the attribute as nested records do. An anonymous enum without a typedef name is not supported since libbpf matches relocations by type name. The attribute has no effect on it. If the attribute is written directly on such an enum, an error is issued by default, consistent with other misuses of the attribute: enum { A1 } __attribute__((preserve_access_index)); error: 'preserve_access_index' attribute has no effect on an anonymous enum without a typedef name [-Wignored-attributes] It can be downgraded to a warning with -Wno-error=ignored-attributes. There is no diagnostic if such an enum gets the attribute implicitly from an enclosing struct or union, or from #pragma clang attribute, so existing annotated records and headers with many anonymous enums like vmlinux.h are not affected. An enumerator is relocated where its value is used at runtime, e.g., - in expressions, such as return values, function arguments, array indices and comparisons, - in conditions of if/switch/loops/?:/&&/|| statements and expressions, - in builtin calls which can be constant evaluated, - in initializers of local variables, including local aggregates, - through a local const variable or its members initialized with such an enumerator, e.g., k in const int k = memory_cgrp_id; if (k == 4) ... An enumerator is an integer constant expression, so clang codegen may fold an expression containing it before the enumerator itself is emitted. Such folding is disabled in the above cases. An enumerator keeps its compile-time value - where a compile-time constant is required, e.g., case labels, array sizes like subsys[CGROUP_SUBSYS_COUNT] above, bitfield widths, static assertions and inline asm "i" operands, - in initializers of global and static variables, including uses of a global or static const variable initialized with an enumerator, - in unevaluated operands, e.g., sizeof(memory_cgrp_id), - if -g is not specified. The enumerators are lowered to the same llvm.bpf.preserve.enum.value intrinsic as __builtin_preserve_enum_value(), so no backend or libbpf change is needed. __builtin_preserve_enum_value() works as before. Its intrinsic lowering is moved into a helper which is shared with the new case. Existing code is mostly not affected since bpftool-generated vmlinux.h only applies the attribute to records and never defines a named enum inside a record. To relocate enum values from vmlinux.h, bpftool can apply the attribute to enums as well. Older clang rejects the attribute on enums with a hard error, which cannot be downgraded with any -W option, either directly: error: 'preserve_access_index' attribute only applies to structs, unions, and classes or through #pragma clang attribute: error: attribute 'preserve_access_index' cannot be applied to 'enum' Checking __clang_major__ is not reliable, since a development build of the same major version prior to this patch still rejects the attribute on enums. So a new macro __BPF_FEATURE_ENUM_PRESERVE_ACCESS_INDEX is defined, and vmlinux.h can apply the attribute to enums only if it is supported: #ifndef BPF_NO_PRESERVE_ACCESS_INDEX #ifdef __BPF_FEATURE_ENUM_PRESERVE_ACCESS_INDEX #pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = any(record, enum)) #else #pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record) #endif #endif --- clang/include/clang/Basic/Attr.td | 2 +- clang/include/clang/Basic/AttrDocs.td | 14 ++ .../clang/Basic/DiagnosticSemaKinds.td | 3 + clang/include/clang/Sema/SemaBPF.h | 6 + clang/lib/Basic/Targets/BPF.cpp | 1 + clang/lib/CodeGen/CGBuiltin.cpp | 2 +- clang/lib/CodeGen/CGDecl.cpp | 3 +- clang/lib/CodeGen/CGExprAgg.cpp | 3 +- clang/lib/CodeGen/CGExprScalar.cpp | 12 +- clang/lib/CodeGen/CodeGenFunction.cpp | 54 +++++ clang/lib/CodeGen/CodeGenFunction.h | 14 ++ clang/lib/CodeGen/TargetBuiltins/ARM.cpp | 74 ++++--- clang/lib/Sema/Sema.cpp | 1 + clang/lib/Sema/SemaBPF.cpp | 23 ++- .../bpf-attr-preserve-access-index-enum.c | 193 ++++++++++++++++++ ...a-attribute-supported-attributes-list.test | 2 +- .../test/Preprocessor/bpf-predefined-macros.c | 8 + .../Sema/bpf-attr-preserve-access-index.c | 37 +++- 18 files changed, 410 insertions(+), 42 deletions(-) create mode 100644 clang/test/CodeGen/bpf-attr-preserve-access-index-enum.c diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index f8bd097733529..b7eccfa414dc6 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -2576,7 +2576,7 @@ def AMDGPUAvailableVisible : StmtAttr, TargetSpecificAttr<TargetAMDGPU> { def BPFPreserveAccessIndex : InheritableAttr, TargetSpecificAttr<TargetBPF> { let Spellings = [Clang<"preserve_access_index">]; - let Subjects = SubjectList<[Record], ErrorDiag>; + let Subjects = SubjectList<[Record, Enum], ErrorDiag>; let Documentation = [BPFPreserveAccessIndexDocs]; let LangOpts = [COnly]; } diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index e8ce3c3dea7f3..40c4030741e19 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -3073,6 +3073,20 @@ attribute for the BPF target. This attribute may be attached to a struct or union declaration, where if -g is specified, it enables preserving struct or union member access debuginfo indices of this struct or union, similar to clang `__builtin_preserve_access_index()`. + +The attribute may also be attached to an enum declaration. If -g is +specified, each use of an enumerator of this enum whose value is computed +at runtime is replaced by an `ENUM_VALUE` CO-RE relocation, similar to +`__builtin_preserve_enum_value(*(enum_type *)enumerator, 1)`, so that the +bpf loader may adjust the value based on the target kernel. Such uses are +not constant folded. Uses that require a compile-time constant, e.g., +case labels, array sizes, bitfield widths, static assertions and global +or static variable initializers, keep the compile-time value. The enum +must be named or have a typedef name; otherwise, the attribute has no +effect and an error is issued, which may be downgraded to a warning with +`-Wno-error=ignored-attributes`. The macro +`__BPF_FEATURE_ENUM_PRESERVE_ACCESS_INDEX` is defined if the attribute is +supported on enum declarations. }]; } diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 3e8ba5bf88a42..a8eb081cd207b 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -13545,6 +13545,9 @@ def err_preserve_enum_value_invalid : Error< "__builtin_preserve_enum_value argument %0 invalid">; def err_preserve_enum_value_not_const: Error< "__builtin_preserve_enum_value argument %0 not a constant">; +def warn_preserve_access_index_anon_enum : Warning< + "%0 attribute has no effect on an anonymous enum without a typedef name">, + InGroup<IgnoredAttributes>, DefaultError; def err_bit_cast_non_trivially_copyable : Error< "'__builtin_bit_cast' %select{source|destination}0 type must be trivially copyable">; diff --git a/clang/include/clang/Sema/SemaBPF.h b/clang/include/clang/Sema/SemaBPF.h index d7577aea70122..dd2644ba5619c 100644 --- a/clang/include/clang/Sema/SemaBPF.h +++ b/clang/include/clang/Sema/SemaBPF.h @@ -15,6 +15,7 @@ #include "clang/AST/ASTFwd.h" #include "clang/Sema/SemaBase.h" +#include "llvm/ADT/MapVector.h" namespace clang { class ParsedAttr; @@ -27,6 +28,11 @@ class SemaBPF : public SemaBase { void handlePreserveAIRecord(RecordDecl *RD); void handlePreserveAccessIndexAttr(Decl *D, const ParsedAttr &AL); + void DiagnoseAnonymousPreserveAIEnums(); + +private: + llvm::MapVector<EnumDecl *, const BPFPreserveAccessIndexAttr *> + AnonPreserveAIEnums; }; } // namespace clang diff --git a/clang/lib/Basic/Targets/BPF.cpp b/clang/lib/Basic/Targets/BPF.cpp index 100769ea4cdb1..f5eb215190265 100644 --- a/clang/lib/Basic/Targets/BPF.cpp +++ b/clang/lib/Basic/Targets/BPF.cpp @@ -47,6 +47,7 @@ void BPFTargetInfo::getTargetDefines(const LangOptions &Opts, Builder.defineMacro("__BPF_FEATURE_MAY_GOTO"); Builder.defineMacro("__BPF_FEATURE_ATOMIC_MEM_ORDERING"); Builder.defineMacro("__BPF_FEATURE_STACK_ARGUMENT"); + Builder.defineMacro("__BPF_FEATURE_ENUM_PRESERVE_ACCESS_INDEX"); if (CPU.empty()) CPU = "v3"; diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp index c93375ff3a18e..dd54a83b03404 100644 --- a/clang/lib/CodeGen/CGBuiltin.cpp +++ b/clang/lib/CodeGen/CGBuiltin.cpp @@ -2725,7 +2725,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, // TODO: Extend this handling to all builtin calls that we can constant-fold. Expr::EvalResult Result; if (E->isPRValue() && E->EvaluateAsRValue(Result, CGM.getContext()) && - !Result.hasSideEffects()) { + !Result.hasSideEffects() && !containsBPFPreservedEnumerator(E)) { if (Result.Val.isInt()) return RValue::get(llvm::ConstantInt::get(getLLVMContext(), Result.Val.getInt())); diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp index 8862f57301412..98c26b1ae80ac 100644 --- a/clang/lib/CodeGen/CGDecl.cpp +++ b/clang/lib/CodeGen/CGDecl.cpp @@ -1538,7 +1538,8 @@ CodeGenFunction::EmitAutoVarAlloca(const VarDecl &D) { if (D.getInit() && (Ty->isArrayType() || Ty->isRecordType()) && (D.isConstexpr() || ((Ty.isPODType(getContext()) || - getContext().getBaseElementType(Ty)->isObjCObjectPointerType())))) { + getContext().getBaseElementType(Ty)->isObjCObjectPointerType()))) && + !containsBPFPreservedEnumerator(D.getInit())) { emission.ConstantAggregateInitializer = ConstantEmitter(*this).tryEmitAbstractForInitializer(D); if (emission.ConstantAggregateInitializer) { diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp index 73c6301c5d585..963229dee9506 100644 --- a/clang/lib/CodeGen/CGExprAgg.cpp +++ b/clang/lib/CodeGen/CGExprAgg.cpp @@ -581,7 +581,8 @@ void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType, // more efficient than per-element initialization, the size of the elements // with explicit initializers should be large enough. if (NumInitElements * elementSize.getQuantity() > 16 && - elementType.isTriviallyCopyableType(CGF.getContext())) { + elementType.isTriviallyCopyableType(CGF.getContext()) && + !CGF.containsBPFPreservedEnumerator(ExprToVisit)) { CodeGen::CodeGenModule &CGM = CGF.CGM; ConstantEmitter Emitter(CGF); QualType GVArrayQTy = CGM.getContext().getAddrSpaceQualType( diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index f1394fc524645..2eca122b1cac8 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -608,8 +608,11 @@ class ScalarExprEmitter // l-values. Value *VisitDeclRefExpr(DeclRefExpr *E) { - if (CodeGenFunction::ConstantEmission Constant = CGF.tryEmitAsConstant(E)) - return CGF.emitScalarConstant(Constant, E); + if (Value *V = CGF.EmitBPFPreservedEnumerator(E)) + return V; + if (!CGF.containsBPFPreservedEnumerator(E)) + if (CodeGenFunction::ConstantEmission Constant = CGF.tryEmitAsConstant(E)) + return CGF.emitScalarConstant(Constant, E); return EmitLoadOfLValue(E); } @@ -2144,7 +2147,10 @@ Value *ScalarExprEmitter::VisitConvertVectorExpr(ConvertVectorExpr *E) { } Value *ScalarExprEmitter::VisitMemberExpr(MemberExpr *E) { - if (CodeGenFunction::ConstantEmission Constant = CGF.tryEmitAsConstant(E)) { + if (CGF.containsBPFPreservedEnumerator(E)) { + // Do not constant fold. + } else if (CodeGenFunction::ConstantEmission Constant = + CGF.tryEmitAsConstant(E)) { CGF.EmitIgnoredExpr(E->getBase()); return CGF.emitScalarConstant(Constant, E); } else { diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index e5bca359e6a85..dcef9e8405567 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -1780,6 +1780,57 @@ bool CodeGenFunction::mightAddDeclToScope(const Stmt *S) { return false; } +QualType CodeGenFunction::getBPFPreservedEnumType(const DeclRefExpr *E) { + const auto *Enumerator = dyn_cast<EnumConstantDecl>(E->getDecl()); + if (!Enumerator || !getTarget().getTriple().isBPF() || !getDebugInfo()) + return QualType(); + + // In C, the type of an enumerator may not be the enum type, so get the + // enum from the declaration context. + const auto *ED = dyn_cast<EnumDecl>(Enumerator->getDeclContext()); + if (!ED || !ED->hasAttr<BPFPreserveAccessIndexAttr>()) + return QualType(); + + // The relocation is matched by type name, so an anonymous enum is + // only supported through its typedef name. + if (ED->getDeclName()) + return getContext().getCanonicalTagType(ED); + if (const TypedefNameDecl *TD = ED->getTypedefNameForAnonDecl()) + return getContext().getTypeDeclType(TD); + return QualType(); +} + +bool CodeGenFunction::containsBPFPreservedEnumerator(const Expr *E) { + if (!getTarget().getTriple().isBPF() || !getDebugInfo()) + return false; + + llvm::SmallPtrSet<const VarDecl *, 4> Visited; + auto Contains = [this, &Visited](auto &&Self, const Stmt *S) -> bool { + if (!S) + return false; + // A ConstantExpr is always emitted as a constant, and the enumerators + // in an unevaluated operand do not contribute their values. + if (isa<ConstantExpr, UnaryExprOrTypeTraitExpr>(S)) + return false; + if (const auto *DRE = dyn_cast<DeclRefExpr>(S)) { + if (!getBPFPreservedEnumType(DRE).isNull()) + return true; + // A local const variable may be constant folded through its + // initializer. + const auto *VD = dyn_cast<VarDecl>(DRE->getDecl()); + if (VD && VD->hasLocalStorage() && VD->getType().isConstQualified() && + Visited.insert(VD).second) + return Self(Self, VD->getInit()); + return false; + } + for (const Stmt *SubStmt : S->children()) + if (Self(Self, SubStmt)) + return true; + return false; + }; + return Contains(Contains, E); +} + /// ConstantFoldsToSimpleInteger - If the specified expression does not fold /// to a constant, or if it does but contains a label, return false. If it /// constant folds return true and set the boolean result in Result. @@ -1813,6 +1864,9 @@ bool CodeGenFunction::ConstantFoldsToSimpleInteger(const Expr *Cond, if (!Cond->EvaluateAsInt(Result, getContext())) return false; // Not foldable, not integer or not fully evaluatable. + if (containsBPFPreservedEnumerator(Cond)) + return false; + llvm::APSInt Int = Result.Val.getInt(); if (!AllowLabels && CodeGenFunction::ContainsLabel(Cond)) return false; // Contains a label. diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 73090c03484b7..279103f9b3501 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -5041,6 +5041,10 @@ class CodeGenFunction : public CodeGenTypeCache { llvm::Value *EmitAArch64BuiltinExpr(unsigned BuiltinID, const CallExpr *E, llvm::Triple::ArchType Arch); llvm::Value *EmitBPFBuiltinExpr(unsigned BuiltinID, const CallExpr *E); + llvm::Value *EmitBPFPreserveEnumValue(const EnumConstantDecl *Enumerator, + QualType EnumTy, SourceLocation Loc, + uint64_t Flag); + llvm::Value *EmitBPFPreservedEnumerator(const DeclRefExpr *E); llvm::Value *BuildVector(ArrayRef<llvm::Value *> Ops); llvm::Value *EmitX86BuiltinExpr(unsigned BuiltinID, const CallExpr *E); @@ -5372,6 +5376,16 @@ class CodeGenFunction : public CodeGenTypeCache { /// current scope, by being a (possibly-labelled) DeclStmt. static bool mightAddDeclToScope(const Stmt *S); + /// For the BPF target, if E refers to an enumerator of an enum with the + /// preserve_access_index attribute, return the enum type to be used for + /// its CO-RE relocation. Otherwise, return a null type. + QualType getBPFPreservedEnumType(const DeclRefExpr *E); + + /// For the BPF target, return true if the expression contains an + /// enumerator to be relocated, directly or through the initializer of a + /// local const variable. Such an expression should not be constant folded. + bool containsBPFPreservedEnumerator(const Expr *E); + /// ConstantFoldsToSimpleInteger - If the specified expression does not fold /// to a constant, or if it does but contains a label, return false. If it /// constant folds return true and set the boolean result in Result. diff --git a/clang/lib/CodeGen/TargetBuiltins/ARM.cpp b/clang/lib/CodeGen/TargetBuiltins/ARM.cpp index 4a7bdba6d917e..0cb239c6da3bd 100644 --- a/clang/lib/CodeGen/TargetBuiltins/ARM.cpp +++ b/clang/lib/CodeGen/TargetBuiltins/ARM.cpp @@ -7287,6 +7287,51 @@ Value *CodeGenFunction::EmitAArch64BuiltinExpr(unsigned BuiltinID, } } +// A sequence number, injected into IR builtin functions, to +// prevent CSE given the only difference of the function +// may just be the debuginfo metadata. +static uint32_t BPFBuiltinSeqNum; + +Value * +CodeGenFunction::EmitBPFPreserveEnumValue(const EnumConstantDecl *Enumerator, + QualType EnumTy, SourceLocation Loc, + uint64_t Flag) { + llvm::DIType *DbgInfo = + getDebugInfo()->getOrCreateStandaloneType(EnumTy, Loc); + + auto InitVal = Enumerator->getInitVal(); + std::string InitValStr; + if (InitVal.isNegative() || InitVal > uint64_t(INT64_MAX)) + InitValStr = std::to_string(InitVal.getSExtValue()); + else + InitValStr = std::to_string(InitVal.getZExtValue()); + std::string EnumStr = Enumerator->getNameAsString() + ":" + InitValStr; + Value *EnumStrVal = Builder.CreateGlobalString(EnumStr); + + Value *FlagValue = ConstantInt::get(Int64Ty, Flag); + Value *SeqNumVal = ConstantInt::get(Int32Ty, BPFBuiltinSeqNum++); + + llvm::Function *IntrinsicFn = Intrinsic::getOrInsertDeclaration( + &CGM.getModule(), Intrinsic::bpf_preserve_enum_value, {}); + CallInst *Fn = + Builder.CreateCall(IntrinsicFn, {SeqNumVal, EnumStrVal, FlagValue}); + Fn->setMetadata(LLVMContext::MD_preserve_access_index, DbgInfo); + return Fn; +} + +Value *CodeGenFunction::EmitBPFPreservedEnumerator(const DeclRefExpr *E) { + QualType EnumTy = getBPFPreservedEnumType(E); + if (EnumTy.isNull()) + return nullptr; + + // Flag 1 is ENUM_VALUE. + Value *V = EmitBPFPreserveEnumValue(cast<EnumConstantDecl>(E->getDecl()), + EnumTy, E->getExprLoc(), 1); + QualType Ty = E->getType(); + return Builder.CreateIntCast(V, ConvertType(Ty), + Ty->isSignedIntegerOrEnumerationType()); +} + Value *CodeGenFunction::EmitBPFBuiltinExpr(unsigned BuiltinID, const CallExpr *E) { assert((BuiltinID == BPF::BI__builtin_preserve_field_info || @@ -7295,11 +7340,6 @@ Value *CodeGenFunction::EmitBPFBuiltinExpr(unsigned BuiltinID, BuiltinID == BPF::BI__builtin_preserve_enum_value) && "unexpected BPF builtin"); - // A sequence number, injected into IR builtin functions, to - // prevent CSE given the only difference of the function - // may just be the debuginfo metadata. - static uint32_t BuiltinSeqNum; - switch (BuiltinID) { default: llvm_unreachable("Unexpected BPF builtin"); @@ -7343,7 +7383,7 @@ Value *CodeGenFunction::EmitBPFBuiltinExpr(unsigned BuiltinID, ConstantInt *Flag = cast<ConstantInt>(EmitScalarExpr(E->getArg(1))); Value *FlagValue = ConstantInt::get(Int64Ty, Flag->getSExtValue()); - Value *SeqNumVal = ConstantInt::get(Int32Ty, BuiltinSeqNum++); + Value *SeqNumVal = ConstantInt::get(Int32Ty, BPFBuiltinSeqNum++); llvm::Function *FnDecl; if (BuiltinID == BPF::BI__builtin_btf_type_id) @@ -7363,8 +7403,6 @@ Value *CodeGenFunction::EmitBPFBuiltinExpr(unsigned BuiltinID, } const Expr *Arg0 = E->getArg(0); - llvm::DIType *DbgInfo = getDebugInfo()->getOrCreateStandaloneType( - Arg0->getType(), Arg0->getExprLoc()); // Find enumerator const auto *UO = cast<UnaryOperator>(Arg0->IgnoreParens()); @@ -7372,25 +7410,9 @@ Value *CodeGenFunction::EmitBPFBuiltinExpr(unsigned BuiltinID, const auto *DR = cast<DeclRefExpr>(CE->getSubExpr()); const auto *Enumerator = cast<EnumConstantDecl>(DR->getDecl()); - auto InitVal = Enumerator->getInitVal(); - std::string InitValStr; - if (InitVal.isNegative() || InitVal > uint64_t(INT64_MAX)) - InitValStr = std::to_string(InitVal.getSExtValue()); - else - InitValStr = std::to_string(InitVal.getZExtValue()); - std::string EnumStr = Enumerator->getNameAsString() + ":" + InitValStr; - Value *EnumStrVal = Builder.CreateGlobalString(EnumStr); - ConstantInt *Flag = cast<ConstantInt>(EmitScalarExpr(E->getArg(1))); - Value *FlagValue = ConstantInt::get(Int64Ty, Flag->getSExtValue()); - Value *SeqNumVal = ConstantInt::get(Int32Ty, BuiltinSeqNum++); - - llvm::Function *IntrinsicFn = Intrinsic::getOrInsertDeclaration( - &CGM.getModule(), Intrinsic::bpf_preserve_enum_value, {}); - CallInst *Fn = - Builder.CreateCall(IntrinsicFn, {SeqNumVal, EnumStrVal, FlagValue}); - Fn->setMetadata(LLVMContext::MD_preserve_access_index, DbgInfo); - return Fn; + return EmitBPFPreserveEnumValue(Enumerator, Arg0->getType(), + Arg0->getExprLoc(), Flag->getSExtValue()); } } } diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index bae41a6e6f5cf..935930ada44fd 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -1335,6 +1335,7 @@ void Sema::ActOnEndOfTranslationUnit() { DiagnoseUnterminatedPragmaAlignPack(); DiagnoseUnterminatedPragmaAttribute(); + BPF().DiagnoseAnonymousPreserveAIEnums(); OpenMP().DiagnoseUnterminatedOpenMPDeclareTarget(); DiagnosePrecisionLossInComplexDivision(); DiagnoseUnusedAPINotesSelectors(); diff --git a/clang/lib/Sema/SemaBPF.cpp b/clang/lib/Sema/SemaBPF.cpp index a9761764bbdc6..8f7183ab73aa6 100644 --- a/clang/lib/Sema/SemaBPF.cpp +++ b/clang/lib/Sema/SemaBPF.cpp @@ -179,10 +179,25 @@ void SemaBPF::handlePreserveAIRecord(RecordDecl *RD) { } void SemaBPF::handlePreserveAccessIndexAttr(Decl *D, const ParsedAttr &AL) { - auto *Rec = cast<RecordDecl>(D); - handlePreserveAIRecord(Rec); - Rec->addAttr(::new (getASTContext()) - BPFPreserveAccessIndexAttr(getASTContext(), AL)); + if (auto *Rec = dyn_cast<RecordDecl>(D)) + handlePreserveAIRecord(Rec); + auto *A = + ::new (getASTContext()) BPFPreserveAccessIndexAttr(getASTContext(), AL); + // The relocation of an enumerator is matched by type name. Whether an + // anonymous enum gets a typedef name is checked at the end of the + // translation unit. Skip #pragma clang attribute which may cover many + // anonymous enums in a header. + if (auto *ED = dyn_cast<EnumDecl>(D); + ED && !ED->getDeclName() && !AL.isPragmaClangAttribute()) + AnonPreserveAIEnums.insert({ED, A}); + D->addAttr(A); +} + +void SemaBPF::DiagnoseAnonymousPreserveAIEnums() { + for (auto [ED, A] : AnonPreserveAIEnums) + if (!ED->getTypedefNameForAnonDecl()) + Diag(A->getLocation(), diag::warn_preserve_access_index_anon_enum) << A; + AnonPreserveAIEnums.clear(); } } // namespace clang diff --git a/clang/test/CodeGen/bpf-attr-preserve-access-index-enum.c b/clang/test/CodeGen/bpf-attr-preserve-access-index-enum.c new file mode 100644 index 0000000000000..d906b101e5f43 --- /dev/null +++ b/clang/test/CodeGen/bpf-attr-preserve-access-index-enum.c @@ -0,0 +1,193 @@ +// REQUIRES: bpf-registered-target +// RUN: %clang_cc1 -triple bpf -emit-llvm -debug-info-kind=limited -disable-llvm-passes \ +// RUN: -Wno-error=ignored-attributes -verify %s -o - | FileCheck %s + +#define __reloc__ __attribute__((preserve_access_index)) + +enum AA { VAL1 = -100, VAL2 = 0xffff8000 }; +typedef enum { VAL10 = 0xffffFFFF80000000 } __BB; +enum e1 { E1_A = 1, E1_C = 10 } __reloc__; +typedef enum { E2_A = 3 } __reloc__ e2; +enum { E3_A = 4 } __reloc__; // expected-warning {{attribute has no effect on an anonymous enum without a typedef name}} +struct s { + enum e4 { E4_A = 5 } a; +} __reloc__; +enum s64 { S64_BIG = 0x100000000, S64_NEG = -5 } __reloc__; +enum u64 { U64_BIG = 0xffffffff80000000 } __reloc__; + +// Contexts requiring a constant keep the compile-time value. +int g1 = E1_C; +int g2[E1_C]; +_Static_assert(E1_A == 1, ""); + +// CHECK: @g1 = global i32 10 +// CHECK: @[[VAL1:[0-9]+]] = private unnamed_addr constant [10 x i8] c"VAL1:-100\00" +// CHECK: @[[VAL2:[0-9]+]] = private unnamed_addr constant [16 x i8] c"VAL2:4294934528\00" +// CHECK: @[[VAL10:[0-9]+]] = private unnamed_addr constant [18 x i8] c"VAL10:-2147483648\00" +// CHECK: @[[E1_A:[0-9]+]] = private unnamed_addr constant [7 x i8] c"E1_A:1\00" +// CHECK: @[[E1_C:[0-9]+]] = private unnamed_addr constant [8 x i8] c"E1_C:10\00" +// CHECK: @[[E2_A:[0-9]+]] = private unnamed_addr constant [7 x i8] c"E2_A:3\00" +// CHECK: @[[E4_A:[0-9]+]] = private unnamed_addr constant [7 x i8] c"E4_A:5\00" +// CHECK: @[[S64_BIG:[0-9]+]] = private unnamed_addr constant [19 x i8] c"S64_BIG:4294967296\00" +// CHECK: @[[S64_NEG:[0-9]+]] = private unnamed_addr constant [11 x i8] c"S64_NEG:-5\00" +// CHECK: @[[U64_BIG:[0-9]+]] = private unnamed_addr constant [20 x i8] c"U64_BIG:-2147483648\00" +// CHECK: @g2 = global [10 x i32] zeroinitializer + +// __builtin_preserve_enum_value() is not affected by the attribute. +int test1(void) { + return __builtin_preserve_enum_value(*(enum AA *)VAL1, 0) + + __builtin_preserve_enum_value(*(enum AA *)VAL2, 1) + + __builtin_preserve_enum_value(*(__BB *)VAL10, 1) + + __builtin_preserve_enum_value(*(enum e1 *)E1_A, 0); +} + +// CHECK: define dso_local i32 @test1 +// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 0, ptr @[[VAL1]], i64 0), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[AA:[0-9]+]] +// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 1, ptr @[[VAL2]], i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[AA]] +// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 2, ptr @[[VAL10]], i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[BB:[0-9]+]] +// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 3, ptr @[[E1_A]], i64 0), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[E1:[0-9]+]] +// CHECK-NOT: @llvm.bpf.preserve.enum.value +// CHECK: ret i32 + +// An enumerator of an attributed enum is an ENUM_VALUE relocation, with or +// without __builtin_preserve_access_index(). +int test2(int x) { + return (x == E1_C) + __builtin_preserve_access_index(E2_A) + (E4_A); +} + +// CHECK: define dso_local i32 @test2 +// CHECK: [[V:%[0-9a-z]+]] = call i64 @llvm.bpf.preserve.enum.value(i32 {{[0-9]+}}, ptr @[[E1_C]], i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[E1]] +// CHECK: [[T:%[0-9a-z]+]] = trunc i64 [[V]] to i32 +// CHECK: icmp eq i32 %{{[0-9a-z]+}}, [[T]] +// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 {{[0-9]+}}, ptr @[[E2_A]], i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[E2:[0-9]+]] +// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 {{[0-9]+}}, ptr @[[E4_A]], i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[E4:[0-9]+]] + +// Conditions and builtin calls using the enumerators are not folded. +int test3(int x) { + if (E1_C == 10) + x++; + while (x < E1_C) + x += E1_C > 5 ? 2 : 3; + switch (E1_A) { + case E1_A: // Case labels keep the compile-time value. + x++; + break; + default: + break; + } + return x + __builtin_bswap32(E1_C); +} + +// CHECK: define dso_local i32 @test3 +// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 {{[0-9]+}}, ptr @{{[0-9]+}}, i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[E1]] +// CHECK: icmp eq i32 %{{[0-9a-z]+}}, 10 +// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 {{[0-9]+}}, ptr @{{[0-9]+}}, i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[E1]] +// CHECK: icmp slt i32 +// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 {{[0-9]+}}, ptr @{{[0-9]+}}, i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[E1]] +// CHECK: icmp sgt i32 %{{[0-9a-z]+}}, 5 +// CHECK: select i1 +// CHECK: [[V:%[0-9a-z]+]] = call i64 @llvm.bpf.preserve.enum.value(i32 {{[0-9]+}}, ptr @{{[0-9]+}}, i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[E1]] +// CHECK: [[T:%[0-9a-z]+]] = trunc i64 [[V]] to i32 +// CHECK: switch i32 [[T]], label %{{.*}} [ +// CHECK-NEXT: i32 1, label +// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 {{[0-9]+}}, ptr @{{[0-9]+}}, i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[E1]] +// CHECK: call i32 @llvm.bswap.i32 + +// Local aggregates initialized with the enumerators are not constant +// initialized. +struct key { int a; int b; }; +void use(void *); +void test4(void) { + struct key k = { E1_A, 2 }; + long arr[4] = { 1, 2, 3, E1_C }; + use(&k); + use(arr); +} + +// CHECK: define dso_local void @test4 +// CHECK-NOT: memcpy +// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 {{[0-9]+}}, ptr @{{[0-9]+}}, i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[E1]] +// CHECK-NOT: memcpy +// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 {{[0-9]+}}, ptr @{{[0-9]+}}, i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[E1]] +// CHECK-NOT: memcpy +// CHECK: ret void + +// Enumerators of an enum without the attribute, of an anonymous enum without +// typedef name, and in unevaluated operands keep their compile-time value. +int test5(void) { + static int s = E1_C; + return s + VAL1 + E3_A + sizeof(E1_C); +} + +// CHECK: define dso_local i32 @test5 +// CHECK-NOT: @llvm.bpf.preserve.enum.value +// CHECK: add nsw i64 %{{[0-9a-z]+}}, -100 +// CHECK-NEXT: add nsw i64 %{{[0-9a-z]+}}, 4 +// CHECK-NEXT: add i64 %{{[0-9a-z]+}}, 4 +// CHECK-NOT: @llvm.bpf.preserve.enum.value +// CHECK: ret i32 + +// Enumerators with 64-bit values are relocated without truncation. +long test6(void) { + return S64_BIG + S64_NEG; +} + +// CHECK: define dso_local i64 @test6 +// CHECK: [[V1:%[0-9a-z]+]] = call i64 @llvm.bpf.preserve.enum.value(i32 {{[0-9]+}}, ptr @[[S64_BIG]], i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[S64:[0-9]+]] +// CHECK: [[V2:%[0-9a-z]+]] = call i64 @llvm.bpf.preserve.enum.value(i32 {{[0-9]+}}, ptr @[[S64_NEG]], i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[S64]] +// CHECK: [[ADD:%[0-9a-z]+]] = add nsw i64 [[V1]], [[V2]] +// CHECK: ret i64 [[ADD]] + +unsigned long test7(void) { + return U64_BIG; +} + +// CHECK: define dso_local i64 @test7 +// CHECK: [[V:%[0-9a-z]+]] = call i64 @llvm.bpf.preserve.enum.value(i32 {{[0-9]+}}, ptr @[[U64_BIG]], i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[U64:[0-9]+]] +// CHECK: ret i64 [[V]] + +// Local const variables initialized with the enumerators are not constant +// folded. Global and static local ones keep the compile-time value. +const int g3 = E1_C; +int test8(int x) { + const int k = E1_C; + const int k2 = k + 1; + const struct key kk = { E1_A, 2 }; + static const int s = E1_C; + if (k2 == 11) + x++; + return x + k + kk.a + s + g3; +} + +// CHECK: define dso_local i32 @test8 +// CHECK: [[V:%[0-9a-z]+]] = call i64 @llvm.bpf.preserve.enum.value(i32 {{[0-9]+}}, ptr @{{[0-9]+}}, i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[E1]] +// CHECK: [[T:%[0-9a-z]+]] = trunc i64 [[V]] to i32 +// CHECK: store i32 [[T]], ptr %k, +// CHECK: [[K:%[0-9a-z]+]] = load i32, ptr %k, +// CHECK: add nsw i32 [[K]], 1 +// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 {{[0-9]+}}, ptr @{{[0-9]+}}, i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[E1]] +// CHECK: [[K2:%[0-9a-z]+]] = load i32, ptr %k2, +// CHECK: icmp eq i32 [[K2]], 11 +// CHECK: load i32, ptr %k, +// CHECK: load i32, ptr %a{{[0-9]+}}, +// CHECK: add nsw i32 %{{[0-9a-z]+}}, 10 +// CHECK-NEXT: add nsw i32 %{{[0-9a-z]+}}, 10 +// CHECK: ret i32 + +// A self-referencing initializer is handled. +int test9(void) { + const int k = k + E1_C; + return k; +} + +// CHECK: define dso_local i32 @test9 +// CHECK: call i64 @llvm.bpf.preserve.enum.value(i32 {{[0-9]+}}, ptr @{{[0-9]+}}, i64 1), !dbg !{{[0-9]+}}, !llvm.preserve.access.index ![[E1]] +// CHECK: ret i32 + +// CHECK-DAG: ![[AA]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "AA" +// CHECK-DAG: ![[BB]] = !DIDerivedType(tag: DW_TAG_typedef, name: "__BB" +// CHECK-DAG: ![[E1]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "e1" +// CHECK-DAG: ![[E2]] = !DIDerivedType(tag: DW_TAG_typedef, name: "e2" +// CHECK-DAG: ![[E4]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "e4" +// CHECK-DAG: ![[S64]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "s64"{{.*}}, size: 64, +// CHECK-DAG: ![[U64]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "u64"{{.*}}, size: 64, diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test index 8f8c1aa3d3f9c..a228186ea9fdb 100644 --- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test +++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test @@ -23,7 +23,7 @@ // CHECK-NEXT: Availability ((SubjectMatchRule_record, SubjectMatchRule_enum, SubjectMatchRule_enum_constant, SubjectMatchRule_field, SubjectMatchRule_function, SubjectMatchRule_namespace, SubjectMatchRule_objc_category, SubjectMatchRule_objc_implementation, SubjectMatchRule_objc_interface, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_objc_protocol, SubjectMatchRule_record, SubjectMatchRule_type_alias, SubjectMatchRule_variable)) // CHECK-NEXT: AvailableOnlyInDefaultEvalMethod (SubjectMatchRule_type_alias) // CHECK-NEXT: BPFFastCall (SubjectMatchRule_hasType_functionType) -// CHECK-NEXT: BPFPreserveAccessIndex (SubjectMatchRule_record) +// CHECK-NEXT: BPFPreserveAccessIndex (SubjectMatchRule_record, SubjectMatchRule_enum) // CHECK-NEXT: BPFPreserveStaticOffset (SubjectMatchRule_record) // CHECK-NEXT: BTFDeclTag (SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_record, SubjectMatchRule_field, SubjectMatchRule_type_alias) // CHECK-NEXT: BuiltinAlias (SubjectMatchRule_function) diff --git a/clang/test/Preprocessor/bpf-predefined-macros.c b/clang/test/Preprocessor/bpf-predefined-macros.c index 90287b7b24e95..29da78c56f9a4 100644 --- a/clang/test/Preprocessor/bpf-predefined-macros.c +++ b/clang/test/Preprocessor/bpf-predefined-macros.c @@ -79,6 +79,9 @@ int x; #ifdef __BPF_FEATURE_STACK_ARGUMENT int y; #endif +#ifdef __BPF_FEATURE_ENUM_PRESERVE_ACCESS_INDEX +int z; +#endif // CHECK: int b; // CHECK: int c; @@ -131,6 +134,11 @@ int y; // CPU_V3: int y; // CPU_V4: int y; +// CPU_V1: int z; +// CPU_V2: int z; +// CPU_V3: int z; +// CPU_V4: int z; + // CPU_GENERIC: int g; // CPU_PROBE: int f; diff --git a/clang/test/Sema/bpf-attr-preserve-access-index.c b/clang/test/Sema/bpf-attr-preserve-access-index.c index 34b8c917febf5..459637b06584e 100644 --- a/clang/test/Sema/bpf-attr-preserve-access-index.c +++ b/clang/test/Sema/bpf-attr-preserve-access-index.c @@ -36,13 +36,42 @@ struct p { int a; }; -int a __reloc__; // expected-error {{'preserve_access_index' attribute only applies to structs, unions, and classes}} -struct s *p __reloc__; // expected-error {{'preserve_access_index' attribute only applies to structs, unions, and classes}} +int a __reloc__; // expected-error {{'preserve_access_index' attribute only applies to structs, unions, classes, and enums}} +struct s *p __reloc__; // expected-error {{'preserve_access_index' attribute only applies to structs, unions, classes, and enums}} -void invalid1(const int __reloc__ *arg) {} // expected-error {{'preserve_access_index' attribute only applies to structs, unions, and classes}} -void invalid2(void) { const int __reloc__ *arg; } // expected-error {{'preserve_access_index' attribute only applies to structs, unions, and classes}} +void invalid1(const int __reloc__ *arg) {} // expected-error {{'preserve_access_index' attribute only applies to structs, unions, classes, and enums}} +void invalid2(void) { const int __reloc__ *arg; } // expected-error {{'preserve_access_index' attribute only applies to structs, unions, classes, and enums}} int valid3(struct t4 *arg) { return arg->a + arg->b[3] + arg->c + arg->d; } int valid4(void *arg) { struct local_t { int a; int b; } __reloc__; return ((struct local_t *)arg)->b; } + +enum e1 { E1_A, E1_B } __reloc__; +enum __reloc__ e2 { E2_A }; +typedef enum { E3_A } __reloc__ e3; +enum e4 { E4_A __reloc__ }; // expected-error {{'preserve_access_index' attribute only applies to structs, unions, classes, and enums}} +int valid5(void) { return E1_B + E2_A + E3_A; } + +// An anonymous enum without typedef name cannot be relocated. +enum { A1 } __reloc__; // expected-error {{'preserve_access_index' attribute has no effect on an anonymous enum without a typedef name}} +enum { A2 } __reloc__ v2; // expected-error {{'preserve_access_index' attribute has no effect on an anonymous enum}} +typedef enum { A3 } __reloc__ *t3p; // expected-error {{'preserve_access_index' attribute has no effect on an anonymous enum}} +struct s4 { enum { A4 } __reloc__ f; }; // expected-error {{'preserve_access_index' attribute has no effect on an anonymous enum}} +enum __reloc__ { A9 } __reloc__; // expected-error {{'preserve_access_index' attribute has no effect on an anonymous enum}} +#pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = enum) +enum { A10 } __reloc__; // expected-error {{'preserve_access_index' attribute has no effect on an anonymous enum}} +#pragma clang attribute pop +int invalid3(void) { + enum { A5 } __reloc__; // expected-error {{'preserve_access_index' attribute has no effect on an anonymous enum}} + return A1 + A2 + A3 + A4 + A5; +} + +// No diagnostic with a typedef name, an attribute implicitly added from the +// enclosing record, or #pragma clang attribute. +typedef enum __reloc__ { A6 } t6; +struct s7 { enum { A7 } f; } __reloc__; +#pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = enum) +enum { A8 }; +#pragma clang attribute pop +int valid6(void) { return A6 + A7 + A8; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
