[clang] 0cd98be - [analyzer] Handle std::swap for std::unique_ptr
Deep Majumder via cfe-commits
cfe-commits at lists.llvm.org
Sun Jul 18 02:09:55 PDT 2021
Author: Deep Majumder
Date: 2021-07-18T14:38:55+05:30
New Revision: 0cd98bef1b6feec067a4c60df4df4d44a842811d
URL: https://github.com/llvm/llvm-project/commit/0cd98bef1b6feec067a4c60df4df4d44a842811d
DIFF: https://github.com/llvm/llvm-project/commit/0cd98bef1b6feec067a4c60df4df4d44a842811d.diff
LOG: [analyzer] Handle std::swap for std::unique_ptr
This patch handles the `std::swap` function specialization
for `std::unique_ptr`. Implemented to be very similar to
how `swap` method is handled
Differential Revision: https://reviews.llvm.org/D104300
Added:
Modified:
clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
clang/lib/StaticAnalyzer/Core/BugReporter.cpp
clang/test/Analysis/smart-ptr-text-output.cpp
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtr.h b/clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
index b4352b450c7fa..6a40f8eda5fa8 100644
--- a/clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
+++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
@@ -25,6 +25,8 @@ bool isStdSmartPtrCall(const CallEvent &Call);
bool isStdSmartPtr(const CXXRecordDecl *RD);
bool isStdSmartPtr(const Expr *E);
+bool isStdSmartPtr(const CXXRecordDecl *RD);
+
/// 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 b56c969ca5a43..2793980de1b60 100644
--- a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -63,7 +63,7 @@ class SmartPtrModeling
private:
void handleReset(const CallEvent &Call, CheckerContext &C) const;
void handleRelease(const CallEvent &Call, CheckerContext &C) const;
- void handleSwap(const CallEvent &Call, CheckerContext &C) const;
+ void handleSwapMethod(const CallEvent &Call, CheckerContext &C) const;
void handleGet(const CallEvent &Call, CheckerContext &C) const;
bool handleAssignOp(const CallEvent &Call, CheckerContext &C) const;
bool handleMoveCtr(const CallEvent &Call, CheckerContext &C,
@@ -73,6 +73,8 @@ class SmartPtrModeling
void handleBoolConversion(const CallEvent &Call, CheckerContext &C) const;
bool handleComparisionOp(const CallEvent &Call, CheckerContext &C) const;
bool handleOstreamOperator(const CallEvent &Call, CheckerContext &C) const;
+ bool handleSwap(ProgramStateRef State, SVal First, SVal Second,
+ CheckerContext &C) const;
std::pair<SVal, ProgramStateRef>
retrieveOrConjureInnerPtrVal(ProgramStateRef State,
const MemRegion *ThisRegion, const Expr *E,
@@ -83,8 +85,9 @@ class SmartPtrModeling
CallDescriptionMap<SmartPtrMethodHandlerFn> SmartPtrMethodHandlers{
{{"reset"}, &SmartPtrModeling::handleReset},
{{"release"}, &SmartPtrModeling::handleRelease},
- {{"swap", 1}, &SmartPtrModeling::handleSwap},
+ {{"swap", 1}, &SmartPtrModeling::handleSwapMethod},
{{"get"}, &SmartPtrModeling::handleGet}};
+ const CallDescription StdSwapCall{{"std", "swap"}, 2};
};
} // end of anonymous namespace
@@ -259,6 +262,15 @@ bool SmartPtrModeling::evalCall(const CallEvent &Call,
if (isStdOstreamOperatorCall(Call))
return handleOstreamOperator(Call, C);
+ if (Call.isCalled(StdSwapCall)) {
+ // Check the first arg, if it is of std::unique_ptr type.
+ assert(Call.getNumArgs() == 2 && "std::swap should have two arguments");
+ const Expr *FirstArg = Call.getArgExpr(0);
+ if (!smartptr::isStdSmartPtr(FirstArg->getType()->getAsCXXRecordDecl()))
+ return false;
+ return handleSwap(State, Call.getArgSVal(0), Call.getArgSVal(1), C);
+ }
+
if (!smartptr::isStdSmartPtrCall(Call))
return false;
@@ -578,43 +590,52 @@ void SmartPtrModeling::handleRelease(const CallEvent &Call,
// pointer.
}
-void SmartPtrModeling::handleSwap(const CallEvent &Call,
- CheckerContext &C) const {
+void SmartPtrModeling::handleSwapMethod(const CallEvent &Call,
+ CheckerContext &C) const {
// To model unique_ptr::swap() method.
const auto *IC = dyn_cast<CXXInstanceCall>(&Call);
if (!IC)
return;
- const MemRegion *ThisRegion = IC->getCXXThisVal().getAsRegion();
- if (!ThisRegion)
- return;
+ auto State = C.getState();
+ handleSwap(State, IC->getCXXThisVal(), Call.getArgSVal(0), C);
+}
- const auto *ArgRegion = Call.getArgSVal(0).getAsRegion();
- if (!ArgRegion)
- return;
+bool SmartPtrModeling::handleSwap(ProgramStateRef State, SVal First,
+ SVal Second, CheckerContext &C) const {
+ const MemRegion *FirstThisRegion = First.getAsRegion();
+ if (!FirstThisRegion)
+ return false;
+ const MemRegion *SecondThisRegion = Second.getAsRegion();
+ if (!SecondThisRegion)
+ return false;
- auto State = C.getState();
- const auto *ThisRegionInnerPointerVal =
- State->get<TrackedRegionMap>(ThisRegion);
- const auto *ArgRegionInnerPointerVal =
- State->get<TrackedRegionMap>(ArgRegion);
+ const auto *FirstInnerPtrVal = State->get<TrackedRegionMap>(FirstThisRegion);
+ const auto *SecondInnerPtrVal =
+ State->get<TrackedRegionMap>(SecondThisRegion);
- // Swap the tracked region values.
- State = updateSwappedRegion(State, ThisRegion, ArgRegionInnerPointerVal);
- State = updateSwappedRegion(State, ArgRegion, ThisRegionInnerPointerVal);
+ State = updateSwappedRegion(State, FirstThisRegion, SecondInnerPtrVal);
+ State = updateSwappedRegion(State, SecondThisRegion, FirstInnerPtrVal);
- C.addTransition(
- State, C.getNoteTag([ThisRegion, ArgRegion](PathSensitiveBugReport &BR,
- llvm::raw_ostream &OS) {
- if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
- !BR.isInteresting(ThisRegion))
- return;
- BR.markInteresting(ArgRegion);
- OS << "Swapped null smart pointer";
- checkAndPrettyPrintRegion(OS, ArgRegion);
- OS << " with smart pointer";
- checkAndPrettyPrintRegion(OS, ThisRegion);
- }));
+ C.addTransition(State, C.getNoteTag([FirstThisRegion, SecondThisRegion](
+ PathSensitiveBugReport &BR,
+ llvm::raw_ostream &OS) {
+ if (&BR.getBugType() != smartptr::getNullDereferenceBugType())
+ return;
+ if (BR.isInteresting(FirstThisRegion) &&
+ !BR.isInteresting(SecondThisRegion)) {
+ BR.markInteresting(SecondThisRegion);
+ BR.markNotInteresting(FirstThisRegion);
+ }
+ if (BR.isInteresting(SecondThisRegion) &&
+ !BR.isInteresting(FirstThisRegion)) {
+ BR.markInteresting(FirstThisRegion);
+ BR.markNotInteresting(SecondThisRegion);
+ }
+ // TODO: We need to emit some note here probably!!
+ }));
+
+ return true;
}
void SmartPtrModeling::handleGet(const CallEvent &Call,
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
index cf4581e8f67b9..d6f69ae03afe5 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2263,6 +2263,8 @@ void PathSensitiveBugReport::markNotInteresting(SymbolRef sym) {
// The metadata part of markInteresting is not reversed here.
// Just making the same region not interesting is incorrect
// in specific cases.
+ if (const auto *meta = dyn_cast<SymbolMetadata>(sym))
+ markNotInteresting(meta->getRegion());
}
void PathSensitiveBugReport::markInteresting(const MemRegion *R,
diff --git a/clang/test/Analysis/smart-ptr-text-output.cpp b/clang/test/Analysis/smart-ptr-text-output.cpp
index f8ecf9192c736..299fed883b3fc 100644
--- a/clang/test/Analysis/smart-ptr-text-output.cpp
+++ b/clang/test/Analysis/smart-ptr-text-output.cpp
@@ -69,20 +69,17 @@ void derefOnReleasedNullRawPtr() {
void derefOnSwappedNullPtr() {
std::unique_ptr<A> P(new A()); // expected-note {{Smart pointer 'P' is constructed}}
- std::unique_ptr<A> PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
- P.swap(PNull); // expected-note {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
+ std::unique_ptr<A> PNull;
+ P.swap(PNull);
PNull->foo(); // No warning.
(*P).foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
// expected-note at -1{{Dereference of null smart pointer 'P'}}
}
-// FIXME: Fix this test when "std::swap" is modeled seperately.
void derefOnStdSwappedNullPtr() {
std::unique_ptr<A> P; // expected-note {{Default constructed smart pointer 'P' is null}}
- std::unique_ptr<A> PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
- std::swap(P, PNull); // expected-note at Inputs/system-header-simulator-cxx.h:979 {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
- // expected-note at -1 {{Calling 'swap<A>'}}
- // expected-note at -2 {{Returning from 'swap<A>'}}
+ std::unique_ptr<A> PNull;
+ std::swap(P, PNull);
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
// expected-note at -1{{Dereference of null smart pointer 'P'}}
}
More information about the cfe-commits
mailing list