[clang] 4868825 - [analyzer] Model comparision methods of std::unique_ptr
Deep Majumder via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 15 21:25:06 PDT 2021
Author: Deep Majumder
Date: 2021-07-16T09:54:05+05:30
New Revision: 48688257c52dfc2c666b64730f0467c2cc38210c
URL: https://github.com/llvm/llvm-project/commit/48688257c52dfc2c666b64730f0467c2cc38210c
DIFF: https://github.com/llvm/llvm-project/commit/48688257c52dfc2c666b64730f0467c2cc38210c.diff
LOG: [analyzer] Model comparision methods of std::unique_ptr
This patch handles all the comparision methods (defined via overloaded
operators) on std::unique_ptr. These operators compare the underlying
pointers, which is modelled by comparing the corresponding inner-pointer
SVal. There is also a special case for comparing the same pointer.
Differential Revision: https://reviews.llvm.org/D104616
Added:
Modified:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
clang/test/Analysis/Inputs/system-header-simulator-cxx.h
clang/test/Analysis/smart-ptr.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
index f253c14cc4870..a81d67ab30639 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
@@ -13,7 +13,9 @@
#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
+#include "clang/AST/OperationKinds.h"
#include "clang/AST/Stmt.h"
+#include "clang/Basic/OperatorKinds.h"
#include "llvm/ADT/Optional.h"
#include <tuple>
@@ -69,6 +71,45 @@ Nullability getNullabilityAnnotation(QualType Type);
/// token for an integer. If we cannot parse the value then None is returned.
llvm::Optional<int> tryExpandAsInteger(StringRef Macro, const Preprocessor &PP);
+class OperatorKind {
+ union {
+ BinaryOperatorKind Bin;
+ UnaryOperatorKind Un;
+ } Op;
+ bool IsBinary;
+
+public:
+ explicit OperatorKind(BinaryOperatorKind Bin) : Op{Bin}, IsBinary{true} {}
+ explicit OperatorKind(UnaryOperatorKind Un) : IsBinary{false} { Op.Un = Un; }
+ bool IsBinaryOp() const { return IsBinary; }
+
+ BinaryOperatorKind GetBinaryOpUnsafe() const {
+ assert(IsBinary && "cannot get binary operator - we have a unary operator");
+ return Op.Bin;
+ }
+
+ Optional<BinaryOperatorKind> GetBinaryOp() const {
+ if (IsBinary)
+ return Op.Bin;
+ return {};
+ }
+
+ UnaryOperatorKind GetUnaryOpUnsafe() const {
+ assert(!IsBinary &&
+ "cannot get unary operator - we have a binary operator");
+ return Op.Un;
+ }
+
+ Optional<UnaryOperatorKind> GetUnaryOp() const {
+ if (!IsBinary)
+ return Op.Un;
+ return {};
+ }
+};
+
+OperatorKind operationKindFromOverloadedOperator(OverloadedOperatorKind OOK,
+ bool IsBinary);
+
} // namespace ento
} // namespace clang
diff --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtr.h b/clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
index 92c386bbb2b09..b4352b450c7fa 100644
--- a/clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
+++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
@@ -22,6 +22,8 @@ namespace smartptr {
/// Returns true if the event call is on smart pointer.
bool isStdSmartPtrCall(const CallEvent &Call);
+bool isStdSmartPtr(const CXXRecordDecl *RD);
+bool isStdSmartPtr(const Expr *E);
/// Returns whether the smart pointer is null or not.
bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegion);
diff --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
index 6ee7bd9252b33..cfc61e89a40d2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -25,10 +25,13 @@
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/Support/ErrorHandling.h"
#include <string>
using namespace clang;
@@ -68,6 +71,11 @@ class SmartPtrModeling
bool updateMovedSmartPointers(CheckerContext &C, const MemRegion *ThisRegion,
const MemRegion *OtherSmartPtrRegion) const;
void handleBoolConversion(const CallEvent &Call, CheckerContext &C) const;
+ bool handleComparisionOp(const CallEvent &Call, CheckerContext &C) const;
+ std::pair<SVal, ProgramStateRef>
+ retrieveOrConjureInnerPtrVal(ProgramStateRef State,
+ const MemRegion *ThisRegion, const Expr *E,
+ QualType Type, CheckerContext &C) const;
using SmartPtrMethodHandlerFn =
void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const;
@@ -89,18 +97,24 @@ bool isStdSmartPtrCall(const CallEvent &Call) {
const auto *MethodDecl = dyn_cast_or_null<CXXMethodDecl>(Call.getDecl());
if (!MethodDecl || !MethodDecl->getParent())
return false;
+ return isStdSmartPtr(MethodDecl->getParent());
+}
- const auto *RecordDecl = MethodDecl->getParent();
- if (!RecordDecl || !RecordDecl->getDeclContext()->isStdNamespace())
+bool isStdSmartPtr(const CXXRecordDecl *RD) {
+ if (!RD || !RD->getDeclContext()->isStdNamespace())
return false;
- if (RecordDecl->getDeclName().isIdentifier()) {
- StringRef Name = RecordDecl->getName();
+ if (RD->getDeclName().isIdentifier()) {
+ StringRef Name = RD->getName();
return Name == "shared_ptr" || Name == "unique_ptr" || Name == "weak_ptr";
}
return false;
}
+bool isStdSmartPtr(const Expr *E) {
+ return isStdSmartPtr(E->getType()->getAsCXXRecordDecl());
+}
+
bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegion) {
const auto *InnerPointVal = State->get<TrackedRegionMap>(ThisRegion);
return InnerPointVal &&
@@ -135,18 +149,11 @@ static ProgramStateRef updateSwappedRegion(ProgramStateRef State,
return State;
}
-// Helper method to get the inner pointer type of specialized smart pointer
-// Returns empty type if not found valid inner pointer type.
-static QualType getInnerPointerType(const CallEvent &Call, CheckerContext &C) {
- const auto *MethodDecl = dyn_cast_or_null<CXXMethodDecl>(Call.getDecl());
- if (!MethodDecl || !MethodDecl->getParent())
- return {};
-
- const auto *RecordDecl = MethodDecl->getParent();
- if (!RecordDecl || !RecordDecl->isInStdNamespace())
+static QualType getInnerPointerType(CheckerContext C, const CXXRecordDecl *RD) {
+ if (!RD || !RD->isInStdNamespace())
return {};
- const auto *TSD = dyn_cast<ClassTemplateSpecializationDecl>(RecordDecl);
+ const auto *TSD = dyn_cast<ClassTemplateSpecializationDecl>(RD);
if (!TSD)
return {};
@@ -157,6 +164,17 @@ static QualType getInnerPointerType(const CallEvent &Call, CheckerContext &C) {
return C.getASTContext().getPointerType(InnerValueType.getCanonicalType());
}
+// Helper method to get the inner pointer type of specialized smart pointer
+// Returns empty type if not found valid inner pointer type.
+static QualType getInnerPointerType(const CallEvent &Call, CheckerContext &C) {
+ const auto *MethodDecl = dyn_cast_or_null<CXXMethodDecl>(Call.getDecl());
+ if (!MethodDecl || !MethodDecl->getParent())
+ return {};
+
+ const auto *RecordDecl = MethodDecl->getParent();
+ return getInnerPointerType(C, RecordDecl);
+}
+
// Helper method to pretty print region and avoid extra spacing.
static void checkAndPrettyPrintRegion(llvm::raw_ostream &OS,
const MemRegion *Region) {
@@ -178,6 +196,16 @@ bool SmartPtrModeling::isBoolConversionMethod(const CallEvent &Call) const {
bool SmartPtrModeling::evalCall(const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
+
+ // If any one of the arg is a unique_ptr, then
+ // we can try this function
+ if (Call.getNumArgs() == 2 &&
+ Call.getDecl()->getDeclContext()->isStdNamespace())
+ if (smartptr::isStdSmartPtr(Call.getArgExpr(0)) ||
+ smartptr::isStdSmartPtr(Call.getArgExpr(1)))
+ if (handleComparisionOp(Call, C))
+ return true;
+
if (!smartptr::isStdSmartPtrCall(Call))
return false;
@@ -272,6 +300,84 @@ bool SmartPtrModeling::evalCall(const CallEvent &Call,
return C.isDifferent();
}
+std::pair<SVal, ProgramStateRef> SmartPtrModeling::retrieveOrConjureInnerPtrVal(
+ ProgramStateRef State, const MemRegion *ThisRegion, const Expr *E,
+ QualType Type, CheckerContext &C) const {
+ const auto *Ptr = State->get<TrackedRegionMap>(ThisRegion);
+ if (Ptr)
+ return {*Ptr, State};
+ auto Val = C.getSValBuilder().conjureSymbolVal(E, C.getLocationContext(),
+ Type, C.blockCount());
+ State = State->set<TrackedRegionMap>(ThisRegion, Val);
+ return {Val, State};
+}
+
+bool SmartPtrModeling::handleComparisionOp(const CallEvent &Call,
+ CheckerContext &C) const {
+ const auto *FC = dyn_cast<SimpleFunctionCall>(&Call);
+ if (!FC)
+ return false;
+ const FunctionDecl *FD = FC->getDecl();
+ if (!FD->isOverloadedOperator())
+ return false;
+ const OverloadedOperatorKind OOK = FD->getOverloadedOperator();
+ if (!(OOK == OO_EqualEqual || OOK == OO_ExclaimEqual || OOK == OO_Less ||
+ OOK == OO_LessEqual || OOK == OO_Greater || OOK == OO_GreaterEqual ||
+ OOK == OO_Spaceship))
+ return false;
+
+ // There are some special cases about which we can infer about
+ // the resulting answer.
+ // For reference, there is a discussion at https://reviews.llvm.org/D104616.
+ // Also, the cppreference page is good to look at
+ // https://en.cppreference.com/w/cpp/memory/unique_ptr/operator_cmp.
+
+ auto makeSValFor = [&C, this](ProgramStateRef State, const Expr *E,
+ SVal S) -> std::pair<SVal, ProgramStateRef> {
+ if (S.isZeroConstant()) {
+ return {S, State};
+ }
+ const MemRegion *Reg = S.getAsRegion();
+ assert(Reg &&
+ "this pointer of std::unique_ptr should be obtainable as MemRegion");
+ QualType Type = getInnerPointerType(C, E->getType()->getAsCXXRecordDecl());
+ return retrieveOrConjureInnerPtrVal(State, Reg, E, Type, C);
+ };
+
+ SVal First = Call.getArgSVal(0);
+ SVal Second = Call.getArgSVal(1);
+ const auto *FirstExpr = Call.getArgExpr(0);
+ const auto *SecondExpr = Call.getArgExpr(1);
+
+ const auto *ResultExpr = Call.getOriginExpr();
+ const auto *LCtx = C.getLocationContext();
+ auto &Bldr = C.getSValBuilder();
+ ProgramStateRef State = C.getState();
+
+ SVal FirstPtrVal, SecondPtrVal;
+ std::tie(FirstPtrVal, State) = makeSValFor(State, FirstExpr, First);
+ std::tie(SecondPtrVal, State) = makeSValFor(State, SecondExpr, Second);
+ BinaryOperatorKind BOK =
+ operationKindFromOverloadedOperator(OOK, true).GetBinaryOpUnsafe();
+ auto RetVal = Bldr.evalBinOp(State, BOK, FirstPtrVal, SecondPtrVal,
+ Call.getResultType());
+
+ if (OOK != OO_Spaceship) {
+ ProgramStateRef TrueState, FalseState;
+ std::tie(TrueState, FalseState) =
+ State->assume(*RetVal.getAs<DefinedOrUnknownSVal>());
+ if (TrueState)
+ C.addTransition(
+ TrueState->BindExpr(ResultExpr, LCtx, Bldr.makeTruthVal(true)));
+ if (FalseState)
+ C.addTransition(
+ FalseState->BindExpr(ResultExpr, LCtx, Bldr.makeTruthVal(false)));
+ } else {
+ C.addTransition(State->BindExpr(ResultExpr, LCtx, RetVal));
+ }
+ return true;
+}
+
void SmartPtrModeling::checkDeadSymbols(SymbolReaper &SymReaper,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
@@ -446,15 +552,8 @@ void SmartPtrModeling::handleGet(const CallEvent &Call,
return;
SVal InnerPointerVal;
- if (const auto *InnerValPtr = State->get<TrackedRegionMap>(ThisRegion)) {
- InnerPointerVal = *InnerValPtr;
- } else {
- const auto *CallExpr = Call.getOriginExpr();
- InnerPointerVal = C.getSValBuilder().conjureSymbolVal(
- CallExpr, C.getLocationContext(), Call.getResultType(), C.blockCount());
- State = State->set<TrackedRegionMap>(ThisRegion, InnerPointerVal);
- }
-
+ std::tie(InnerPointerVal, State) = retrieveOrConjureInnerPtrVal(
+ State, ThisRegion, Call.getOriginExpr(), Call.getResultType(), C);
State = State->BindExpr(Call.getOriginExpr(), C.getLocationContext(),
InnerPointerVal);
// TODO: Add NoteTag, for how the raw pointer got using 'get' method.
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp b/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
index cae728815b412..626ae1ae80663 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
@@ -148,5 +148,39 @@ llvm::Optional<int> tryExpandAsInteger(StringRef Macro,
return IntValue.getSExtValue();
}
+OperatorKind operationKindFromOverloadedOperator(OverloadedOperatorKind OOK,
+ bool IsBinary) {
+ llvm::StringMap<BinaryOperatorKind> BinOps{
+#define BINARY_OPERATION(Name, Spelling) {Spelling, BO_##Name},
+#include "clang/AST/OperationKinds.def"
+ };
+ llvm::StringMap<UnaryOperatorKind> UnOps{
+#define UNARY_OPERATION(Name, Spelling) {Spelling, UO_##Name},
+#include "clang/AST/OperationKinds.def"
+ };
+
+ switch (OOK) {
+#define OVERLOADED_OPERATOR(Name, Spelling, Token, Unary, Binary, MemberOnly) \
+ case OO_##Name: \
+ if (IsBinary) { \
+ auto BinOpIt = BinOps.find(Spelling); \
+ if (BinOpIt != BinOps.end()) \
+ return OperatorKind(BinOpIt->second); \
+ else \
+ llvm_unreachable("operator was expected to be binary but is not"); \
+ } else { \
+ auto UnOpIt = UnOps.find(Spelling); \
+ if (UnOpIt != UnOps.end()) \
+ return OperatorKind(UnOpIt->second); \
+ else \
+ llvm_unreachable("operator was expected to be unary but is not"); \
+ } \
+ break;
+#include "clang/Basic/OperatorKinds.def"
+ default:
+ llvm_unreachable("unexpected operator kind");
+ }
+}
+
} // namespace ento
} // namespace clang
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
index 87984d02c2f6e..e80c6039f9e2a 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -978,6 +978,61 @@ template <typename T>
void swap(unique_ptr<T> &x, unique_ptr<T> &y) noexcept {
x.swap(y);
}
+
+template <typename T1, typename T2>
+bool operator==(const unique_ptr<T1> &x, const unique_ptr<T2> &y);
+
+template <typename T1, typename T2>
+bool operator!=(const unique_ptr<T1> &x, const unique_ptr<T2> &y);
+
+template <typename T1, typename T2>
+bool operator<(const unique_ptr<T1> &x, const unique_ptr<T2> &y);
+
+template <typename T1, typename T2>
+bool operator>(const unique_ptr<T1> &x, const unique_ptr<T2> &y);
+
+template <typename T1, typename T2>
+bool operator<=(const unique_ptr<T1> &x, const unique_ptr<T2> &y);
+
+template <typename T1, typename T2>
+bool operator>=(const unique_ptr<T1> &x, const unique_ptr<T2> &y);
+
+template <typename T>
+bool operator==(const unique_ptr<T> &x, nullptr_t y);
+
+template <typename T>
+bool operator!=(const unique_ptr<T> &x, nullptr_t y);
+
+template <typename T>
+bool operator<(const unique_ptr<T> &x, nullptr_t y);
+
+template <typename T>
+bool operator>(const unique_ptr<T> &x, nullptr_t y);
+
+template <typename T>
+bool operator<=(const unique_ptr<T> &x, nullptr_t y);
+
+template <typename T>
+bool operator>=(const unique_ptr<T> &x, nullptr_t y);
+
+template <typename T>
+bool operator==(nullptr_t x, const unique_ptr<T> &y);
+
+template <typename T>
+bool operator!=(nullptr_t x, const unique_ptr<T> &y);
+
+template <typename T>
+bool operator>(nullptr_t x, const unique_ptr<T> &y);
+
+template <typename T>
+bool operator<(nullptr_t x, const unique_ptr<T> &y);
+
+template <typename T>
+bool operator>=(nullptr_t x, const unique_ptr<T> &y);
+
+template <typename T>
+bool operator<=(nullptr_t x, const unique_ptr<T> &y);
+
} // namespace std
#endif
diff --git a/clang/test/Analysis/smart-ptr.cpp b/clang/test/Analysis/smart-ptr.cpp
index 7761ac4cb4316..ab2e49d720ee5 100644
--- a/clang/test/Analysis/smart-ptr.cpp
+++ b/clang/test/Analysis/smart-ptr.cpp
@@ -457,3 +457,52 @@ void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr<A> P) {
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
}
}
+
+// The following is a silly function,
+// but serves to test if we are picking out
+// standard comparision functions from custom ones.
+template <typename T>
+bool operator<(std::unique_ptr<T> &x, double d);
+
+void uniquePtrComparision(std::unique_ptr<int> unknownPtr) {
+ auto ptr = std::unique_ptr<int>(new int(13));
+ auto nullPtr = std::unique_ptr<int>();
+ auto otherPtr = std::unique_ptr<int>(new int(29));
+
+ clang_analyzer_eval(ptr == ptr); // expected-warning{{TRUE}}
+ clang_analyzer_eval(ptr > ptr); // expected-warning{{FALSE}}
+ clang_analyzer_eval(ptr <= ptr); // expected-warning{{TRUE}}
+
+ clang_analyzer_eval(nullPtr <= unknownPtr); // expected-warning{{TRUE}}
+ clang_analyzer_eval(unknownPtr >= nullPtr); // expected-warning{{TRUE}}
+
+ clang_analyzer_eval(ptr != otherPtr); // expected-warning{{TRUE}}
+ clang_analyzer_eval(ptr > nullPtr); // expected-warning{{TRUE}}
+
+ clang_analyzer_eval(ptr != nullptr); // expected-warning{{TRUE}}
+ clang_analyzer_eval(nullPtr != nullptr); // expected-warning{{FALSE}}
+ clang_analyzer_eval(nullptr <= unknownPtr); // expected-warning{{TRUE}}
+}
+
+void uniquePtrComparisionStateSplitting(std::unique_ptr<int> unknownPtr) {
+ auto ptr = std::unique_ptr<int>(new int(13));
+
+ clang_analyzer_eval(ptr > unknownPtr); // expected-warning{{TRUE}}
+ // expected-warning at -1{{FALSE}}
+}
+
+void uniquePtrComparisionDifferingTypes(std::unique_ptr<int> unknownPtr) {
+ auto ptr = std::unique_ptr<int>(new int(13));
+ auto nullPtr = std::unique_ptr<A>();
+ auto otherPtr = std::unique_ptr<double>(new double(3.14));
+
+ clang_analyzer_eval(nullPtr <= unknownPtr); // expected-warning{{TRUE}}
+ clang_analyzer_eval(unknownPtr >= nullPtr); // expected-warning{{TRUE}}
+
+ clang_analyzer_eval(ptr != otherPtr); // expected-warning{{TRUE}}
+ clang_analyzer_eval(ptr > nullPtr); // expected-warning{{TRUE}}
+
+ clang_analyzer_eval(ptr != nullptr); // expected-warning{{TRUE}}
+ clang_analyzer_eval(nullPtr != nullptr); // expected-warning{{FALSE}}
+ clang_analyzer_eval(nullptr <= unknownPtr); // expected-warning{{TRUE}}
+}
More information about the cfe-commits
mailing list