RedDocMD updated this revision to Diff 353068.
RedDocMD added a comment.
Some more refactoring
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104300/new/
https://reviews.llvm.org/D104300
Files:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
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
Index: clang/test/Analysis/smart-ptr-text-output.cpp
===================================================================
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -69,20 +69,17 @@
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@-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@Inputs/system-header-simulator-cxx.h:979 {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
- // expected-note@-1 {{Calling 'swap<A>'}}
- // expected-note@-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@-1{{Dereference of null smart pointer 'P'}}
}
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2261,6 +2261,14 @@
markInteresting(meta->getRegion(), TKind);
}
+void PathSensitiveBugReport::markNotInteresting(SymbolRef sym) {
+ if (!sym)
+ return;
+ InterestingSymbols.erase(sym);
+ if (const auto *meta = dyn_cast<SymbolMetadata>(sym))
+ markNotInteresting(meta->getRegion());
+}
+
void PathSensitiveBugReport::markInteresting(const MemRegion *R,
bugreporter::TrackingKind TKind) {
if (!R)
@@ -2273,6 +2281,17 @@
markInteresting(SR->getSymbol(), TKind);
}
+void PathSensitiveBugReport::markNotInteresting(const MemRegion *R) {
+ if (!R)
+ return;
+
+ R = R->getBaseRegion();
+ InterestingRegions.erase(R);
+
+ if (const auto *SR = dyn_cast<SymbolicRegion>(R))
+ markNotInteresting(SR->getSymbol());
+}
+
void PathSensitiveBugReport::markInteresting(SVal V,
bugreporter::TrackingKind TKind) {
markInteresting(V.getAsRegion(), TKind);
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -60,7 +60,7 @@
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,
@@ -68,14 +68,17 @@
bool updateMovedSmartPointers(CheckerContext &C, const MemRegion *ThisRegion,
const MemRegion *OtherSmartPtrRegion) const;
void handleBoolConversion(const CallEvent &Call, CheckerContext &C) const;
+ bool handleSwap(ProgramStateRef State, SVal First, SVal Second,
+ CheckerContext &C) const;
using SmartPtrMethodHandlerFn =
void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const;
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
@@ -91,11 +94,15 @@
return false;
const auto *RecordDecl = MethodDecl->getParent();
- if (!RecordDecl || !RecordDecl->getDeclContext()->isStdNamespace())
+ return isStdSmartPtr(RecordDecl);
+}
+
+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;
@@ -178,6 +185,16 @@
bool SmartPtrModeling::evalCall(const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
+
+ 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;
@@ -395,43 +412,52 @@
// 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,
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
@@ -23,6 +23,8 @@
/// Returns true if the event call is on smart pointer.
bool isStdSmartPtrCall(const CallEvent &Call);
+bool isStdSmartPtr(const CXXRecordDecl *RD);
+
/// Returns whether the smart pointer is null or not.
bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegion);
Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -432,6 +432,8 @@
void markInteresting(SymbolRef sym, bugreporter::TrackingKind TKind =
bugreporter::TrackingKind::Thorough);
+ void markNotInteresting(SymbolRef sym);
+
/// Marks a region as interesting. Different kinds of interestingness will
/// be processed differently by visitors (e.g. if the tracking kind is
/// condition, will append "will be used as a condition" to the message).
@@ -439,6 +441,8 @@
const MemRegion *R,
bugreporter::TrackingKind TKind = bugreporter::TrackingKind::Thorough);
+ void markNotInteresting(const MemRegion *R);
+
/// Marks a symbolic value as interesting. Different kinds of interestingness
/// will be processed differently by visitors (e.g. if the tracking kind is
/// condition, will append "will be used as a condition" to the message).
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
