https://github.com/spaits created https://github.com/llvm/llvm-project/pull/87886
None From b9d22d9ebc152ac0bccc7e196f8bbecc9302d833 Mon Sep 17 00:00:00 2001 From: Gabor Spaits <gaborspai...@gmail.com> Date: Sat, 13 Jan 2024 21:06:52 +0100 Subject: [PATCH 1/2] [analyzer] Implement modeling of std::swap and bind to SVal to std::get call --- .../Checkers/StdVariantChecker.cpp | 105 ++++++++++++++---- .../Checkers/TaggedUnionModeling.h | 80 ++++++++++--- .../Checkers/UndefinedAssignmentChecker.cpp | 8 +- .../Inputs/system-header-simulator-cxx.h | 3 + clang/test/Analysis/std-variant-checker.cpp | 30 ++++- 5 files changed, 185 insertions(+), 41 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp index f7b7befe28ee7d..f86aea6032f205 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp @@ -6,6 +6,8 @@ // //===----------------------------------------------------------------------===// +#include "clang/AST/CXXInheritance.h" +#include "clang/AST/Expr.h" #include "clang/AST/Type.h" #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" @@ -14,12 +16,16 @@ #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/MemRegion.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h" #include "llvm/ADT/FoldingSet.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Casting.h" +#include "llvm/Support/ErrorHandling.h" +#include "llvm/Support/raw_ostream.h" +#include <cassert> #include <optional> -#include <string_view> #include "TaggedUnionModeling.h" @@ -27,7 +33,7 @@ using namespace clang; using namespace ento; using namespace tagged_union_modeling; -REGISTER_MAP_WITH_PROGRAMSTATE(VariantHeldTypeMap, const MemRegion *, QualType) +REGISTER_MAP_WITH_PROGRAMSTATE(VariantHeldMap, const MemRegion *, SVal) namespace clang::ento::tagged_union_modeling { @@ -132,8 +138,11 @@ class StdVariantChecker : public Checker<eval::Call, check::RegionChanges> { CallDescription VariantConstructor{{"std", "variant", "variant"}}; CallDescription VariantAssignmentOperator{{"std", "variant", "operator="}}; CallDescription StdGet{{"std", "get"}, 1, 1}; + CallDescription StdSwap{{"std", "swap"}, 2}; + CallDescription StdEmplace{{"std", "variant", "emplace"}}; - BugType BadVariantType{this, "BadVariantType", "BadVariantType"}; + BugType BadVariantType{ + this, "The active type of std::variant differs from the accessed."}; public: ProgramStateRef checkRegionChanges(ProgramStateRef State, @@ -145,8 +154,8 @@ class StdVariantChecker : public Checker<eval::Call, check::RegionChanges> { if (!Call) return State; - return removeInformationStoredForDeadInstances<VariantHeldTypeMap>( - *Call, State, Regions); + return removeInformationStoredForDeadInstances<VariantHeldMap>(*Call, State, + Regions); } bool evalCall(const CallEvent &Call, CheckerContext &C) const { @@ -158,6 +167,13 @@ class StdVariantChecker : public Checker<eval::Call, check::RegionChanges> { if (StdGet.matches(Call)) return handleStdGetCall(Call, C); + if (StdSwap.matches(Call)) + return handleStdSwapCall<VariantHeldMap>(Call, C); + + // TODO Implement the modeling of std::variants emplace method. + if (StdEmplace.matches(Call)) + return handleStdVariantEmplaceCall(Call, C); + // First check if a constructor call is happening. If it is a // constructor call, check if it is an std::variant constructor call. bool IsVariantConstructor = @@ -188,16 +204,15 @@ class StdVariantChecker : public Checker<eval::Call, check::RegionChanges> { return false; } - handleConstructorAndAssignment<VariantHeldTypeMap>(Call, C, ThisSVal); - return true; + return handleConstructorAndAssignment<VariantHeldMap>(Call, C, ThisSVal); } return false; } private: - // The default constructed std::variant must be handled separately - // by default the std::variant is going to hold a default constructed instance - // of the first type of the possible types + // The default constructed std::variant must be handled separately. + // When an std::variant instance is default constructed it holds + // a value-initialized value of the first type alternative. void handleDefaultConstructor(const CXXConstructorCall *ConstructorCall, CheckerContext &C) const { SVal ThisSVal = ConstructorCall->getCXXThisVal(); @@ -206,23 +221,42 @@ class StdVariantChecker : public Checker<eval::Call, check::RegionChanges> { if (!ThisMemRegion) return; + // Get the first type alternative of the std::variant instance. + assert((ThisSVal.getType(C.getASTContext())->isPointerType() || + ThisSVal.getType(C.getASTContext())->isReferenceType()) && + "The This SVal must be a pointer!"); + std::optional<QualType> DefaultType = getNthTemplateTypeArgFromVariant( ThisSVal.getType(C.getASTContext())->getPointeeType().getTypePtr(), 0); if (!DefaultType) return; + // We conjure a symbol that represents the value-initialized value held by + // the default constructed std::variant. This could be made more precise + // if we would actually simulate the value-initialization of the value. + // + // We are storing pointer/reference typed SVals because when an + // std::variant is constructed with a value constructor a reference is + // received. The SVal representing this parameter will also have reference + // type. We use this SVal to store information about the value held is an + // std::variant instance. Here we are conforming to this and also use + // reference type. Also if we would not use reference typed SVals + // the analyzer would crash when handling the cast expression with the + // reason that the SVal is a NonLoc SVal. + SVal DefaultConstructedHeldValue = C.getSValBuilder().conjureSymbolVal( + ConstructorCall->getOriginExpr(), C.getLocationContext(), + C.getASTContext().getLValueReferenceType(*DefaultType), C.blockCount()); + ProgramStateRef State = ConstructorCall->getState(); - State = State->set<VariantHeldTypeMap>(ThisMemRegion, *DefaultType); + State = + State->set<VariantHeldMap>(ThisMemRegion, DefaultConstructedHeldValue); C.addTransition(State); } bool handleStdGetCall(const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = Call.getState(); - const auto &ArgType = Call.getArgSVal(0) - .getType(C.getASTContext()) - ->getPointeeType() - .getTypePtr(); + const auto &ArgType = Call.getArgExpr(0)->getType().getTypePtr(); // We have to make sure that the argument is an std::variant. // There is another std::get with std::pair argument if (!isStdVariant(ArgType)) @@ -231,16 +265,22 @@ class StdVariantChecker : public Checker<eval::Call, check::RegionChanges> { // Get the mem region of the argument std::variant and look up the type // information that we know about it. const MemRegion *ArgMemRegion = Call.getArgSVal(0).getAsRegion(); - const QualType *StoredType = State->get<VariantHeldTypeMap>(ArgMemRegion); - if (!StoredType) + const SVal *StoredSVal = State->get<VariantHeldMap>(ArgMemRegion); + if (!StoredSVal) + return false; + + QualType RefStoredType = StoredSVal->getType(C.getASTContext()); + + if (RefStoredType->getPointeeType().isNull()) return false; + QualType StoredType = RefStoredType->getPointeeType(); const CallExpr *CE = cast<CallExpr>(Call.getOriginExpr()); const FunctionDecl *FD = CE->getDirectCallee(); 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 @@ -265,16 +305,22 @@ class StdVariantChecker : public Checker<eval::Call, check::RegionChanges> { } QualType RetrievedCanonicalType = RetrievedType.getCanonicalType(); - QualType StoredCanonicalType = StoredType->getCanonicalType(); - if (RetrievedCanonicalType == StoredCanonicalType) + QualType StoredCanonicalType = StoredType.getCanonicalType(); + if (RetrievedCanonicalType.isNull() || StoredType.isNull()) + return false; + + if (RetrievedCanonicalType == StoredCanonicalType) { + State = State->BindExpr(CE, C.getLocationContext(), *StoredSVal); + C.addTransition(State); return true; + } - ExplodedNode *ErrNode = C.generateNonFatalErrorNode(); + ExplodedNode *ErrNode = C.generateErrorNode(); if (!ErrNode) return false; llvm::SmallString<128> Str; llvm::raw_svector_ostream OS(Str); - std::string StoredTypeName = StoredType->getAsString(); + std::string StoredTypeName = StoredType.getAsString(); std::string RetrievedTypeName = RetrievedType.getAsString(); OS << "std::variant " << ArgMemRegion->getDescriptiveName() << " held " << indefiniteArticleBasedOnVowel(StoredTypeName[0]) << " \'" @@ -286,6 +332,21 @@ class StdVariantChecker : public Checker<eval::Call, check::RegionChanges> { C.emitReport(std::move(R)); return true; } + + // TODO Implement modeling of std::variant's emplace method. + // Currently when this method call is encountered we just + // stop the modeling of that std::variant instance. + bool handleStdVariantEmplaceCall(const CallEvent &Call, + CheckerContext &C) const { + const auto *AsMemberCall = dyn_cast_or_null<CXXMemberCall>(&Call); + if (!AsMemberCall) + return false; + const MemRegion *ThisRegion = AsMemberCall->getCXXThisVal().getAsRegion(); + if (!ThisRegion) + return false; + C.addTransition(C.getState()->remove<VariantHeldMap>(ThisRegion)); + return true; + } }; bool clang::ento::shouldRegisterStdVariantChecker( diff --git a/clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h b/clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h index 6de33da107a3f9..1f07d4b6bba9c5 100644 --- a/clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h +++ b/clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h @@ -9,15 +9,11 @@ #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> +#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h" +#include <vector> namespace clang::ento::tagged_union_modeling { @@ -30,6 +26,7 @@ bool isMoveAssignmentCall(const CallEvent &Call); bool isMoveConstructorCall(const CallEvent &Call); bool isStdType(const Type *Type, const std::string &TypeName); bool isStdVariant(const Type *Type); +void printSVals(std::vector<SVal> v); // When invalidating regions, we also have to follow that by invalidating the // corresponding custom data in the program state. @@ -51,27 +48,29 @@ removeInformationStoredForDeadInstances(const CallEvent &Call, } template <class TypeMap> -void handleConstructorAndAssignment(const CallEvent &Call, CheckerContext &C, +bool handleConstructorAndAssignment(const CallEvent &Call, CheckerContext &C, SVal ThisSVal) { ProgramStateRef State = Call.getState(); if (!State) - return; + return false; auto ArgSVal = Call.getArgSVal(0); const auto *ThisRegion = ThisSVal.getAsRegion(); const auto *ArgMemRegion = ArgSVal.getAsRegion(); + if (!ArgMemRegion) + return false; // Make changes to the state according to type of constructor/assignment bool IsCopy = isCopyConstructorCall(Call) || isCopyAssignmentCall(Call); bool IsMove = isMoveConstructorCall(Call) || isMoveAssignmentCall(Call); // First we handle copy and move operations if (IsCopy || IsMove) { - const QualType *OtherQType = State->get<TypeMap>(ArgMemRegion); - + // const QualType *OtherQType = State->get<TypeMap>(ArgMemRegion); + const SVal *OtherSVal = State->get<TypeMap>(ArgMemRegion); // If the argument of a copy constructor or assignment is unknown then // we will not know the argument of the copied to object. - if (!OtherQType) { + if (!OtherSVal) { State = State->remove<TypeMap>(ThisRegion); } else { // When move semantics is used we can only know that the moved from @@ -80,18 +79,65 @@ void handleConstructorAndAssignment(const CallEvent &Call, CheckerContext &C, if (IsMove) State = State->remove<TypeMap>(ArgMemRegion); - State = State->set<TypeMap>(ThisRegion, *OtherQType); + State = State->set<TypeMap>(ThisRegion, *OtherSVal); } } else { - // Value constructor - auto ArgQType = ArgSVal.getType(C.getASTContext()); - const Type *ArgTypePtr = ArgQType.getTypePtr(); + // Value constructor. + // Here we create a MemRegion and an SVal for the value held by + // std::variant, since std::variant owns the held value. + // + // If we don't do this here later on UndefinedAssignmentChecker will warn + // for garbage value assignment. + const auto *Mreg = ArgSVal.getAsRegion(); + SVal PointedToSVal = C.getState()->getSVal(Mreg); + + auto SymForStdVariantHeldValue = C.getSValBuilder().conjureSymbolVal( + "std::variant held value", Call.getArgExpr(0), C.getLocationContext(), + C.blockCount()); + auto *SymRegForStdVariantHeldValue = + C.getSValBuilder().getRegionManager().getSymbolicRegion( + SymForStdVariantHeldValue.getAsSymbol()); + auto LocForVariantHeldValue = + C.getSValBuilder().makeLoc(SymRegForStdVariantHeldValue); + State = State->bindLoc(LocForVariantHeldValue, PointedToSVal, + C.getLocationContext()); + State = State->set<TypeMap>(ThisRegion, LocForVariantHeldValue); + } + + C.addTransition(State); + return true; +} + +template <class HeldValueMap> +bool handleStdSwapCall(const CallEvent &Call, CheckerContext &C) { + assert(Call.getNumArgs() == 2 && + "This function only handles std::swap with two arguments."); + + for (unsigned i = 0; i < Call.getNumArgs(); i++) { + if (!isStdVariant(Call.getArgExpr(i)->getType().getTypePtr())) + return false; + } + + ProgramStateRef State = C.getState(); + + const MemRegion *LeftRegion = Call.getArgSVal(0).getAsRegion(); + const MemRegion *RightRegion = Call.getArgSVal(1).getAsRegion(); + if (!LeftRegion || !RightRegion) + return false; - QualType WoPointer = ArgTypePtr->getPointeeType(); - State = State->set<TypeMap>(ThisRegion, WoPointer); + const SVal *LeftSVal = State->get<HeldValueMap>(LeftRegion); + const SVal *RightSVal = State->get<HeldValueMap>(RightRegion); + if (!LeftSVal || !RightSVal) { + State = State->remove<HeldValueMap>(LeftRegion); + State = State->remove<HeldValueMap>(RightRegion); + C.addTransition(State); + return false; } + State = State->set<HeldValueMap>(LeftRegion, *RightSVal); + State = State->set<HeldValueMap>(RightRegion, *LeftSVal); C.addTransition(State); + return true; } } // namespace clang::ento::tagged_union_modeling diff --git a/clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp index ddc6cc9e8202c7..bb503c2db8d3b7 100644 --- a/clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp @@ -16,6 +16,7 @@ #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "llvm/Support/raw_ostream.h" using namespace clang; using namespace ento; @@ -101,13 +102,18 @@ void UndefinedAssignmentChecker::checkBind(SVal location, SVal val, if (OS.str().empty()) OS << BT.getDescription(); - auto R = std::make_unique<PathSensitiveBugReport>(BT, OS.str(), N); if (ex) { R->addRange(ex->getSourceRange()); bugreporter::trackExpressionValue(N, ex, *R); } C.emitReport(std::move(R)); + // llvm::errs() << "Undef checker reports to loc: \n"; + // location.dump(); + // llvm::errs() << "\nand Val:\n"; + // val.dump(); + // //C.getState()->dump(); + // llvm::errs() << "\n"; } void ento::registerUndefinedAssignmentChecker(CheckerManager &mgr) { diff --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h index 3ef7af2ea6c6ab..dac96e3a0dcc4d 100644 --- a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h +++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h @@ -1354,6 +1354,7 @@ template <typename Ret, typename... Args> class packaged_task<Ret(Args...)> { constexpr add_pointer_t<T> get_if(variant<Types...>*) noexcept; template <class T, class... Types> constexpr add_pointer_t<const T> get_if(const variant<Types...>*) noexcept; + template <class... Types> class variant { @@ -1371,6 +1372,8 @@ template <typename Ret, typename... Args> class packaged_task<Ret(Args...)> { template<typename T, typename = std::enable_if_t<!is_same_v<std::variant<Types...>, decay_t<T>>>> variant& operator=(T&&); + template<class T, class... Args> + constexpr T& emplace(Args&&...); }; #endif diff --git a/clang/test/Analysis/std-variant-checker.cpp b/clang/test/Analysis/std-variant-checker.cpp index 7f136c06b19cc6..3234c82d27eaa2 100644 --- a/clang/test/Analysis/std-variant-checker.cpp +++ b/clang/test/Analysis/std-variant-checker.cpp @@ -81,6 +81,7 @@ void stdGetObject() { Foo f = std::get<Foo>(v); int i = std::get<int>(v); // expected-warning {{std::variant 'v' held a 'Foo', not an 'int'}} (void)i; + (void)f; } void stdGetPointerAndPointee() { @@ -355,4 +356,31 @@ void nonInlineFunctionCallPtr() { char c = std::get<char> (v); // no-warning (void)a; (void)c; -} \ No newline at end of file +} + +//----------------------------------------------------------------------------// +// std::swap for std::variant +//----------------------------------------------------------------------------// + +void swapForVariants() { + std::variant<int, char> a = 5; + std::variant<int, char> b = 'C'; + std::swap(a, b); + int a1 = std::get<int>(b); + char c = std::get<int>(a); // expected-warning {{std::variant 'a' held a 'char', not an 'int'}} + (void)a1; + (void)c; +} + +//----------------------------------------------------------------------------// +// std::swap for std::variant +//----------------------------------------------------------------------------// + +void stdEmplace() { + std::variant<int, char> v = 'c'; + v.emplace<int> (5); + int a = std::get<int> (v); // no-warning + char c = std::get<char> (v); // no-warning + (void)a; + (void)c; +} From b250e2db91515b33c427bfa1b9faf6f376010bf0 Mon Sep 17 00:00:00 2001 From: Gabor Spaits <gaborspai...@gmail.com> Date: Sat, 6 Apr 2024 20:11:14 +0200 Subject: [PATCH 2/2] Fix a bug with bindings and eval calls Some test still fail with similar reasons before the bug fix. See: + /home/spaits/repo/llvm-project/build/bin/clang /home/spaits/repo/llvm-project/clang/test/Analysis/std-variant-checker.cpp -std=c++17 -Xclang -verify --analyze -Xclang -analyzer-checker=core -Xclang -analyzer-checker=debug.ExprInspection -Xclang -analyzer-checker=core,alpha.core.StdVariant error: 'expected-warning' diagnostics expected but not seen: File /home/spaits/repo/llvm-project/clang/test/Analysis/std-variant-checker.cpp Line 146: std::variant 'v' held a 'float', not an 'int' error: 'expected-warning' diagnostics seen but not expected: File /home/spaits/repo/llvm-project/clang/test/Analysis/std-variant-checker.cpp Line 142: Assigned value is garbage or undefined [core.uninitialized.Assign] File /home/spaits/repo/llvm-project/clang/test/Analysis/std-variant-checker.cpp Line 391: The right operand of '/' is a garbage value [core.UndefinedBinaryOperatorResult] 3 errors generated. --- .../Checkers/TaggedUnionModeling.h | 27 +-------- .../Checkers/UndefinedAssignmentChecker.cpp | 6 -- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 2 + clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp | 56 +++++++++++++++++-- clang/test/Analysis/std-variant-checker.cpp | 7 +++ 5 files changed, 64 insertions(+), 34 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h b/clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h index 1f07d4b6bba9c5..c70e3a452129db 100644 --- a/clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h +++ b/clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h @@ -13,7 +13,7 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h" -#include <vector> +#include "llvm/Support/raw_ostream.h" namespace clang::ento::tagged_union_modeling { @@ -26,7 +26,6 @@ bool isMoveAssignmentCall(const CallEvent &Call); bool isMoveConstructorCall(const CallEvent &Call); bool isStdType(const Type *Type, const std::string &TypeName); bool isStdVariant(const Type *Type); -void printSVals(std::vector<SVal> v); // When invalidating regions, we also have to follow that by invalidating the // corresponding custom data in the program state. @@ -81,28 +80,8 @@ bool handleConstructorAndAssignment(const CallEvent &Call, CheckerContext &C, State = State->set<TypeMap>(ThisRegion, *OtherSVal); } - } else { - // Value constructor. - // Here we create a MemRegion and an SVal for the value held by - // std::variant, since std::variant owns the held value. - // - // If we don't do this here later on UndefinedAssignmentChecker will warn - // for garbage value assignment. - const auto *Mreg = ArgSVal.getAsRegion(); - SVal PointedToSVal = C.getState()->getSVal(Mreg); - - auto SymForStdVariantHeldValue = C.getSValBuilder().conjureSymbolVal( - "std::variant held value", Call.getArgExpr(0), C.getLocationContext(), - C.blockCount()); - auto *SymRegForStdVariantHeldValue = - C.getSValBuilder().getRegionManager().getSymbolicRegion( - SymForStdVariantHeldValue.getAsSymbol()); - auto LocForVariantHeldValue = - C.getSValBuilder().makeLoc(SymRegForStdVariantHeldValue); - State = State->bindLoc(LocForVariantHeldValue, PointedToSVal, - C.getLocationContext()); - State = State->set<TypeMap>(ThisRegion, LocForVariantHeldValue); - } + } else + State = State->set<TypeMap>(ThisRegion, ArgSVal); C.addTransition(State); return true; diff --git a/clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp index bb503c2db8d3b7..39dda07bf7c8d1 100644 --- a/clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp @@ -108,12 +108,6 @@ void UndefinedAssignmentChecker::checkBind(SVal location, SVal val, bugreporter::trackExpressionValue(N, ex, *R); } C.emitReport(std::move(R)); - // llvm::errs() << "Undef checker reports to loc: \n"; - // location.dump(); - // llvm::errs() << "\nand Val:\n"; - // val.dump(); - // //C.getState()->dump(); - // llvm::errs() << "\n"; } void ento::registerUndefinedAssignmentChecker(CheckerManager &mgr) { diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp index 24e91a22fd6884..be02b4849a6bbf 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -2261,6 +2261,8 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, Bldr.takeNodes(Pred); const auto *C = cast<CastExpr>(S); ExplodedNodeSet dstExpr; + // Point of interes: maybe my cast somehow will not get visited or will + // be visited before I bind to it. VisitCast(C, C->getSubExpr(), Pred, dstExpr); // Handle the postvisit checks. diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp index 7e431f7e598c4c..5bdbc0e755c4d5 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -11,9 +11,15 @@ //===----------------------------------------------------------------------===// #include "clang/AST/DeclCXX.h" +#include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h" +#include "llvm/Support/Casting.h" +#include "llvm/Support/raw_ostream.h" #include <optional> using namespace clang; @@ -37,6 +43,19 @@ static SVal conjureOffsetSymbolOnLocation( return Symbol; } +// Update the SVal bound to the Cast expression with the SVal +// bound to the casted expression +static ProgramStateRef updateStateAfterSimpleCast(StmtNodeBuilder& Bldr, + ExplodedNode *Pred, + const CastExpr *CastE, + const Expr *CastedE) { + ProgramStateRef state = Pred->getState(); + const LocationContext *LCtx = Pred->getLocationContext(); + SVal V = state->getSVal(CastedE, LCtx); + return state->BindExpr(CastE, LCtx, V); + //Bldr.generateNode(CastE, Pred, state); +} + void ExprEngine::VisitBinaryOperator(const BinaryOperator* B, ExplodedNode *Pred, ExplodedNodeSet &Dst) { @@ -332,10 +351,8 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex, case CK_FunctionToPointerDecay: case CK_BuiltinFnToFnPtr: { // Copy the SVal of Ex to CastE. - ProgramStateRef state = Pred->getState(); - const LocationContext *LCtx = Pred->getLocationContext(); - SVal V = state->getSVal(Ex, LCtx); - state = state->BindExpr(CastE, LCtx, V); + // Point of interest + state = updateStateAfterSimpleCast(Bldr, Pred, CastE, Ex); Bldr.generateNode(CastE, Pred, state); continue; } @@ -602,6 +619,37 @@ void ExprEngine::VisitDeclStmt(const DeclStmt *DS, ExplodedNode *Pred, ExplodedNode *UpdatedN = N; SVal InitVal = state->getSVal(InitEx, LC); + // The call expression to which we have bound something is hidden behind + // an implicit cast expression. + + // This is a workaround for those checkers that are evaluating calls + // with return value, and are "behind" a cast expression. A good example + // for this is std::variant checker. + // Let's see the following code as an example: + // + // int a = std::get<int>(v); + // + // The AST for the std::get call shall look something like this: + // + // ImplicitCastExpr <FunctionToPointerDecay> + // `-CallExpr + // + // First the handling of `ImplicitCastExpr <FunctionToPointerDecay>` + // happens in the ExprEngine::VisitCast function. After that + // std::variant checker evaluates the std::get call and binds + // an SVal to the call expression. The problem here is that + // the handling of the casting is the responsible to bind the + // sub expressions (in our case std::get call expressions) value + // to the cast expression. + if (auto *AsImplCast = dyn_cast_or_null<CastExpr>(InitEx); + AsImplCast && InitVal.isUndef()) { + // InitVal = state->getSVal(AsImplCast->getSubExpr(), LC); + state = updateStateAfterSimpleCast(B, Pred, AsImplCast, + AsImplCast->getSubExpr()); + B.generateNode(InitEx, Pred, state); + InitVal = state->getSVal(InitEx, LC); + } + assert(DS->isSingleDecl()); if (getObjectUnderConstruction(state, DS, LC)) { state = finishObjectConstruction(state, DS, LC); diff --git a/clang/test/Analysis/std-variant-checker.cpp b/clang/test/Analysis/std-variant-checker.cpp index 3234c82d27eaa2..4f6b5fb124bf8a 100644 --- a/clang/test/Analysis/std-variant-checker.cpp +++ b/clang/test/Analysis/std-variant-checker.cpp @@ -384,3 +384,10 @@ void stdEmplace() { (void)a; (void)c; } + +void followHeldValue() { + std::variant<int, char> v = 0; + int a = std::get<int> (v); + int b = 5/a; + (void)b; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits