llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Doug Wyatt (dougsonos) <details> <summary>Changes</summary> - In Sema, when encountering Decls with function effects needing verification, add them to a vector, DeclsWithEffectsToVerify. - Update AST serialization to include DeclsWithEffectsToVerify. - In AnalysisBasedWarnings, use DeclsWithEffectsToVerify as a work queue, verifying functions with declared effects, and inferring (when permitted and necessary) whether their callees have effects. --- Patch is 68.21 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/99656.diff 16 Files Affected: - (modified) clang/include/clang/AST/Type.h (+1-1) - (modified) clang/include/clang/Basic/DiagnosticGroups.td (+1) - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+49) - (modified) clang/include/clang/Sema/Sema.h (+13) - (modified) clang/include/clang/Serialization/ASTBitCodes.h (+4) - (modified) clang/include/clang/Serialization/ASTReader.h (+3) - (modified) clang/include/clang/Serialization/ASTWriter.h (+1) - (modified) clang/lib/Sema/AnalysisBasedWarnings.cpp (+1259) - (modified) clang/lib/Sema/SemaDecl.cpp (+56) - (modified) clang/lib/Sema/SemaExpr.cpp (+4) - (modified) clang/lib/Sema/SemaLambda.cpp (+5) - (modified) clang/lib/Serialization/ASTReader.cpp (+19) - (modified) clang/lib/Serialization/ASTWriter.cpp (+12) - (added) clang/test/Sema/attr-nonblocking-constraints.cpp (+194) - (modified) clang/test/Sema/attr-nonblocking-syntax.cpp (+1) - (added) clang/test/SemaObjCXX/attr-nonblocking-constraints.mm (+23) ``````````diff diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index 25defea58c2dc..08141f75de8db 100644 --- a/clang/include/clang/AST/Type.h +++ b/clang/include/clang/AST/Type.h @@ -4699,7 +4699,7 @@ class FunctionEffect { private: LLVM_PREFERRED_TYPE(Kind) - unsigned FKind : 3; + uint8_t FKind : 3; // Expansion: for hypothetical TCB+types, there could be one Kind for TCB, // then ~16(?) bits "SubKind" to map to a specific named TCB. SubKind would diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 19c3f1e043349..55d9442a939da 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1557,6 +1557,7 @@ def UnsafeBufferUsage : DiagGroup<"unsafe-buffer-usage", [UnsafeBufferUsageInCon // Warnings and notes related to the function effects system underlying // the nonblocking and nonallocating attributes. def FunctionEffects : DiagGroup<"function-effects">; +def PerfConstraintImpliesNoexcept : DiagGroup<"perf-constraint-implies-noexcept">; // Warnings and notes InstallAPI verification. def InstallAPIViolation : DiagGroup<"installapi-violation">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index d60f32674ca3a..ec02c02d158c8 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10928,6 +10928,55 @@ def warn_imp_cast_drops_unaligned : Warning< InGroup<DiagGroup<"unaligned-qualifier-implicit-cast">>; // Function effects +def warn_func_effect_allocates : Warning< + "'%0' function must not allocate or deallocate memory">, + InGroup<FunctionEffects>; +def note_func_effect_allocates : Note< + "function cannot be inferred '%0' because it allocates/deallocates memory">; +def warn_func_effect_throws_or_catches : Warning< + "'%0' function must not throw or catch exceptions">, + InGroup<FunctionEffects>; +def note_func_effect_throws_or_catches : Note< + "function cannot be inferred '%0' because it throws or catches exceptions">; +def warn_func_effect_has_static_local : Warning< + "'%0' function must not have static locals">, + InGroup<FunctionEffects>; +def note_func_effect_has_static_local : Note< + "function cannot be inferred '%0' because it has a static local">; +def warn_func_effect_uses_thread_local : Warning< + "'%0' function must not use thread-local variables">, + InGroup<FunctionEffects>; +def note_func_effect_uses_thread_local : Note< + "function cannot be inferred '%0' because it uses a thread-local variable">; +def warn_func_effect_calls_objc : Warning< + "'%0' function must not access an ObjC method or property">, + InGroup<FunctionEffects>; +def note_func_effect_calls_objc : Note< + "function cannot be inferred '%0' because it accesses an ObjC method or property">; +def warn_func_effect_calls_func_without_effect : Warning< + "'%0' function must not call non-'%0' function '%1'">, + InGroup<FunctionEffects>; +def warn_func_effect_calls_expr_without_effect : Warning< + "'%0' function must not call non-'%0' expression">, + InGroup<FunctionEffects>; +def note_func_effect_calls_func_without_effect : Note< + "function cannot be inferred '%0' because it calls non-'%0' function '%1'">; +def note_func_effect_call_extern : Note< + "function cannot be inferred '%0' because it has no definition in this translation unit">; +def note_func_effect_call_disallows_inference : Note< + "function does not permit inference of '%0'">; +def note_func_effect_call_virtual : Note< + "virtual method cannot be inferred '%0'">; +def note_func_effect_call_func_ptr : Note< + "function pointer cannot be inferred '%0'">; +def warn_perf_constraint_implies_noexcept : Warning< + "'%0' function should be declared noexcept">, + InGroup<PerfConstraintImpliesNoexcept>; + +// FIXME: It would be nice if we could provide fuller template expansion notes. +def note_func_effect_from_template : Note< + "in template expansion here">; + // spoofing nonblocking/nonallocating def warn_invalid_add_func_effects : Warning< "attribute '%0' should not be added via type conversion">, diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index d638d31e050dc..e1867348497da 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -875,6 +875,13 @@ class Sema final : public SemaBase { // ----- function effects --- + /// All functions/lambdas/blocks which have bodies and which have a non-empty + /// FunctionEffectsRef to be verified. + SmallVector<const Decl *> DeclsWithEffectsToVerify; + /// The union of all effects present on DeclsWithEffectsToVerify. Conditions + /// are all null. + FunctionEffectSet AllEffectsToVerify; + /// Warn when implicitly changing function effects. void diagnoseFunctionEffectConversion(QualType DstType, QualType SrcType, SourceLocation Loc); @@ -891,6 +898,12 @@ class Sema final : public SemaBase { SourceLocation NewLoc, SourceLocation OldLoc); + /// Potentially add a FunctionDecl or BlockDecl to DeclsWithEffectsToVerify. + void maybeAddDeclWithEffects(const Decl *D, const FunctionEffectsRef &FX); + + /// Unconditionally add a Decl to DeclsWithEfffectsToVerify. + void addDeclWithEffects(const Decl *D, const FunctionEffectsRef &FX); + /// Try to parse the conditional expression attached to an effect attribute /// (e.g. 'nonblocking'). (c.f. Sema::ActOnNoexceptSpec). Return an empty /// optional on error. diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h index 5dd0ba33f8a9c..b975db88dbaae 100644 --- a/clang/include/clang/Serialization/ASTBitCodes.h +++ b/clang/include/clang/Serialization/ASTBitCodes.h @@ -721,6 +721,10 @@ enum ASTRecordTypes { /// Record code for \#pragma clang unsafe_buffer_usage begin/end PP_UNSAFE_BUFFER_USAGE = 69, + + /// Record code for Sema's vector of functions/blocks with effects to + /// be verified. + DECLS_WITH_EFFECTS_TO_VERIFY = 70, }; /// Record types used within a source manager block. diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index 76e51ac7ab979..1d8985602146b 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -948,6 +948,9 @@ class ASTReader /// Sema tracks these to emit deferred diags. llvm::SmallSetVector<GlobalDeclID, 4> DeclsToCheckForDeferredDiags; + /// The IDs of all decls with function effects to be checked. + SmallVector<GlobalDeclID, 0> DeclsWithEffectsToVerify; + private: struct ImportedSubmodule { serialization::SubmoduleID ID; diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h index a0e475ec9f862..4eaf77e8cb8d9 100644 --- a/clang/include/clang/Serialization/ASTWriter.h +++ b/clang/include/clang/Serialization/ASTWriter.h @@ -592,6 +592,7 @@ class ASTWriter : public ASTDeserializationListener, void WriteMSPointersToMembersPragmaOptions(Sema &SemaRef); void WritePackPragmaOptions(Sema &SemaRef); void WriteFloatControlPragmaOptions(Sema &SemaRef); + void WriteDeclsWithEffectsToVerify(Sema &SemaRef); void WriteModuleFileExtension(Sema &SemaRef, ModuleFileExtensionWriter &Writer); diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 0f604c61fa3af..3909d5b44a32e 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2397,6 +2397,1262 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler { }; } // namespace +// ============================================================================= + +namespace FXAnalysis { + +enum class DiagnosticID : uint8_t { + None = 0, // sentinel for an empty Diagnostic + Throws, + Catches, + CallsObjC, + AllocatesMemory, + HasStaticLocal, + AccessesThreadLocal, + + // These only apply to callees, where the analysis stops at the Decl + DeclDisallowsInference, + + CallsDeclWithoutEffect, + CallsExprWithoutEffect, +}; + +// Holds an effect diagnosis, potentially for the entire duration of the +// analysis phase, in order to refer to it when explaining why a caller has been +// made unsafe by a callee. +struct Diagnostic { + FunctionEffect Effect; + DiagnosticID ID = DiagnosticID::None; + SourceLocation Loc; + const Decl *Callee = nullptr; // only valid for Calls* + + Diagnostic() = default; + + Diagnostic(const FunctionEffect &Effect, DiagnosticID ID, SourceLocation Loc, + const Decl *Callee = nullptr) + : Effect(Effect), ID(ID), Loc(Loc), Callee(Callee) {} +}; + +enum class SpecialFuncType : uint8_t { None, OperatorNew, OperatorDelete }; +enum class CallType { + // unknown: probably function pointer + Unknown, + Function, + Virtual, + Block +}; + +// Return whether a function's effects CAN be verified. +// The question of whether it SHOULD be verified is independent. +static bool functionIsVerifiable(const FunctionDecl *FD) { + if (!(FD->hasBody() || FD->isInlined())) { + // externally defined; we couldn't verify if we wanted to. + return false; + } + if (FD->isTrivial()) { + // Otherwise `struct x { int a; };` would have an unverifiable default + // constructor. + return true; + } + return true; +} + +/// A mutable set of FunctionEffect, for use in places where any conditions +/// have been resolved or can be ignored. +class EffectSet { + // This implementation optimizes footprint, since we hold one of these for + // every function visited, which, due to inference, can be many more functions + // than have declared effects. + + template <typename T, typename SizeT, SizeT Capacity> struct FixedVector { + SizeT Count = 0; + T Items[Capacity] = {}; + + using value_type = T; + + using iterator = T *; + using const_iterator = const T *; + iterator begin() { return &Items[0]; } + iterator end() { return &Items[Count]; } + const_iterator begin() const { return &Items[0]; } + const_iterator end() const { return &Items[Count]; } + const_iterator cbegin() const { return &Items[0]; } + const_iterator cend() const { return &Items[Count]; } + + void insert(iterator I, const T &Value) { + assert(Count < Capacity); + iterator E = end(); + if (I != E) + std::copy_backward(I, E, E + 1); + *I = Value; + ++Count; + } + + void push_back(const T &Value) { + assert(Count < Capacity); + Items[Count++] = Value; + } + }; + + // As long as FunctionEffect is only 1 byte, and there are only 2 verifiable + // effects, this fixed-size vector with a capacity of 7 is more than + // sufficient and is only 8 bytes. + FixedVector<FunctionEffect, uint8_t, 7> Impl; + +public: + EffectSet() = default; + explicit EffectSet(FunctionEffectsRef FX) { insert(FX); } + + operator ArrayRef<FunctionEffect>() const { + return ArrayRef(Impl.cbegin(), Impl.cend()); + } + + using iterator = const FunctionEffect *; + iterator begin() const { return Impl.cbegin(); } + iterator end() const { return Impl.cend(); } + + void insert(const FunctionEffect &Effect) { + FunctionEffect *Iter = Impl.begin(); + FunctionEffect *End = Impl.end(); + // linear search; lower_bound is overkill for a tiny vector like this + for (; Iter != End; ++Iter) { + if (*Iter == Effect) + return; + if (Effect < *Iter) + break; + } + Impl.insert(Iter, Effect); + } + void insert(const EffectSet &Set) { + for (const FunctionEffect &Item : Set) { + // push_back because set is already sorted + Impl.push_back(Item); + } + } + void insert(FunctionEffectsRef FX) { + for (const FunctionEffectWithCondition &EC : FX) { + assert(EC.Cond.getCondition() == + nullptr); // should be resolved by now, right? + // push_back because set is already sorted + Impl.push_back(EC.Effect); + } + } + bool contains(const FunctionEffect::Kind EK) const { + for (const FunctionEffect &E : Impl) + if (E.kind() == EK) + return true; + return false; + } + + void dump(llvm::raw_ostream &OS) const; + + static EffectSet difference(ArrayRef<FunctionEffect> LHS, + ArrayRef<FunctionEffect> RHS) { + EffectSet Result; + std::set_difference(LHS.begin(), LHS.end(), RHS.begin(), RHS.end(), + std::back_inserter(Result.Impl)); + return Result; + } +}; + +LLVM_DUMP_METHOD void EffectSet::dump(llvm::raw_ostream &OS) const { + OS << "Effects{"; + bool First = true; + for (const FunctionEffect &Effect : *this) { + if (!First) + OS << ", "; + else + First = false; + OS << Effect.name(); + } + OS << "}"; +} + +// Transitory, more extended information about a callable, which can be a +// function, block, function pointer, etc. +struct CallableInfo { + // CDecl holds the function's definition, if any. + // FunctionDecl if CallType::Function or Virtual + // BlockDecl if CallType::Block + const Decl *CDecl; + mutable std::optional<std::string> MaybeName; + SpecialFuncType FuncType = SpecialFuncType::None; + EffectSet Effects; + CallType CType = CallType::Unknown; + + CallableInfo(Sema &SemaRef, const Decl &CD, + SpecialFuncType FT = SpecialFuncType::None) + : CDecl(&CD), FuncType(FT) { + FunctionEffectsRef FXRef; + + if (auto *FD = dyn_cast<FunctionDecl>(CDecl)) { + // Use the function's definition, if any. + if (const FunctionDecl *Def = FD->getDefinition()) + CDecl = FD = Def; + CType = CallType::Function; + if (auto *Method = dyn_cast<CXXMethodDecl>(FD); + Method && Method->isVirtual()) + CType = CallType::Virtual; + FXRef = FD->getFunctionEffects(); + } else if (auto *BD = dyn_cast<BlockDecl>(CDecl)) { + CType = CallType::Block; + FXRef = BD->getFunctionEffects(); + } else if (auto *VD = dyn_cast<ValueDecl>(CDecl)) { + // ValueDecl is function, enum, or variable, so just look at its type. + FXRef = FunctionEffectsRef::get(VD->getType()); + } + Effects = EffectSet(FXRef); + } + + bool isDirectCall() const { + return CType == CallType::Function || CType == CallType::Block; + } + + bool isVerifiable() const { + switch (CType) { + case CallType::Unknown: + case CallType::Virtual: + break; + case CallType::Block: + return true; + case CallType::Function: + return functionIsVerifiable(dyn_cast<FunctionDecl>(CDecl)); + } + return false; + } + + /// Generate a name for logging and diagnostics. + std::string name(Sema &Sem) const { + if (!MaybeName) { + std::string Name; + llvm::raw_string_ostream OS(Name); + + if (auto *FD = dyn_cast<FunctionDecl>(CDecl)) + FD->getNameForDiagnostic(OS, Sem.getPrintingPolicy(), + /*Qualified=*/true); + else if (auto *BD = dyn_cast<BlockDecl>(CDecl)) + OS << "(block " << BD->getBlockManglingNumber() << ")"; + else if (auto *VD = dyn_cast<NamedDecl>(CDecl)) + VD->printQualifiedName(OS); + MaybeName = Name; + } + return *MaybeName; + } +}; + +// ---------- +// Map effects to single diagnostics, to hold the first (of potentially many) +// diagnostics pertaining to an effect, per function. +class EffectToDiagnosticMap { + // Since we currently only have a tiny number of effects (typically no more + // than 1), use a sorted SmallVector with an inline capacity of 1. Since it + // is often empty, use a unique_ptr to the SmallVector. + // Note that Diagnostic itself contains a FunctionEffect which is the key. + using ImplVec = llvm::SmallVector<Diagnostic, 1>; + std::unique_ptr<ImplVec> Impl; + +public: + // Insert a new diagnostic if we do not already have one for its effect. + void maybeInsert(const Diagnostic &Diag) { + if (Impl == nullptr) + Impl = std::make_unique<ImplVec>(); + auto *Iter = _find(Diag.Effect); + if (Iter != Impl->end() && Iter->Effect == Diag.Effect) + return; + + Impl->insert(Iter, Diag); + } + + const Diagnostic *lookup(FunctionEffect Key) { + if (Impl == nullptr) + return nullptr; + + auto *Iter = _find(Key); + if (Iter != Impl->end() && Iter->Effect == Key) + return &*Iter; + + return nullptr; + } + + size_t size() const { return Impl ? Impl->size() : 0; } + +private: + ImplVec::iterator _find(const FunctionEffect &key) { + // A linear search suffices for a tiny number of possible effects. + auto *End = Impl->end(); + for (auto *Iter = Impl->begin(); Iter != End; ++Iter) + if (!(Iter->Effect < key)) + return Iter; + return End; + } +}; + +// ---------- +// State pertaining to a function whose AST is walked and whose effect analysis +// is dependent on a subsequent analysis of other functions. +class PendingFunctionAnalysis { + friend class CompleteFunctionAnalysis; + +public: + struct DirectCall { + const Decl *Callee; + SourceLocation CallLoc; + // Not all recursive calls are detected, just enough + // to break cycles. + bool Recursed = false; + + DirectCall(const Decl *D, SourceLocation CallLoc) + : Callee(D), CallLoc(CallLoc) {} + }; + + // We always have two disjoint sets of effects to verify: + // 1. Effects declared explicitly by this function. + // 2. All other inferrable effects needing verification. + EffectSet DeclaredVerifiableEffects; + EffectSet FXToInfer; + +private: + // Diagnostics pertaining to the function's explicit effects. + SmallVector<Diagnostic, 0> DiagnosticsForExplicitFX; + + // Diagnostics pertaining to other, non-explicit, inferrable effects. + EffectToDiagnosticMap InferrableEffectToFirstDiagnostic; + + // These unverified direct calls are what keeps the analysis "pending", + // until the callees can be verified. + SmallVector<DirectCall, 0> UnverifiedDirectCalls; + +public: + PendingFunctionAnalysis( + Sema &Sem, const CallableInfo &CInfo, + ArrayRef<FunctionEffect> AllInferrableEffectsToVerify) { + DeclaredVerifiableEffects = CInfo.Effects; + + // Check for effects we are not allowed to infer + EffectSet InferrableFX; + + for (const FunctionEffect &effect : AllInferrableEffectsToVerify) { + if (effect.canInferOnFunction(*CInfo.CDecl)) + InferrableFX.insert(effect); + else { + // Add a diagnostic for this effect if a caller were to + // try to infer it. + InferrableEffectToFirstDiagnostic.maybeInsert( + Diagnostic(effect, DiagnosticID::DeclDisallowsInference, + CInfo.CDecl->getLocation())); + } + } + // InferrableFX is now the set of inferrable effects which are not + // prohibited + FXToInfer = EffectSet::difference(InferrableFX, DeclaredVerifiableEffects); + } + + // Hide the way that diagnostics for explicitly required effects vs. inferred + // ones are handled differently. + void checkAddDiagnostic(bool Inferring, const Diagnostic &NewDiag) { + if (!Inferring) + DiagnosticsForExplicitFX.push_back(NewDiag); + else + InferrableEffectToFirstDiagnostic.maybeInsert(NewDiag); + } + + void addUnverifiedDirectCall(const Decl *D, SourceLocation CallLoc) { + UnverifiedDirectCalls.emplace_back(D, CallLoc); + } + + // Analysis is complete when there are no unverified direct calls. + bool isComplete() const { return UnverifiedDirectCalls.empty(); } + + const Diagnostic *diagnosticForInferrableEffect(FunctionEffect effect) { + return InferrableEffectToFirstDiagnostic.lookup(effect); + } + + SmallVector<DirectCall, 0> &unverifiedCalls() { + assert(!isComplete()); + return UnverifiedDirectCalls; + } + + SmallVector<Diagnostic, 0> &getDiagnosticsForExplicitFX() { + return DiagnosticsForExplicitFX; + } + + void dump(Sema &SemaRef, llvm::raw_ostream &OS) const { + OS << "Pending: Declared "; + DeclaredVerifiableEffects.dump(OS); + OS << ", " << DiagnosticsForExplicitFX.size() << " diags; "; + OS << " Infer "; + FXToInfer.dump(OS); + OS << ", " << InferrableEffectToFi... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/99656 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
