https://github.com/spaits updated https://github.com/llvm/llvm-project/pull/76580
From a19329050600d4d89cc698b686d7aaa13e0c22c2 Mon Sep 17 00:00:00 2001 From: Gabor Spaits <gaborspai...@gmail.com> Date: Fri, 29 Dec 2023 17:54:34 +0100 Subject: [PATCH 01/18] [analyzer] Add std::any checker --- clang/docs/analyzer/checkers.rst | 21 ++ .../clang/StaticAnalyzer/Checkers/Checkers.td | 4 + .../StaticAnalyzer/Checkers/CMakeLists.txt | 1 + .../StaticAnalyzer/Checkers/StdAnyChecker.cpp | 201 ++++++++++++++++++ .../Checkers/StdVariantChecker.cpp | 46 ++-- .../Checkers/TaggedUnionModeling.h | 9 +- .../Inputs/system-header-simulator-cxx.h | 59 +++++ clang/test/Analysis/std-any-checker.cpp | 170 +++++++++++++++ clang/test/Analysis/std-variant-checker.cpp | 8 + 9 files changed, 490 insertions(+), 29 deletions(-) create mode 100644 clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp create mode 100644 clang/test/Analysis/std-any-checker.cpp diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index bb637cf1b8007b..867bdfc9012253 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -2095,6 +2095,27 @@ This checker is a part of ``core.StackAddressEscape``, but is temporarily disabl // returned block } +.. _alpha-core-StdAny: + +alpha.core.StdAny (C++) +""""""""""""""""""""""" +Check if a value of active type is retrieved from an ``std::any`` instance with ``std::any_cats`` +or if the ``std::any`` instance holds type. In case of bad any type access +(the accessed type differs from the active type, or the instance has no stored value) +a warning is emitted. Currently, this checker does not take exception handling into account. + +.. code-block:: cpp + + void anyCast() { + std::any a = 5; + char c = std::any_cast<char>(a); // // warn: "int" is the active alternative + } + + void noTypeHeld() { + std::any a; + int i = std::any_cast<int>(a); // warn: any 'a' is empty + } + .. _alpha-core-StdVariant: alpha.core.StdVariant (C++) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index e7774e5a9392d2..954584fadb225d 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -321,6 +321,10 @@ def C11LockChecker : Checker<"C11Lock">, Dependencies<[PthreadLockBase]>, Documentation<HasDocumentation>; +def StdAnyChecker : Checker<"StdAny">, + HelpText<"Check for bad type access for std::any.">, + Documentation<HasDocumentation>; + def StdVariantChecker : Checker<"StdVariant">, HelpText<"Check for bad type access for std::variant.">, Documentation<HasDocumentation>; diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt index 4443ffd0929388..4e3475ff7f37da 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -107,6 +107,7 @@ add_clang_library(clangStaticAnalyzerCheckers SmartPtrChecker.cpp SmartPtrModeling.cpp StackAddrEscapeChecker.cpp + StdAnyChecker.cpp StdLibraryFunctionsChecker.cpp StdVariantChecker.cpp STLAlgorithmModeling.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp new file mode 100644 index 00000000000000..647ee72e555140 --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp @@ -0,0 +1,201 @@ +//===- StdAnyChecker.cpp -------------------------------------*- C++ -*----===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "llvm/Support/ErrorHandling.h" + +#include "TaggedUnionModeling.h" + +using namespace clang; +using namespace ento; +using namespace tagged_union_modeling; + +REGISTER_MAP_WITH_PROGRAMSTATE(AnyHeldTypeMap, const MemRegion *, QualType) + +class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { + CallDescription AnyConstructor{{"std", "any", "any"}}; + CallDescription AnyAsOp{{"std", "any", "operator="}}; + CallDescription AnyReset{{"std", "any", "reset"}, 0, 0}; + CallDescription AnyCast{{"std", "any_cast"}, 1, 1}; + + BugType BadAnyType{this, "BadAnyType", "BadAnyType"}; + BugType NullAnyType{this, "NullAnyType", "NullAnyType"}; + +public: + ProgramStateRef + checkRegionChanges(ProgramStateRef State, + const InvalidatedSymbols *Invalidated, + ArrayRef<const MemRegion *> ExplicitRegions, + ArrayRef<const MemRegion *> Regions, + const LocationContext *LCtx, const CallEvent *Call) const { + if (!Call) + return State; + + return removeInformationStoredForDeadInstances<AnyHeldTypeMap>(*Call, State, + Regions); + } + + bool evalCall(const CallEvent &Call, CheckerContext &C) const { + // Do not take implementation details into consideration + if (Call.isCalledFromSystemHeader()) + return false; + + if (AnyCast.matches(Call)) + return handleAnyCastCall(Call, C); + + if (AnyReset.matches(Call)) { + const auto *AsMemberCall = dyn_cast<CXXMemberCall>(&Call); + if (!AsMemberCall) + return false; + + const auto *ThisMemRegion = AsMemberCall->getCXXThisVal().getAsRegion(); + if (!ThisMemRegion) + return false; + + setNullTypeAny(ThisMemRegion, C); + return true; + } + + bool IsAnyConstructor = + isa<CXXConstructorCall>(Call) && AnyConstructor.matches(Call); + bool IsAnyAssignmentOperatorCall = + isa<CXXMemberOperatorCall>(Call) && AnyAsOp.matches(Call); + + if (IsAnyConstructor || IsAnyAssignmentOperatorCall) { + auto State = C.getState(); + SVal ThisSVal = [&]() { + if (IsAnyConstructor) { + const auto *AsConstructorCall = dyn_cast<CXXConstructorCall>(&Call); + return AsConstructorCall->getCXXThisVal(); + } + if (IsAnyAssignmentOperatorCall) { + const auto *AsMemberOpCall = dyn_cast<CXXMemberOperatorCall>(&Call); + return AsMemberOpCall->getCXXThisVal(); + } + llvm_unreachable("We must have an assignment operator or constructor"); + }(); + + // default constructor call + // in this case the any holds a null type + if (Call.getNumArgs() == 0) { + const auto *ThisMemRegion = ThisSVal.getAsRegion(); + setNullTypeAny(ThisMemRegion, C); + return true; + } + + if (Call.getNumArgs() != 1) + return false; + + handleConstructorAndAssignment<AnyHeldTypeMap>(Call, C, ThisSVal); + return true; + } + return false; + } + +private: + // When an std::any is rested or default constructed it has a null type. + // We represent it by storing a null QualType. + void setNullTypeAny(const MemRegion *Mem, CheckerContext &C) const { + auto State = C.getState(); + State = State->set<AnyHeldTypeMap>(Mem, QualType{}); + C.addTransition(State); + } + + // this function name is terrible + bool handleAnyCastCall(const CallEvent &Call, CheckerContext &C) const { + auto State = C.getState(); + + if (Call.getNumArgs() != 1) + return false; + + auto ArgSVal = Call.getArgSVal(0); + + // The argument is aether a const reference or a right value reference + // We need the type referred + const auto *ArgType = ArgSVal.getType(C.getASTContext()) + .getTypePtr() + ->getPointeeType() + .getTypePtr(); + if (!isStdAny(ArgType)) + return false; + + const auto *AnyMemRegion = ArgSVal.getAsRegion(); + + if (!State->contains<AnyHeldTypeMap>(AnyMemRegion)) + return false; + + // get the type we are trying to get from any + const CallExpr *CE = cast<CallExpr>(Call.getOriginExpr()); + const FunctionDecl *FD = CE->getDirectCallee(); + if (FD->getTemplateSpecializationArgs()->size() < 1) + return false; + + const auto &FirstTemplateArgument = + FD->getTemplateSpecializationArgs()->asArray()[0]; + if (FirstTemplateArgument.getKind() != TemplateArgument::ArgKind::Type) + return false; + + auto TypeOut = FirstTemplateArgument.getAsType(); + const auto *TypeStored = State->get<AnyHeldTypeMap>(AnyMemRegion); + + // Report when we try to use std::any_cast on an std::any that held a null + // type + if (TypeStored->isNull()) { + ExplodedNode *ErrNode = C.generateNonFatalErrorNode(); + if (!ErrNode) + return false; + llvm::SmallString<128> Str; + llvm::raw_svector_ostream OS(Str); + OS << "std::any " << AnyMemRegion->getDescriptiveName() << " is empty."; + auto R = std::make_unique<PathSensitiveBugReport>(NullAnyType, OS.str(), + ErrNode); + C.emitReport(std::move(R)); + return true; + } + + // Check if the right type is being accessed + // There is spacial case for object types. + QualType RetrievedCanonicalType = TypeOut.getCanonicalType(); + QualType StoredCanonicalType = TypeStored->getCanonicalType(); + if (RetrievedCanonicalType == StoredCanonicalType) + return true; + + // Report when the type we want to get out of std::any is wrong + ExplodedNode *ErrNode = C.generateNonFatalErrorNode(); + if (!ErrNode) + return false; + llvm::SmallString<128> Str; + llvm::raw_svector_ostream OS(Str); + std::string StoredTypeName = StoredCanonicalType.getAsString(); + std::string RetrievedTypeName = RetrievedCanonicalType.getAsString(); + OS << "std::any " << AnyMemRegion->getDescriptiveName() << " held " + << indefiniteArticleBasedOnVowel(StoredTypeName[0]) << " \'" + << StoredTypeName << "\', not " + << indefiniteArticleBasedOnVowel(RetrievedTypeName[0]) << " \'" + << RetrievedTypeName << "\'"; + auto R = + std::make_unique<PathSensitiveBugReport>(BadAnyType, OS.str(), ErrNode); + C.emitReport(std::move(R)); + return true; + } +}; + +bool clang::ento::shouldRegisterStdAnyChecker( + clang::ento::CheckerManager const &mgr) { + return true; +} + +void clang::ento::registerStdAnyChecker(clang::ento::CheckerManager &mgr) { + mgr.registerChecker<StdAnyChecker>(); +} \ No newline at end of file diff --git a/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp index f7b7befe28ee7d..62ca9f0ae836e0 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp @@ -17,9 +17,7 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h" #include "llvm/ADT/FoldingSet.h" #include "llvm/ADT/StringRef.h" -#include "llvm/Support/Casting.h" #include <optional> -#include <string_view> #include "TaggedUnionModeling.h" @@ -87,6 +85,28 @@ bool isStdVariant(const Type *Type) { return isStdType(Type, llvm::StringLiteral("variant")); } +bool isStdAny(const Type *Type) { + return isStdType(Type, llvm::StringLiteral("any")); +} + +bool isVowel(char a) { + switch (a) { + case 'a': + case 'e': + case 'i': + case 'o': + case 'u': + return true; + default: + return false; + } +} + +llvm::StringRef indefiniteArticleBasedOnVowel(char a) { + if (isVowel(a)) + return "an"; + return "a"; +} } // end of namespace clang::ento::tagged_union_modeling static std::optional<ArrayRef<TemplateArgument>> @@ -108,25 +128,6 @@ getNthTemplateTypeArgFromVariant(const Type *varType, unsigned i) { return (*VariantTemplates)[i].getAsType(); } -static bool isVowel(char a) { - switch (a) { - case 'a': - case 'e': - case 'i': - case 'o': - case 'u': - return true; - default: - return false; - } -} - -static llvm::StringRef indefiniteArticleBasedOnVowel(char a) { - if (isVowel(a)) - return "an"; - return "a"; -} - class StdVariantChecker : public Checker<eval::Call, check::RegionChanges> { // Call descriptors to find relevant calls CallDescription VariantConstructor{{"std", "variant", "variant"}}; @@ -184,9 +185,8 @@ class StdVariantChecker : public Checker<eval::Call, check::RegionChanges> { } else if (IsVariantAssignmentOperatorCall) { const auto &AsMemberOpCall = cast<CXXMemberOperatorCall>(Call); ThisSVal = AsMemberOpCall.getCXXThisVal(); - } else { + } else return false; - } handleConstructorAndAssignment<VariantHeldTypeMap>(Call, C, ThisSVal); return true; diff --git a/clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h b/clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h index 6de33da107a3f9..760622f954f550 100644 --- a/clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h +++ b/clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h @@ -9,15 +9,9 @@ #ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_TAGGEDUNIONMODELING_H #define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_TAGGEDUNIONMODELING_H -#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" -#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" -#include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" -#include "llvm/ADT/FoldingSet.h" -#include <numeric> namespace clang::ento::tagged_union_modeling { @@ -30,6 +24,9 @@ bool isMoveAssignmentCall(const CallEvent &Call); bool isMoveConstructorCall(const CallEvent &Call); bool isStdType(const Type *Type, const std::string &TypeName); bool isStdVariant(const Type *Type); +bool isStdAny(const Type *Type); +bool isVowel(char a); +llvm::StringRef indefiniteArticleBasedOnVowel(char a); // When invalidating regions, we also have to follow that by invalidating the // corresponding custom data in the program state. diff --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h index 3ef7af2ea6c6ab..e1ad590526eb38 100644 --- a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h +++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h @@ -1372,6 +1372,65 @@ template <typename Ret, typename... Args> class packaged_task<Ret(Args...)> { typename = std::enable_if_t<!is_same_v<std::variant<Types...>, decay_t<T>>>> variant& operator=(T&&); }; + + class bad_any_cast; + + // class any + class any; + + // non-member functions + void swap(any& x, any& y) noexcept; + + template<class T, class... Args> + any make_any(Args&&... args); + template<class T, class U, class... Args> + any make_any(initializer_list<U> il, Args&&... args); + + template<class T> + T any_cast(const any& operand); + template<class T> + T any_cast(any& operand); + template<class T> + T any_cast(any&& operand); + + template<class T> + const T* any_cast(const any* operand) noexcept; + template<class T> + T* any_cast(any* operand) noexcept; + + class any { + public: + // construction and destruction + constexpr any() noexcept; + + any(const any& other); + any(any&& other) noexcept; + + template<class T> + any(T&& value); + + ~any(); + + // assignments + any& operator=(const any& rhs); + any& operator=(any&& rhs) noexcept; + + template<typename T, + typename = std::enable_if_t<!is_same_v<std::any, decay_t<T>>>> + any& operator=(T&& rhs); + + // modifiers + template<class T, class... Args> + decay_t<T>& emplace(Args&&...); + template<class T, class U, class... Args> + decay_t<T>& emplace(initializer_list<U>, Args&&...); + void reset() noexcept; + void swap(any& rhs) noexcept; + + // observers + bool has_value() const noexcept; + }; + #endif } // namespace std diff --git a/clang/test/Analysis/std-any-checker.cpp b/clang/test/Analysis/std-any-checker.cpp new file mode 100644 index 00000000000000..d28d311b32d843 --- /dev/null +++ b/clang/test/Analysis/std-any-checker.cpp @@ -0,0 +1,170 @@ +// RUN: %clang %s -Xclang -verify --analyze \ +// RUN: -Xclang -analyzer-checker=core \ +// RUN: -Xclang -analyzer-checker=debug.ExprInspection \ +// RUN: -Xclang -analyzer-checker=core,alpha.core.StdAny + +#include "Inputs/system-header-simulator-cxx.h" + +void clang_analyzer_warnIfReached(); +void clang_analyzer_eval(int); + + +class DummyClass{ + public: + void foo(){}; +}; + +void nonInlined(std::any &a); +void nonInlinedConst(const std::any & a); + +void inlined(std::any &a) { + a = 5; +} + +using any_t = std::any; +using any_tt = any_t; + + +//----------------------------------------------------------------------------// +// std::any_cast +//----------------------------------------------------------------------------// +void objectHeld() { + std::any a = DummyClass{}; + DummyClass d = std::any_cast<DummyClass>(a); + d.foo(); +} + +void formVariable() { + std::any a = 5; + int b = std::any_cast<int>(a); + char c = std::any_cast<char>(a); // expected-warning {{std::any 'a' held an 'int', not a 'char'}} + (void)b; + (void)c; +} + +void pointerHeld() { + int i = 5; + std::any a = &i; + int* x = std::any_cast<int*>(a); + char c = std::any_cast<char>(a); // expected-warning {{std::any 'a' held an 'int *', not a 'char'}} + (void)x; + (void)c; +} + +//----------------------------------------------------------------------------// +// Empty std::any +//----------------------------------------------------------------------------// + +void noTypeHeld() { + std::any a; + int i = std::any_cast<int>(a); // expected-warning {{any 'a' is empty}} + (void)i; +} + +void reset() { + std::any a = 15; + a.reset(); + int i = std::any_cast<int>(a); // expected-warning {{any 'a' is empty}} + (void)i; +} + + +//----------------------------------------------------------------------------// +// Typedefs +//----------------------------------------------------------------------------// + +void typedefedAny () { + any_t a = 5; + int i = std::any_cast<int>(a); + char c = std::any_cast<char>(a); // expected-warning {{std::any 'a' held an 'int', not a 'char'}} + (void)i; + (void)c; +} + +void typedefedTypedefedAny () { + any_tt a = 5; + int i = std::any_cast<int>(a); + char c = std::any_cast<char>(a); // expected-warning {{std::any 'a' held an 'int', not a 'char'}} + (void)i; + (void)c; +} + +//----------------------------------------------------------------------------// +// Constructors and assignments +//----------------------------------------------------------------------------// + +void assignmentOp () { + std::any a; + a = 5; + int i = std::any_cast<int>(a); + char c = std::any_cast<char>(a); // expected-warning {{std::any 'a' held an 'int', not a 'char'}} + (void)i; + (void)c; +} + +void constructor() { + std::any a(5); + int i = std::any_cast<int>(a); + char c = std::any_cast<char>(a); // expected-warning {{std::any 'a' held an 'int', not a 'char'}} + (void)i; + (void)c; +} + +void copyCtor() { + std::any a(5); + std::any b(a); + int i = std::any_cast<int>(a); + char c = std::any_cast<char>(a); // expected-warning {{std::any 'a' held an 'int', not a 'char'}} + (void)i; + (void)c; +} + +void copyCtorNullType() { + std::any a; + std::any b(a); + char c = std::any_cast<char>(a); // expected-warning {{any 'a' is empty}} + (void)c; +} + +void copyAssignment() { + std::any a = 5; + std::any b = 'c'; + char c = std::any_cast<char>(b); + (void)c; + b = a; + int i = std::any_cast<int>(b); + c = std::any_cast<char>(b); // expected-warning {{std::any 'b' held an 'int', not a 'char'}} + (void)i; + (void)c; +} + +//----------------------------------------------------------------------------// +// Function calls +//----------------------------------------------------------------------------// + +void nonInlinedRefCall() { + std::any a = 5; + nonInlined(a); + int i = std::any_cast<int>(a); + char c = std::any_cast<char>(a); + (void)i; + (void)c; +} + +void nonInlinedConstRefCall() { + std::any a = 5; + nonInlinedConst(a); + int i = std::any_cast<int>(a); + char c = std::any_cast<char>(a); // expected-warning {{std::any 'a' held an 'int', not a 'char'}} + (void)i; + (void)c; +} + +void inlinedCall() { + std::any a = 'c'; + inlined(a); + int i = std::any_cast<int>(a); + char c = std::any_cast<char>(a); // expected-warning {{std::any 'a' held an 'int', not a 'char'}} + (void)i; + (void)c; +} \ No newline at end of file diff --git a/clang/test/Analysis/std-variant-checker.cpp b/clang/test/Analysis/std-variant-checker.cpp index 7f136c06b19cc6..bdae26742826af 100644 --- a/clang/test/Analysis/std-variant-checker.cpp +++ b/clang/test/Analysis/std-variant-checker.cpp @@ -58,6 +58,14 @@ void wontConfuseStdGets() { //----------------------------------------------------------------------------// // std::get //----------------------------------------------------------------------------// +void stdGetType2() { + std::variant<int, char> v = {25}; + int a = std::get<int>(v); + char c = std::get<char>(v); // expected-warning {{std::variant 'v' held an 'int', not a 'char'}} + (void)a; + (void)c; +} + void stdGetType() { std::variant<int, char> v = 25; int a = std::get<int>(v); From a0679b9dfc1a91372ef5108c3ee6f66f9130c64c Mon Sep 17 00:00:00 2001 From: Gabor Spaits <gaborspai...@gmail.com> Date: Fri, 29 Dec 2023 21:33:16 +0100 Subject: [PATCH 02/18] Omit unused params for checkRegionChanges --- clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp index 647ee72e555140..70852b832de518 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp @@ -33,12 +33,12 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { BugType NullAnyType{this, "NullAnyType", "NullAnyType"}; public: - ProgramStateRef - checkRegionChanges(ProgramStateRef State, - const InvalidatedSymbols *Invalidated, - ArrayRef<const MemRegion *> ExplicitRegions, - ArrayRef<const MemRegion *> Regions, - const LocationContext *LCtx, const CallEvent *Call) const { + ProgramStateRef checkRegionChanges(ProgramStateRef State, + const InvalidatedSymbols *, + ArrayRef<const MemRegion *>, + ArrayRef<const MemRegion *> Regions, + const LocationContext *, + const CallEvent *Call) const { if (!Call) return State; From f204040aed16c7ae0030a3db586e5239240dd342 Mon Sep 17 00:00:00 2001 From: Gabor Spaits <gaborspai...@gmail.com> Date: Fri, 29 Dec 2023 21:40:32 +0100 Subject: [PATCH 03/18] Add better explanation for BugTypes --- clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp index 70852b832de518..2faee3cd1374f8 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp @@ -29,8 +29,8 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { CallDescription AnyReset{{"std", "any", "reset"}, 0, 0}; CallDescription AnyCast{{"std", "any_cast"}, 1, 1}; - BugType BadAnyType{this, "BadAnyType", "BadAnyType"}; - BugType NullAnyType{this, "NullAnyType", "NullAnyType"}; + const BugType BadAnyType{this, "Bad std::any type access.", "BadAnyType"}; + const BugType NullAnyType{this, "std::any has no value", "NullAnyType"}; public: ProgramStateRef checkRegionChanges(ProgramStateRef State, From 065e61e09d6a151ea6b5ff9ff82ac11350e0f747 Mon Sep 17 00:00:00 2001 From: Gabor Spaits <gaborspai...@gmail.com> Date: Fri, 29 Dec 2023 21:41:26 +0100 Subject: [PATCH 04/18] Add explicit flag to use c++ 17 for tests --- clang/test/Analysis/std-any-checker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/Analysis/std-any-checker.cpp b/clang/test/Analysis/std-any-checker.cpp index d28d311b32d843..4e495d4d62b48a 100644 --- a/clang/test/Analysis/std-any-checker.cpp +++ b/clang/test/Analysis/std-any-checker.cpp @@ -1,4 +1,4 @@ -// RUN: %clang %s -Xclang -verify --analyze \ +// RUN: %clang %s -std=c++17 -Xclang -verify --analyze \ // RUN: -Xclang -analyzer-checker=core \ // RUN: -Xclang -analyzer-checker=debug.ExprInspection \ // RUN: -Xclang -analyzer-checker=core,alpha.core.StdAny From e88d89d20d6259dc05e52f065aad371b684e3967 Mon Sep 17 00:00:00 2001 From: Gabor Spaits <gaborspai...@gmail.com> Date: Fri, 29 Dec 2023 21:45:15 +0100 Subject: [PATCH 05/18] Change comments according to llvm style --- .../StaticAnalyzer/Checkers/StdAnyChecker.cpp | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp index 2faee3cd1374f8..5f8af1e651ba8f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp @@ -86,8 +86,8 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { llvm_unreachable("We must have an assignment operator or constructor"); }(); - // default constructor call - // in this case the any holds a null type + // Default constructor call. + // In this case the any holds a null type. if (Call.getNumArgs() == 0) { const auto *ThisMemRegion = ThisSVal.getAsRegion(); setNullTypeAny(ThisMemRegion, C); @@ -112,7 +112,6 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { C.addTransition(State); } - // this function name is terrible bool handleAnyCastCall(const CallEvent &Call, CheckerContext &C) const { auto State = C.getState(); @@ -121,8 +120,8 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { auto ArgSVal = Call.getArgSVal(0); - // The argument is aether a const reference or a right value reference - // We need the type referred + // The argument is ether a const reference or a right value reference. + // We need the type referred. const auto *ArgType = ArgSVal.getType(C.getASTContext()) .getTypePtr() ->getPointeeType() @@ -135,7 +134,7 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { if (!State->contains<AnyHeldTypeMap>(AnyMemRegion)) return false; - // get the type we are trying to get from any + // Get the type we are trying to retrieve from any. const CallExpr *CE = cast<CallExpr>(Call.getOriginExpr()); const FunctionDecl *FD = CE->getDirectCallee(); if (FD->getTemplateSpecializationArgs()->size() < 1) @@ -150,7 +149,7 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { const auto *TypeStored = State->get<AnyHeldTypeMap>(AnyMemRegion); // Report when we try to use std::any_cast on an std::any that held a null - // type + // type. if (TypeStored->isNull()) { ExplodedNode *ErrNode = C.generateNonFatalErrorNode(); if (!ErrNode) @@ -164,14 +163,13 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { return true; } - // Check if the right type is being accessed - // There is spacial case for object types. + // Check if the right type is being accessed. QualType RetrievedCanonicalType = TypeOut.getCanonicalType(); QualType StoredCanonicalType = TypeStored->getCanonicalType(); if (RetrievedCanonicalType == StoredCanonicalType) return true; - // Report when the type we want to get out of std::any is wrong + // Report when the type we want to get out of std::any is wrong. ExplodedNode *ErrNode = C.generateNonFatalErrorNode(); if (!ErrNode) return false; From 4c249ee7c4fc3ec2b992b664b66895436dbd0da4 Mon Sep 17 00:00:00 2001 From: Gabor Spaits <gaborspai...@gmail.com> Date: Fri, 29 Dec 2023 22:02:47 +0100 Subject: [PATCH 06/18] Change less to not equeal comparison --- clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp index 5f8af1e651ba8f..c1b2af562e78d3 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp @@ -137,7 +137,7 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { // Get the type we are trying to retrieve from any. const CallExpr *CE = cast<CallExpr>(Call.getOriginExpr()); const FunctionDecl *FD = CE->getDirectCallee(); - if (FD->getTemplateSpecializationArgs()->size() < 1) + if (FD->getTemplateSpecializationArgs()->size() != 1) return false; const auto &FirstTemplateArgument = From 7108efbb360861703aa73525df11c3d843b51aea Mon Sep 17 00:00:00 2001 From: Gabor Spaits <gaborspai...@gmail.com> Date: Fri, 29 Dec 2023 22:11:57 +0100 Subject: [PATCH 07/18] Remove state manipulation from setNullTypeAny --- clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp index c1b2af562e78d3..df7b9c9c152bf2 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp @@ -13,6 +13,7 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h" #include "llvm/Support/ErrorHandling.h" #include "TaggedUnionModeling.h" @@ -63,7 +64,7 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { if (!ThisMemRegion) return false; - setNullTypeAny(ThisMemRegion, C); + C.addTransition(setNullTypeAny(ThisMemRegion, C)); return true; } @@ -90,7 +91,7 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { // In this case the any holds a null type. if (Call.getNumArgs() == 0) { const auto *ThisMemRegion = ThisSVal.getAsRegion(); - setNullTypeAny(ThisMemRegion, C); + C.addTransition(setNullTypeAny(ThisMemRegion, C)); return true; } @@ -106,10 +107,10 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { private: // When an std::any is rested or default constructed it has a null type. // We represent it by storing a null QualType. - void setNullTypeAny(const MemRegion *Mem, CheckerContext &C) const { + ProgramStateRef setNullTypeAny(const MemRegion *Mem, CheckerContext &C) const { auto State = C.getState(); - State = State->set<AnyHeldTypeMap>(Mem, QualType{}); - C.addTransition(State); + return State->set<AnyHeldTypeMap>(Mem, QualType{}); + //C.addTransition(State); } bool handleAnyCastCall(const CallEvent &Call, CheckerContext &C) const { From 0631181759f5efc5f8268c6d736120567cd4ad4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Spaits?= <48805437+spa...@users.noreply.github.com> Date: Wed, 3 Jan 2024 15:27:39 +0100 Subject: [PATCH 08/18] Update clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h Co-authored-by: whisperity <whisper...@gmail.com> --- clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h b/clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h index 760622f954f550..75a804219f1b1c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h +++ b/clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h @@ -23,8 +23,8 @@ bool isCopyAssignmentCall(const CallEvent &Call); bool isMoveAssignmentCall(const CallEvent &Call); bool isMoveConstructorCall(const CallEvent &Call); bool isStdType(const Type *Type, const std::string &TypeName); -bool isStdVariant(const Type *Type); bool isStdAny(const Type *Type); +bool isStdVariant(const Type *Type); bool isVowel(char a); llvm::StringRef indefiniteArticleBasedOnVowel(char a); From 8505607ee313709fa2682edc2be377b7128f59e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Spaits?= <48805437+spa...@users.noreply.github.com> Date: Wed, 3 Jan 2024 15:28:00 +0100 Subject: [PATCH 09/18] Update clang/docs/analyzer/checkers.rst Co-authored-by: whisperity <whisper...@gmail.com> --- clang/docs/analyzer/checkers.rst | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 867bdfc9012253..b4f58b09d3bdff 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -2099,10 +2099,12 @@ This checker is a part of ``core.StackAddressEscape``, but is temporarily disabl alpha.core.StdAny (C++) """"""""""""""""""""""" -Check if a value of active type is retrieved from an ``std::any`` instance with ``std::any_cats`` -or if the ``std::any`` instance holds type. In case of bad any type access -(the accessed type differs from the active type, or the instance has no stored value) -a warning is emitted. Currently, this checker does not take exception handling into account. +Check if a value of active type is retrieved from a ``std::any`` instance with ``std::any_cast`` +or if the instance holds a value. In case of bad access +(the accessed type differs from the active type, or the instance is empty) +a warning is emitted. + +**Limitation:** The checker does not take exception handling into account. .. code-block:: cpp From 023ab28acdd8c692d16db97233721d9589e83c66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Spaits?= <48805437+spa...@users.noreply.github.com> Date: Wed, 3 Jan 2024 15:28:19 +0100 Subject: [PATCH 10/18] Update clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp Co-authored-by: whisperity <whisper...@gmail.com> --- clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp index df7b9c9c152bf2..35ec5bb1f229ce 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp @@ -105,8 +105,8 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { } private: - // When an std::any is rested or default constructed it has a null type. - // We represent it by storing a null QualType. + // When a std::any is reset or default constructed it has a null type. + // We represent it by storing an empty QualType. ProgramStateRef setNullTypeAny(const MemRegion *Mem, CheckerContext &C) const { auto State = C.getState(); return State->set<AnyHeldTypeMap>(Mem, QualType{}); From 68a4d471e3b28f76dfc38587999f4b5bb3ca7d9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Spaits?= <48805437+spa...@users.noreply.github.com> Date: Wed, 3 Jan 2024 15:28:48 +0100 Subject: [PATCH 11/18] Update clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp Co-authored-by: whisperity <whisper...@gmail.com> --- clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp index 35ec5bb1f229ce..98d9465d005bc4 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp @@ -121,7 +121,7 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { auto ArgSVal = Call.getArgSVal(0); - // The argument is ether a const reference or a right value reference. + // The argument is either a const reference or a right value reference. // We need the type referred. const auto *ArgType = ArgSVal.getType(C.getASTContext()) .getTypePtr() From c6ce2ab3583c83eb31afab0f4949c1107cceb8b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Spaits?= <48805437+spa...@users.noreply.github.com> Date: Wed, 3 Jan 2024 15:29:15 +0100 Subject: [PATCH 12/18] Update clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp Co-authored-by: whisperity <whisper...@gmail.com> --- clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp index 98d9465d005bc4..2d6e17db84bb34 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp @@ -149,7 +149,7 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { auto TypeOut = FirstTemplateArgument.getAsType(); const auto *TypeStored = State->get<AnyHeldTypeMap>(AnyMemRegion); - // Report when we try to use std::any_cast on an std::any that held a null + // Report when we try to use std::any_cast on a std::any that held a null // type. if (TypeStored->isNull()) { ExplodedNode *ErrNode = C.generateNonFatalErrorNode(); From baf9e52827b0ff3a7d3518604619a48082ee2702 Mon Sep 17 00:00:00 2001 From: Gabor Spaits <gaborspai...@gmail.com> Date: Thu, 4 Jan 2024 20:10:29 +0100 Subject: [PATCH 13/18] Remove redundant test case --- clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp | 4 ++-- clang/test/Analysis/std-variant-checker.cpp | 8 -------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp index 2d6e17db84bb34..31d83993f1c3b2 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp @@ -107,10 +107,10 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { private: // When a std::any is reset or default constructed it has a null type. // We represent it by storing an empty QualType. - ProgramStateRef setNullTypeAny(const MemRegion *Mem, CheckerContext &C) const { + ProgramStateRef setNullTypeAny(const MemRegion *Mem, + CheckerContext &C) const { auto State = C.getState(); return State->set<AnyHeldTypeMap>(Mem, QualType{}); - //C.addTransition(State); } bool handleAnyCastCall(const CallEvent &Call, CheckerContext &C) const { diff --git a/clang/test/Analysis/std-variant-checker.cpp b/clang/test/Analysis/std-variant-checker.cpp index bdae26742826af..7f136c06b19cc6 100644 --- a/clang/test/Analysis/std-variant-checker.cpp +++ b/clang/test/Analysis/std-variant-checker.cpp @@ -58,14 +58,6 @@ void wontConfuseStdGets() { //----------------------------------------------------------------------------// // std::get //----------------------------------------------------------------------------// -void stdGetType2() { - std::variant<int, char> v = {25}; - int a = std::get<int>(v); - char c = std::get<char>(v); // expected-warning {{std::variant 'v' held an 'int', not a 'char'}} - (void)a; - (void)c; -} - void stdGetType() { std::variant<int, char> v = 25; int a = std::get<int>(v); From 2652148b44bba07373a2d8a9cfdc758db748ec2b Mon Sep 17 00:00:00 2001 From: Gabor Spaits <gaborspai...@gmail.com> Date: Thu, 4 Jan 2024 20:15:19 +0100 Subject: [PATCH 14/18] Generate fatal errors for bad type access --- clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp | 2 +- clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp index 31d83993f1c3b2..62adf0ab4eac3b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp @@ -152,7 +152,7 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { // Report when we try to use std::any_cast on a std::any that held a null // type. if (TypeStored->isNull()) { - ExplodedNode *ErrNode = C.generateNonFatalErrorNode(); + ExplodedNode *ErrNode = C.generateErrorNode(); if (!ErrNode) return false; llvm::SmallString<128> Str; diff --git a/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp index 62ca9f0ae836e0..1157b4f854963b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp @@ -269,7 +269,7 @@ class StdVariantChecker : public Checker<eval::Call, check::RegionChanges> { if (RetrievedCanonicalType == StoredCanonicalType) return true; - ExplodedNode *ErrNode = C.generateNonFatalErrorNode(); + ExplodedNode *ErrNode = C.generateErrorNode(); if (!ErrNode) return false; llvm::SmallString<128> Str; From eecbe9d3eab99cc3b8101ecc05216454c3c95174 Mon Sep 17 00:00:00 2001 From: Gabor Spaits <gaborspai...@gmail.com> Date: Thu, 4 Jan 2024 20:23:38 +0100 Subject: [PATCH 15/18] Remove redundant escapes --- clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp | 8 ++++---- clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp index 62adf0ab4eac3b..87c8e8020d6117 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp @@ -179,10 +179,10 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { std::string StoredTypeName = StoredCanonicalType.getAsString(); std::string RetrievedTypeName = RetrievedCanonicalType.getAsString(); OS << "std::any " << AnyMemRegion->getDescriptiveName() << " held " - << indefiniteArticleBasedOnVowel(StoredTypeName[0]) << " \'" - << StoredTypeName << "\', not " - << indefiniteArticleBasedOnVowel(RetrievedTypeName[0]) << " \'" - << RetrievedTypeName << "\'"; + << indefiniteArticleBasedOnVowel(StoredTypeName[0]) << " '" + << StoredTypeName << "', not " + << indefiniteArticleBasedOnVowel(RetrievedTypeName[0]) << " '" + << RetrievedTypeName << "'"; auto R = std::make_unique<PathSensitiveBugReport>(BadAnyType, OS.str(), ErrNode); C.emitReport(std::move(R)); diff --git a/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp index 1157b4f854963b..30bd6aeb1612ea 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp @@ -277,10 +277,10 @@ class StdVariantChecker : public Checker<eval::Call, check::RegionChanges> { std::string StoredTypeName = StoredType->getAsString(); std::string RetrievedTypeName = RetrievedType.getAsString(); OS << "std::variant " << ArgMemRegion->getDescriptiveName() << " held " - << indefiniteArticleBasedOnVowel(StoredTypeName[0]) << " \'" - << StoredTypeName << "\', not " - << indefiniteArticleBasedOnVowel(RetrievedTypeName[0]) << " \'" - << RetrievedTypeName << "\'"; + << indefiniteArticleBasedOnVowel(StoredTypeName[0]) << " '" + << StoredTypeName << "', not " + << indefiniteArticleBasedOnVowel(RetrievedTypeName[0]) << " '" + << RetrievedTypeName << "'"; auto R = std::make_unique<PathSensitiveBugReport>(BadVariantType, OS.str(), ErrNode); C.emitReport(std::move(R)); From eb179836b91370e0ee2704d4175d8ac5e133847f Mon Sep 17 00:00:00 2001 From: Gabor Spaits <gaborspai...@gmail.com> Date: Thu, 4 Jan 2024 20:27:28 +0100 Subject: [PATCH 16/18] Use get instead of asArray()[0] --- clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp | 2 +- clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp index 87c8e8020d6117..4c4ef2a38eeaa2 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp @@ -142,7 +142,7 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { return false; const auto &FirstTemplateArgument = - FD->getTemplateSpecializationArgs()->asArray()[0]; + FD->getTemplateSpecializationArgs()->get(0); if (FirstTemplateArgument.getKind() != TemplateArgument::ArgKind::Type) return false; diff --git a/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp index 30bd6aeb1612ea..182b91439166be 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp @@ -240,7 +240,7 @@ class StdVariantChecker : public Checker<eval::Call, check::RegionChanges> { if (FD->getTemplateSpecializationArgs()->size() < 1) return false; - const auto &TypeOut = FD->getTemplateSpecializationArgs()->asArray()[0]; + const auto &TypeOut = FD->getTemplateSpecializationArgs()->get(0); // std::get's first template parameter can be the type we want to get // out of the std::variant or a natural number which is the position of // the requested type in the argument type list of the std::variant's From a2569cd47574657ad919251c9ecbe6557cb2ce86 Mon Sep 17 00:00:00 2001 From: Gabor Spaits <gaborspai...@gmail.com> Date: Thu, 4 Jan 2024 20:34:05 +0100 Subject: [PATCH 17/18] Do note use State->contains() unnecessarily --- clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp index 4c4ef2a38eeaa2..aa3a8587ca13d3 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp @@ -132,7 +132,8 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { const auto *AnyMemRegion = ArgSVal.getAsRegion(); - if (!State->contains<AnyHeldTypeMap>(AnyMemRegion)) + const auto *TypeStored = State->get<AnyHeldTypeMap>(AnyMemRegion); + if (!TypeStored) return false; // Get the type we are trying to retrieve from any. @@ -147,7 +148,6 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { return false; auto TypeOut = FirstTemplateArgument.getAsType(); - const auto *TypeStored = State->get<AnyHeldTypeMap>(AnyMemRegion); // Report when we try to use std::any_cast on a std::any that held a null // type. From b62d0d105866dfcc9daa06095252fb24bf60aaff Mon Sep 17 00:00:00 2001 From: Gabor Spaits <gaborspai...@gmail.com> Date: Thu, 4 Jan 2024 20:43:05 +0100 Subject: [PATCH 18/18] Move handlig of any.reset to a handler --- .../StaticAnalyzer/Checkers/StdAnyChecker.cpp | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp index aa3a8587ca13d3..55154443403108 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdAnyChecker.cpp @@ -55,18 +55,8 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { if (AnyCast.matches(Call)) return handleAnyCastCall(Call, C); - if (AnyReset.matches(Call)) { - const auto *AsMemberCall = dyn_cast<CXXMemberCall>(&Call); - if (!AsMemberCall) - return false; - - const auto *ThisMemRegion = AsMemberCall->getCXXThisVal().getAsRegion(); - if (!ThisMemRegion) - return false; - - C.addTransition(setNullTypeAny(ThisMemRegion, C)); - return true; - } + if (AnyReset.matches(Call)) + return handleResetCall(Call, C); bool IsAnyConstructor = isa<CXXConstructorCall>(Call) && AnyConstructor.matches(Call); @@ -188,6 +178,19 @@ class StdAnyChecker : public Checker<eval::Call, check::RegionChanges> { C.emitReport(std::move(R)); return true; } + + bool handleResetCall(const CallEvent &Call, CheckerContext &C) const { + const auto *AsMemberCall = dyn_cast<CXXMemberCall>(&Call); + if (!AsMemberCall) + return false; + + const auto *ThisMemRegion = AsMemberCall->getCXXThisVal().getAsRegion(); + if (!ThisMemRegion) + return false; + + C.addTransition(setNullTypeAny(ThisMemRegion, C)); + return true; + } }; bool clang::ento::shouldRegisterStdAnyChecker( _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits