llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-modules Author: Doug Wyatt (dougsonos) <details> <summary>Changes</summary> Rough first draft. This is an early PR to solicit comments on the overall approach and a number of outstanding questions. Some of the larger ones, "earlier" in the flow (from parse -> Sema): - Does the FunctionEffect / FunctionEffectSet abstraction make sense (Type.h)? The idea is that an abstract effect system for functions could easily support things like "TCB with types" or adding "nowait" to the "nolock/noalloc" group of effects. - FunctionEffect/FunctionEffectSet need to be serialized as part of FunctionProtoType - Conversions between functions / function pointers with and without the attribute work in C++ but not in C and I'm a bit lost (there's a bunch of debug logging around this e.g. Sema::IsFunctionConversion and Sema::ImpCastExprToType) --- Patch is 116.62 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/84983.diff 26 Files Affected: - (modified) clang/include/clang/AST/Decl.h (+9) - (modified) clang/include/clang/AST/Type.h (+294-3) - (modified) clang/include/clang/AST/TypeProperties.td (+5) - (modified) clang/include/clang/Basic/Attr.td (+23) - (modified) clang/include/clang/Basic/AttrDocs.td (+17) - (modified) clang/include/clang/Basic/DiagnosticGroups.td (+4) - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+95) - (modified) clang/include/clang/Sema/Sema.h (+16) - (modified) clang/lib/AST/ASTContext.cpp (+26-5) - (modified) clang/lib/AST/Decl.cpp (+9) - (modified) clang/lib/AST/Type.cpp (+286) - (modified) clang/lib/AST/TypePrinter.cpp (+12) - (modified) clang/lib/Sema/AnalysisBasedWarnings.cpp (+1237) - (modified) clang/lib/Sema/Sema.cpp (+23) - (modified) clang/lib/Sema/SemaDecl.cpp (+82) - (modified) clang/lib/Sema/SemaDeclAttr.cpp (+28) - (modified) clang/lib/Sema/SemaDeclCXX.cpp (+21) - (modified) clang/lib/Sema/SemaExpr.cpp (+4) - (modified) clang/lib/Sema/SemaExprCXX.cpp (+7-1) - (modified) clang/lib/Sema/SemaLambda.cpp (+5) - (modified) clang/lib/Sema/SemaOverload.cpp (+27) - (modified) clang/lib/Sema/SemaType.cpp (+125-1) - (added) clang/test/Sema/attr-nolock-wip.cpp (+19) - (added) clang/test/Sema/attr-nolock.cpp (+78) - (added) clang/test/Sema/attr-nolock2.cpp (+84) - (added) clang/test/Sema/attr-nolock3.cpp (+139) ``````````diff diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index a5879591f4c659..0460f30ce8a8b4 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -3008,6 +3008,13 @@ class FunctionDecl : public DeclaratorDecl, /// computed and stored. unsigned getODRHash() const; + FunctionEffectSet getFunctionEffects() const { + const auto *FPT = getType()->getAs<FunctionProtoType>(); + if (FPT) + return FPT->getFunctionEffects(); + return {}; + } + // Implement isa/cast/dyncast/etc. static bool classof(const Decl *D) { return classofKind(D->getKind()); } static bool classofKind(Kind K) { @@ -4633,6 +4640,8 @@ class BlockDecl : public Decl, public DeclContext { SourceRange getSourceRange() const override LLVM_READONLY; + FunctionEffectSet getFunctionEffects() const; + // Implement isa/cast/dyncast/etc. static bool classof(const Decl *D) { return classofKind(D->getKind()); } static bool classofKind(Kind K) { return K == Block; } diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index 1942b0e67f65a3..de21561ce5dcad 100644 --- a/clang/include/clang/AST/Type.h +++ b/clang/include/clang/AST/Type.h @@ -4047,8 +4047,12 @@ class FunctionType : public Type { LLVM_PREFERRED_TYPE(bool) unsigned HasArmTypeAttributes : 1; + LLVM_PREFERRED_TYPE(bool) + unsigned HasFunctionEffects : 1; + FunctionTypeExtraBitfields() - : NumExceptionType(0), HasArmTypeAttributes(false) {} + : NumExceptionType(0), HasArmTypeAttributes(false), + HasFunctionEffects(false) {} }; /// The AArch64 SME ACLE (Arm C/C++ Language Extensions) define a number @@ -4181,6 +4185,131 @@ class FunctionNoProtoType : public FunctionType, public llvm::FoldingSetNode { } }; +class FunctionEffect; +class FunctionEffectSet; + +// It is the user's responsibility to keep this in set form: elements are +// ordered and unique. +// We could hide the mutating methods which are capable of breaking the +// invariant, but they're needed and safe when used with STL set algorithms. +class MutableFunctionEffectSet : public SmallVector<const FunctionEffect *, 4> { +public: + using SmallVector::insert; + using SmallVector::SmallVector; + + /// Maintains order/uniquenesss. + void insert(const FunctionEffect *effect); + + MutableFunctionEffectSet &operator|=(FunctionEffectSet rhs); +}; + +class FunctionEffectSet { +public: + // These sets will tend to be very small (1 element), so represent them as + // sorted vectors, which are compatible with the STL set algorithms. Using an + // array or vector also means the elements are contiguous, keeping iterators + // simple. + +private: + // 'Uniqued' refers to the set itself being uniqued. Storage is allocated + // separately. Use ArrayRef for its iterators. Subclass so as to be able to + // compare (it seems ArrayRef would silently convert itself to a vector for + // comparison?!). + class UniquedAndSortedFX : public llvm::ArrayRef<const FunctionEffect *> { + public: + using Base = llvm::ArrayRef<const FunctionEffect *>; + + UniquedAndSortedFX(Base Array) : Base(Array) {} + UniquedAndSortedFX(const FunctionEffect **Ptr, size_t Len) + : Base(Ptr, Len) {} + + bool operator<(const UniquedAndSortedFX &rhs) const; + }; + + // Could have used a TinyPtrVector if it were unique-able. + // Empty set has a null Impl. + llvm::PointerUnion<const FunctionEffect *, const UniquedAndSortedFX *> Impl; + + explicit FunctionEffectSet(const FunctionEffect *Single) : Impl(Single) {} + explicit FunctionEffectSet(const UniquedAndSortedFX *Multi) : Impl(Multi) {} + +public: + using Differences = + SmallVector<std::pair<const FunctionEffect *, bool /*added*/>>; + + FunctionEffectSet() : Impl(nullptr) {} + + void *getOpaqueValue() const { return Impl.getOpaqueValue(); } + + explicit operator bool() const { return !empty(); } + bool empty() const { + if (Impl.isNull()) + return true; + if (const UniquedAndSortedFX *Vec = + dyn_cast_if_present<const UniquedAndSortedFX *>(Impl)) + return Vec->empty(); + return false; + } + size_t size() const { + if (empty()) + return 0; + if (isa<const FunctionEffect *>(Impl)) + return 1; + return cast<const UniquedAndSortedFX *>(Impl)->size(); + } + + using iterator = const FunctionEffect *const *; + + iterator begin() const { + if (isa<const FunctionEffect *>(Impl)) + return Impl.getAddrOfPtr1(); + return cast<const UniquedAndSortedFX *>(Impl)->begin(); + } + + iterator end() const { + if (isa<const FunctionEffect *>(Impl)) + return begin() + (Impl.isNull() ? 0 : 1); + return cast<const UniquedAndSortedFX *>(Impl)->end(); + } + + ArrayRef<const FunctionEffect *> items() const { return {begin(), end()}; } + + // Since iterators are non-trivial and sets are very often empty, + // encourage short-circuiting loops for the empty set. + // void for_each(llvm::function_ref<void(const FunctionEffect*)> func) const; + + bool operator==(const FunctionEffectSet &other) const { + return Impl == other.Impl; + } + bool operator!=(const FunctionEffectSet &other) const { + return Impl != other.Impl; + } + bool operator<(const FunctionEffectSet &other) const; + + void dump(llvm::raw_ostream &OS) const; + + /// Factory functions: return instances with uniqued implementations. + static FunctionEffectSet create(const FunctionEffect &single) { + return FunctionEffectSet{&single}; + } + static FunctionEffectSet create(llvm::ArrayRef<const FunctionEffect *> items); + + /// Union. Caller should check for incompatible effects. + FunctionEffectSet operator|(const FunctionEffectSet &rhs) const; + /// Intersection. + MutableFunctionEffectSet operator&(const FunctionEffectSet &rhs) const; + /// Difference. + MutableFunctionEffectSet operator-(const FunctionEffectSet &rhs) const; + + /// Caller should short-circuit by checking for equality first. + static Differences differences(const FunctionEffectSet &Old, + const FunctionEffectSet &New); + + /// Extract the effects from a Type if it is a BlockType or FunctionProtoType, + /// or pointer to one. + static FunctionEffectSet get(const Type &TyRef); +}; + /// Represents a prototype with parameter type info, e.g. /// 'int foo(int)' or 'int foo(void)'. 'void' is represented as having no /// parameters, not as having a single void parameter. Such a type can have @@ -4195,7 +4324,8 @@ class FunctionProtoType final FunctionProtoType, QualType, SourceLocation, FunctionType::FunctionTypeExtraBitfields, FunctionType::FunctionTypeArmAttributes, FunctionType::ExceptionType, - Expr *, FunctionDecl *, FunctionType::ExtParameterInfo, Qualifiers> { + Expr *, FunctionDecl *, FunctionType::ExtParameterInfo, + FunctionEffectSet, Qualifiers> { friend class ASTContext; // ASTContext creates these. friend TrailingObjects; @@ -4226,6 +4356,9 @@ class FunctionProtoType final // an ExtParameterInfo for each of the parameters. Present if and // only if hasExtParameterInfos() is true. // + // * Optionally, a FunctionEffectSet. Present if and only if + // hasFunctionEffects() is true. + // // * Optionally a Qualifiers object to represent extra qualifiers that can't // be represented by FunctionTypeBitfields.FastTypeQuals. Present if and only // if hasExtQualifiers() is true. @@ -4284,6 +4417,7 @@ class FunctionProtoType final ExceptionSpecInfo ExceptionSpec; const ExtParameterInfo *ExtParameterInfos = nullptr; SourceLocation EllipsisLoc; + FunctionEffectSet FunctionEffects; ExtProtoInfo() : Variadic(false), HasTrailingReturn(false), @@ -4301,7 +4435,8 @@ class FunctionProtoType final bool requiresFunctionProtoTypeExtraBitfields() const { return ExceptionSpec.Type == EST_Dynamic || - requiresFunctionProtoTypeArmAttributes(); + requiresFunctionProtoTypeArmAttributes() || + FunctionEffects; } bool requiresFunctionProtoTypeArmAttributes() const { @@ -4349,6 +4484,10 @@ class FunctionProtoType final return hasExtParameterInfos() ? getNumParams() : 0; } + unsigned numTrailingObjects(OverloadToken<FunctionEffectSet>) const { + return hasFunctionEffects(); + } + /// Determine whether there are any argument types that /// contain an unexpanded parameter pack. static bool containsAnyUnexpandedParameterPack(const QualType *ArgArray, @@ -4450,6 +4589,7 @@ class FunctionProtoType final EPI.RefQualifier = getRefQualifier(); EPI.ExtParameterInfos = getExtParameterInfosOrNull(); EPI.AArch64SMEAttributes = getAArch64SMEAttributes(); + EPI.FunctionEffects = getFunctionEffects(); return EPI; } @@ -4661,6 +4801,18 @@ class FunctionProtoType final return false; } + bool hasFunctionEffects() const { + if (!hasExtraBitfields()) + return false; + return getTrailingObjects<FunctionTypeExtraBitfields>()->HasFunctionEffects; + } + + FunctionEffectSet getFunctionEffects() const { + if (hasFunctionEffects()) + return *getTrailingObjects<FunctionEffectSet>(); + return {}; + } + bool isSugared() const { return false; } QualType desugar() const { return QualType(this, 0); } @@ -7754,6 +7906,145 @@ QualType DecayedType::getPointeeType() const { void FixedPointValueToString(SmallVectorImpl<char> &Str, llvm::APSInt Val, unsigned Scale); +// ------------------------------------------------------------------------------ + +// TODO: Should FunctionEffect be located elsewhere, where Decl is not +// forward-declared? +class Decl; +class CXXMethodDecl; + +/// Represents an abstract function effect. +class FunctionEffect { +public: + enum EffectType { + kGeneric, + kNoLockTrue, + kNoAllocTrue, + }; + + /// Flags describing behaviors of the effect. + using Flags = unsigned; + enum FlagBit : unsigned { + // Some effects require verification, e.g. nolock(true); others might not? + // (no example yet) + kRequiresVerification = 0x1, + + // Does this effect want to verify all function calls originating in + // functions having this effect? + kVerifyCalls = 0x2, + + // Can verification inspect callees' implementations? (e.g. nolock: yes, + // tcb+types: no) + kInferrableOnCallees = 0x4, + + // Language constructs which effects can diagnose as disallowed. + kExcludeThrow = 0x8, + kExcludeCatch = 0x10, + kExcludeObjCMessageSend = 0x20, + kExcludeStaticLocalVars = 0x40, + kExcludeThreadLocalVars = 0x80 + }; + +private: + const EffectType Type_; + const Flags Flags_; + const char *Name; + +public: + using CalleeDeclOrType = + llvm::PointerUnion<const Decl *, const FunctionProtoType *>; + + FunctionEffect(EffectType T, Flags F, const char *Name) + : Type_(T), Flags_(F), Name(Name) {} + virtual ~FunctionEffect(); + + /// The type of the effect. + EffectType type() const { return Type_; } + + /// Flags describing behaviors of the effect. + Flags flags() const { return Flags_; } + + /// The description printed in diagnostics, e.g. 'nolock'. + StringRef name() const { return Name; } + + /// The description used by TypePrinter, e.g. __attribute__((clang_nolock)) + virtual std::string attribute() const = 0; + + /// Return true if adding or removing the effect as part of a type conversion + /// should generate a diagnostic. + virtual bool diagnoseConversion(bool Adding, QualType OldType, + FunctionEffectSet OldFX, QualType NewType, + FunctionEffectSet NewFX) const; + + /// Return true if adding or removing the effect in a redeclaration should + /// generate a diagnostic. + virtual bool diagnoseRedeclaration(bool Adding, + const FunctionDecl &OldFunction, + FunctionEffectSet OldFX, + const FunctionDecl &NewFunction, + FunctionEffectSet NewFX) const; + + /// Return true if adding or removing the effect in a C++ virtual method + /// override should generate a diagnostic. + virtual bool diagnoseMethodOverride(bool Adding, + const CXXMethodDecl &OldMethod, + FunctionEffectSet OldFX, + const CXXMethodDecl &NewMethod, + FunctionEffectSet NewFX) const; + + /// Return true if the effect is allowed to be inferred on the specified Decl + /// (may be a FunctionDecl or BlockDecl). Only used if the effect has + /// kInferrableOnCallees flag set. Example: This allows nolock(false) to + /// prevent inference for the function. + virtual bool canInferOnDecl(const Decl *Caller, + FunctionEffectSet CallerFX) const; + + // Called if kVerifyCalls flag is set; return false for success. When true is + // returned for a direct call, then the kInferrableOnCallees flag may trigger + // inference rather than an immediate diagnostic. Caller should be assumed to + // have the effect (it may not have it explicitly when inferring). + virtual bool diagnoseFunctionCall(bool Direct, const Decl *Caller, + FunctionEffectSet CallerFX, + CalleeDeclOrType Callee, + FunctionEffectSet CalleeFX) const; +}; + +/// FunctionEffect subclass for nolock and noalloc (whose behaviors are close +/// to identical). +class NoLockNoAllocEffect : public FunctionEffect { + bool isNoLock() const { return type() == kNoLockTrue; } + +public: + static const NoLockNoAllocEffect &nolock_instance(); + static const NoLockNoAllocEffect &noalloc_instance(); + + NoLockNoAllocEffect(EffectType Type, const char *Name); + ~NoLockNoAllocEffect() override; + + std::string attribute() const override; + + bool diagnoseConversion(bool Adding, QualType OldType, + FunctionEffectSet OldFX, QualType NewType, + FunctionEffectSet NewFX) const override; + + bool diagnoseRedeclaration(bool Adding, const FunctionDecl &OldFunction, + FunctionEffectSet OldFX, + const FunctionDecl &NewFunction, + FunctionEffectSet NewFX) const override; + + bool diagnoseMethodOverride(bool Adding, const CXXMethodDecl &OldMethod, + FunctionEffectSet OldFX, + const CXXMethodDecl &NewMethod, + FunctionEffectSet NewFX) const override; + + bool canInferOnDecl(const Decl *Caller, + FunctionEffectSet CallerFX) const override; + + bool diagnoseFunctionCall(bool Direct, const Decl *Caller, + FunctionEffectSet CallerFX, CalleeDeclOrType Callee, + FunctionEffectSet CalleeFX) const override; +}; + } // namespace clang #endif // LLVM_CLANG_AST_TYPE_H diff --git a/clang/include/clang/AST/TypeProperties.td b/clang/include/clang/AST/TypeProperties.td index 0ba172a4035fdb..1d5d8f696977e7 100644 --- a/clang/include/clang/AST/TypeProperties.td +++ b/clang/include/clang/AST/TypeProperties.td @@ -326,6 +326,10 @@ let Class = FunctionProtoType in { def : Property<"AArch64SMEAttributes", UInt32> { let Read = [{ node->getAArch64SMEAttributes() }]; } + /* TODO: How to serialize FunctionEffect / FunctionEffectSet? + def : Property<"functionEffects", FunctionEffectSet> { + let Read = [{ node->getFunctionEffects() }]; + }*/ def : Creator<[{ auto extInfo = FunctionType::ExtInfo(noReturn, hasRegParm, regParm, @@ -342,6 +346,7 @@ let Class = FunctionProtoType in { epi.ExtParameterInfos = extParameterInfo.empty() ? nullptr : extParameterInfo.data(); epi.AArch64SMEAttributes = AArch64SMEAttributes; + //epi.FunctionEffects = functionEffects; return ctx.getFunctionType(returnType, parameters, epi); }]>; } diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index fd7970d0451acd..4033b9efb86f39 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -1402,6 +1402,29 @@ def CXX11NoReturn : InheritableAttr { let Documentation = [CXX11NoReturnDocs]; } +def NoLock : DeclOrTypeAttr { + let Spellings = [CXX11<"clang", "nolock">, + C23<"clang", "nolock">, + GNU<"clang_nolock">]; + + // Subjects - not needed? + //let Subjects = SubjectList<[FunctionLike, Block, TypedefName], ErrorDiag>; + let Args = [DefaultBoolArgument<"Cond", /*default*/1>]; + let HasCustomParsing = 0; + let Documentation = [NoLockNoAllocDocs]; +} + +def NoAlloc : DeclOrTypeAttr { + let Spellings = [CXX11<"clang", "noalloc">, + C23<"clang", "noalloc">, + GNU<"clang_noalloc">]; + // Subjects - not needed? + //let Subjects = SubjectList<[FunctionLike, Block, TypedefName], ErrorDiag>; + let Args = [DefaultBoolArgument<"Cond", /*default*/1>]; + let HasCustomParsing = 0; + let Documentation = [NoLockNoAllocDocs]; +} + // Similar to CUDA, OpenCL attributes do not receive a [[]] spelling because // the specification does not expose them with one currently. def OpenCLKernel : InheritableAttr { diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 2c07cd09b0d5b7..bb897c708ebbea 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -7973,3 +7973,20 @@ requirement: } }]; } + +def NoLockNoAllocDocs : Documentation { + let Category = DocCatType; + let Content = [{ +The ``nolock`` and ``noalloc`` attributes can be attached to functions, blocks, +function pointers, lambdas, and member functions. The attributes identify code +which must not allocate memory or lock, and the compiler uses the attributes to +verify these requirements. + +Like ``noexcept``, ``nolock`` and ``noalloc`` have an optional argument, a +compile-time constant boolean expression. By default, the argument is true, so +``[[clang::nolock(true)]]`` is equivalent to ``[[clang::nolock]]``, and declares +the function type as never locking. + +TODO: how much of the RFC to include here? Make it a separate page? + }]; +} diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 3f14167d6b8469..0d6e9f0289f6bc 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1507,3 +1507,7 @@ def ReadOnlyPlacementChecks : DiagGroup<"read-only-types">; // Warnings and fixes to support the "safe buffers" programming model. def UnsafeBufferUsageInContainer : DiagGroup<"unsafe-buffer-usage-in-container">; def UnsafeBufferUsage : DiagGroup<"unsafe-buffer-usage", [UnsafeBufferUsageInContainer]>; + +// Warnings and notes related to the function effects system underlying +// the nolock and noalloc attributes. +def FunctionEffects : DiagGroup<"function-effects">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index c54105507753eb..df553a47a8dbe7 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10778,6 +10778,101 @@ def warn_imp_cast_drops_unaligned : Warning< "implicit cast from type %0 to type %1 drops __unaligned qualifier">, InGroup<DiagGroup<"unaligned-qualifier-implicit-cast">>; +def warn_func_effect_allocates : Warning< + "'%0' function '%1' must not allocate or deallocate memory">, + InGroup<FunctionEffects>; + +def note_func_effect_allocates : Note< + "'%1' cannot be inferred '%0' because it allocates/deallocates memory">; + +def warn_func_effect_throws_or_catches : Warning< + "'%0' function '%1' must not throw or catch exceptions">, + InGroup<FunctionEffects>; + +def note_func_effect_throws_or_catches : Note< + "'%1' cannot be inferred '%0' because it throws or catches exceptions">; + +def warn_func_effect_has_static_local : Warning< + "'%0' function '%1' must not have sta... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/84983 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits