[clang] Variant checker bindings (PR #87886)
Gábor Spaits via cfe-commits
cfe-commits at lists.llvm.org
Sat Apr 6 11:17:01 PDT 2024
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 <gaborspaits1 at 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 <gaborspaits1 at 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;
+}
More information about the cfe-commits
mailing list