[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbc3d4d9ed783: [analyzer] Add bool operator modeling for
unque_ptr (authored by vrnithinkumar).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
Files:
clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
clang/test/Analysis/smart-ptr-text-output.cpp
clang/test/Analysis/smart-ptr.cpp
Index: clang/test/Analysis/smart-ptr.cpp
===
--- clang/test/Analysis/smart-ptr.cpp
+++ clang/test/Analysis/smart-ptr.cpp
@@ -8,12 +8,13 @@
void clang_analyzer_warnIfReached();
void clang_analyzer_numTimesReached();
void clang_analyzer_eval(bool);
+void clang_analyzer_warnOnDeadSymbol(int *);
void derefAfterMove(std::unique_ptr P) {
std::unique_ptr Q = std::move(P);
if (Q)
clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
- *Q.get() = 1; // no-warning
+ *Q.get() = 1; // expected-warning {{Dereference of null pointer [core.NullDereference]}}
if (P)
clang_analyzer_warnIfReached(); // no-warning
// TODO: Report a null dereference (instead).
@@ -375,3 +376,77 @@
std::unique_ptr P(functionReturnsRValueRef());
P->foo(); // No warning.
}
+
+void derefConditionOnNullPtr() {
+ std::unique_ptr P;
+ if (P)
+P->foo(); // No warning.
+ else
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnNotNullPtr() {
+ std::unique_ptr P;
+ if (!P)
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnValidPtr() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull;
+ if (P)
+PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnNotValidPtr() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull;
+ if (!P)
+PNull->foo(); // No warning.
+}
+
+void derefConditionOnUnKnownPtr(std::unique_ptr P) {
+ if (P)
+P->foo(); // No warning.
+ else
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnValidPtrAfterReset(std::unique_ptr P) {
+ P.reset(new A());
+ if (!P)
+P->foo(); // No warning.
+ else
+P->foo(); // No warning.
+}
+
+void innerPointerSymbolLiveness() {
+ std::unique_ptr P(new int());
+ clang_analyzer_warnOnDeadSymbol(P.get());
+ int *RP = P.release();
+} // expected-warning{{SYMBOL DEAD}}
+
+void boolOpCreatedConjuredSymbolLiveness(std::unique_ptr P) {
+ if (P) {
+int *X = P.get();
+clang_analyzer_warnOnDeadSymbol(X);
+ }
+} // expected-warning{{SYMBOL DEAD}}
+
+void getCreatedConjuredSymbolLiveness(std::unique_ptr P) {
+ int *X = P.get();
+ clang_analyzer_warnOnDeadSymbol(X);
+ int Y;
+ if (!P) {
+Y = *P.get(); // expected-warning {{Dereference of null pointer [core.NullDereference]}}
+// expected-warning@-1 {{SYMBOL DEAD}}
+ }
+}
+
+int derefConditionOnUnKnownPtr(int *q) {
+ std::unique_ptr P(q);
+ if (P)
+return *P; // No warning.
+ else
+return *P; // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
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
@@ -199,3 +199,108 @@
PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}}
// expected-note@-1{{Dereference of null smart pointer 'PToMove'}}
}
+
+void derefConditionOnNullPtrFalseBranch() {
+ std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
+ if (P) { // expected-note {{Taking false branch}}
+P->foo(); // No warning.
+ } else {
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+ }
+}
+
+void derefConditionOnNullPtrTrueBranch() {
+ std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
+ if (!P) { // expected-note {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+ }
+}
+
+void derefConditionOnValidPtrTrueBranch() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
+ if (P) { // expected-note {{Taking true branch}}
+PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LG! Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86027/new/ https://reviews.llvm.org/D86027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
vrnithinkumar marked an inline comment as done. vrnithinkumar added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:556 +const LocationContext *LC = C.getLocationContext(); +InnerPointerVal = C.getSValBuilder().conjureSymbolVal( +CallExpr, LC, InnerPointerType, C.blockCount()); xazax.hun wrote: > Don't we want to actually add InnerPointerVal to TrackedRegionMap in this > case? > > I might be wrong but I cannot find where do we actually record the fact that > this freshly conjured symbol belongs to the unique_ptr we are modeling. Thanks for catching that. We have to update the `TrackedRegionMap` to track the created conjureSymbolVal `InnerPointerVal`. Updated to add it to the `TrackedRegionMap` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86027/new/ https://reviews.llvm.org/D86027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
vrnithinkumar updated this revision to Diff 288974.
vrnithinkumar added a comment.
- Fixing minor spacing issue
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
Files:
clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
clang/test/Analysis/smart-ptr-text-output.cpp
clang/test/Analysis/smart-ptr.cpp
Index: clang/test/Analysis/smart-ptr.cpp
===
--- clang/test/Analysis/smart-ptr.cpp
+++ clang/test/Analysis/smart-ptr.cpp
@@ -8,12 +8,13 @@
void clang_analyzer_warnIfReached();
void clang_analyzer_numTimesReached();
void clang_analyzer_eval(bool);
+void clang_analyzer_warnOnDeadSymbol(int *);
void derefAfterMove(std::unique_ptr P) {
std::unique_ptr Q = std::move(P);
if (Q)
clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
- *Q.get() = 1; // no-warning
+ *Q.get() = 1; // expected-warning {{Dereference of null pointer [core.NullDereference]}}
if (P)
clang_analyzer_warnIfReached(); // no-warning
// TODO: Report a null dereference (instead).
@@ -375,3 +376,77 @@
std::unique_ptr P(functionReturnsRValueRef());
P->foo(); // No warning.
}
+
+void derefConditionOnNullPtr() {
+ std::unique_ptr P;
+ if (P)
+P->foo(); // No warning.
+ else
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnNotNullPtr() {
+ std::unique_ptr P;
+ if (!P)
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnValidPtr() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull;
+ if (P)
+PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnNotValidPtr() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull;
+ if (!P)
+PNull->foo(); // No warning.
+}
+
+void derefConditionOnUnKnownPtr(std::unique_ptr P) {
+ if (P)
+P->foo(); // No warning.
+ else
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnValidPtrAfterReset(std::unique_ptr P) {
+ P.reset(new A());
+ if (!P)
+P->foo(); // No warning.
+ else
+P->foo(); // No warning.
+}
+
+void innerPointerSymbolLiveness() {
+ std::unique_ptr P(new int());
+ clang_analyzer_warnOnDeadSymbol(P.get());
+ int *RP = P.release();
+} // expected-warning{{SYMBOL DEAD}}
+
+void boolOpCreatedConjuredSymbolLiveness(std::unique_ptr P) {
+ if (P) {
+int *X = P.get();
+clang_analyzer_warnOnDeadSymbol(X);
+ }
+} // expected-warning{{SYMBOL DEAD}}
+
+void getCreatedConjuredSymbolLiveness(std::unique_ptr P) {
+ int *X = P.get();
+ clang_analyzer_warnOnDeadSymbol(X);
+ int Y;
+ if (!P) {
+Y = *P.get(); // expected-warning {{Dereference of null pointer [core.NullDereference]}}
+// expected-warning@-1 {{SYMBOL DEAD}}
+ }
+}
+
+int derefConditionOnUnKnownPtr(int *q) {
+ std::unique_ptr P(q);
+ if (P)
+return *P; // No warning.
+ else
+return *P; // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
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
@@ -199,3 +199,108 @@
PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}}
// expected-note@-1{{Dereference of null smart pointer 'PToMove'}}
}
+
+void derefConditionOnNullPtrFalseBranch() {
+ std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
+ if (P) { // expected-note {{Taking false branch}}
+P->foo(); // No warning.
+ } else {
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+ }
+}
+
+void derefConditionOnNullPtrTrueBranch() {
+ std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
+ if (!P) { // expected-note {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+ }
+}
+
+void derefConditionOnValidPtrTrueBranch() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
+ if (P) { // expected-note {{Taking true branch}}
+PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'PNull'}}
+ } else {
+PNull->foo(); // No warning
+ }
+}
+
+void derefConditionOnValidPtrFalseBranch() {
+
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
vrnithinkumar updated this revision to Diff 288972.
vrnithinkumar added a comment.
- Addressing review comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
Files:
clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
clang/test/Analysis/smart-ptr-text-output.cpp
clang/test/Analysis/smart-ptr.cpp
Index: clang/test/Analysis/smart-ptr.cpp
===
--- clang/test/Analysis/smart-ptr.cpp
+++ clang/test/Analysis/smart-ptr.cpp
@@ -8,12 +8,13 @@
void clang_analyzer_warnIfReached();
void clang_analyzer_numTimesReached();
void clang_analyzer_eval(bool);
+void clang_analyzer_warnOnDeadSymbol(int *);
void derefAfterMove(std::unique_ptr P) {
std::unique_ptr Q = std::move(P);
if (Q)
clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
- *Q.get() = 1; // no-warning
+ *Q.get() = 1; // expected-warning {{Dereference of null pointer [core.NullDereference]}}
if (P)
clang_analyzer_warnIfReached(); // no-warning
// TODO: Report a null dereference (instead).
@@ -375,3 +376,77 @@
std::unique_ptr P(functionReturnsRValueRef());
P->foo(); // No warning.
}
+
+void derefConditionOnNullPtr() {
+ std::unique_ptr P;
+ if (P)
+P->foo(); // No warning.
+ else
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnNotNullPtr() {
+ std::unique_ptr P;
+ if (!P)
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnValidPtr() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull;
+ if (P)
+PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnNotValidPtr() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull;
+ if (!P)
+PNull->foo(); // No warning.
+}
+
+void derefConditionOnUnKnownPtr(std::unique_ptr P) {
+ if (P)
+P->foo(); // No warning.
+ else
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnValidPtrAfterReset(std::unique_ptr P) {
+ P.reset(new A());
+ if (!P)
+P->foo(); // No warning.
+ else
+P->foo(); // No warning.
+}
+
+void innerPointerSymbolLiveness() {
+ std::unique_ptr P(new int());
+ clang_analyzer_warnOnDeadSymbol(P.get());
+ int *RP = P.release();
+} // expected-warning{{SYMBOL DEAD}}
+
+void boolOpCreatedConjuredSymbolLiveness(std::unique_ptr P) {
+ if (P) {
+int *X = P.get();
+clang_analyzer_warnOnDeadSymbol(X);
+ }
+} // expected-warning{{SYMBOL DEAD}}
+
+void getCreatedConjuredSymbolLiveness(std::unique_ptr P) {
+ int *X = P.get();
+ clang_analyzer_warnOnDeadSymbol(X);
+ int Y;
+ if (!P) {
+Y = *P.get(); // expected-warning {{Dereference of null pointer [core.NullDereference]}}
+// expected-warning@-1 {{SYMBOL DEAD}}
+ }
+}
+
+int derefConditionOnUnKnownPtr(int *q) {
+ std::unique_ptr P(q);
+ if (P)
+return *P; // No warning.
+ else
+return *P; // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
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
@@ -170,7 +170,6 @@
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
// expected-note@-1 {{Dereference of null smart pointer 'P'}}
}
-
void derefMoveConstructedWithNullPtr() {
std::unique_ptr PToMove; // expected-note {{Default constructed smart pointer 'PToMove' is null}}
std::unique_ptr P(std::move(PToMove)); // expected-note {{A null pointer value is moved to 'P'}}
@@ -199,3 +198,109 @@
PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}}
// expected-note@-1{{Dereference of null smart pointer 'PToMove'}}
}
+
+void derefConditionOnNullPtrFalseBranch() {
+ std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
+ if (P) { // expected-note {{Taking false branch}}
+P->foo(); // No warning.
+ } else {
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+ }
+}
+
+void derefConditionOnNullPtrTrueBranch() {
+ std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
+ if (!P) { // expected-note {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+ }
+}
+
+void derefConditionOnValidPtrTrueBranch() {
+ std::unique_ptr P(new A());
+
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
xazax.hun added inline comments. Herald added a subscriber: danielkiss. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:556 +const LocationContext *LC = C.getLocationContext(); +InnerPointerVal = C.getSValBuilder().conjureSymbolVal( +CallExpr, LC, InnerPointerType, C.blockCount()); Don't we want to actually add InnerPointerVal to TrackedRegionMap in this case? I might be wrong but I cannot find where do we actually record the fact that this freshly conjured symbol belongs to the unique_ptr we are modeling. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86027/new/ https://reviews.llvm.org/D86027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
NoQ accepted this revision. NoQ added a comment. I have no other concerns, i think this is good to go! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86027/new/ https://reviews.llvm.org/D86027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
vrnithinkumar marked 2 inline comments as done.
vrnithinkumar added inline comments.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:483
+llvm::raw_ostream ) {
+ BR.markInteresting(ThisRegion);
+ OS << "Smart pointer";
NoQ wrote:
> Wait, why are we marking the region interesting here? Not every null smart
> pointer is interesting, only the ones that are actually dereferenced.
This was added to show how the smart pointer, even though it is not
dereferenced. But why did we take true/false branch based on this.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:497-499
+ OS << "Smart pointer";
+ checkAndPrettyPrintRegion(OS, ThisRegion);
+ OS << " is non-null";
NoQ wrote:
> Also i'm starting to suspect that these notes aren't actually needed when
> there's no "Assuming..."; in particular, we don't emit such notes for raw
> pointers, so we probably shouldn't emit them for smart pointers either.
> There's anyway going to be a note about "Taking true branch..." or something
> like that, which is sufficient to understand what was the smart pointer.
>
> I.e., this note is unnecessary:
> ```lang=c++
> void foo() {
> std::unique_ptr P = nullptr;
> if (P) {// Smart pointer 'P' is null
> // Taking false branch
> }
> }
> ```
> The second note, "Taking false branch", is sufficient for explaining to the
> user that the smart pointer is known to be null. The user may ask why the
> smart pointer is null, but the note isn't explaining it. The way you marked
> the region as interesting (as i noticed in the above inline comment) would
> have indeed explained why it's null, but at this point we draw the line and
> believe that if the region isn't already interesting then the user most
> likely doesn't need to know why it's null (and if it's already interesting
> then there's no need to mark it again).
>
> But this note is necessary:
> ```lang=c++
> void foo(std::unique_ptr P) {
> if (P) {// Assuming smart pointer 'P' is null
> // Taking false branch
> }
> }
> ```
> This note conveys the extra information that we don't know for a fact that
> the smart pointer is null on the current path based on previous analysis, but
> instead the analyzer is *assuming* that it's possible that it's null due to
> the presence of the check in the code. That's a big deal.
Since the "Taking false branch" is enough to implies the smart pointer in null,
removing the notes which are unnecessary.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
vrnithinkumar updated this revision to Diff 288048.
vrnithinkumar marked an inline comment as done.
vrnithinkumar added a comment.
- Removing unnecessary notetags.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
Files:
clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
clang/test/Analysis/smart-ptr-text-output.cpp
clang/test/Analysis/smart-ptr.cpp
Index: clang/test/Analysis/smart-ptr.cpp
===
--- clang/test/Analysis/smart-ptr.cpp
+++ clang/test/Analysis/smart-ptr.cpp
@@ -8,12 +8,13 @@
void clang_analyzer_warnIfReached();
void clang_analyzer_numTimesReached();
void clang_analyzer_eval(bool);
+void clang_analyzer_warnOnDeadSymbol(int *);
void derefAfterMove(std::unique_ptr P) {
std::unique_ptr Q = std::move(P);
if (Q)
clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
- *Q.get() = 1; // no-warning
+ *Q.get() = 1; // expected-warning {{Dereference of null pointer [core.NullDereference]}}
if (P)
clang_analyzer_warnIfReached(); // no-warning
// TODO: Report a null dereference (instead).
@@ -333,3 +334,69 @@
P = returnRValRefOfUniquePtr();
P->foo(); // No warning.
}
+
+void derefConditionOnNullPtr() {
+ std::unique_ptr P;
+ if (P)
+P->foo(); // No warning.
+ else
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnNotNullPtr() {
+ std::unique_ptr P;
+ if (!P)
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnValidPtr() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull;
+ if (P)
+PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnNotValidPtr() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull;
+ if (!P)
+PNull->foo(); // No warning.
+}
+
+void derefConditionOnUnKnownPtr(std::unique_ptr P) {
+ if (P)
+P->foo(); // No warning.
+ else
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnValidPtrAfterReset(std::unique_ptr P) {
+ P.reset(new A());
+ if (!P)
+P->foo(); // No warning.
+ else
+P->foo(); // No warning.
+}
+
+void innerPointerSymbolLiveness() {
+ std::unique_ptr P(new int());
+ clang_analyzer_warnOnDeadSymbol(P.get());
+ int *RP = P.release();
+} // expected-warning{{SYMBOL DEAD}}
+
+void boolOpCreatedConjuredSymbolLiveness(std::unique_ptr P) {
+ if (P) {
+int *X = P.get();
+clang_analyzer_warnOnDeadSymbol(X);
+ }
+} // expected-warning{{SYMBOL DEAD}}
+
+void getCreatedConjuredSymbolLiveness(std::unique_ptr P) {
+ int *X = P.get();
+ clang_analyzer_warnOnDeadSymbol(X);
+ int Y;
+ if (!P) {
+Y = *P.get(); // expected-warning {{Dereference of null pointer [core.NullDereference]}}
+// expected-warning@-1 {{SYMBOL DEAD}}
+ }
+}
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
@@ -170,3 +170,108 @@
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
// expected-note@-1 {{Dereference of null smart pointer 'P'}}
}
+
+void derefConditionOnNullPtrFalseBranch() {
+ std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
+ if (P) { // expected-note {{Taking false branch}}
+P->foo(); // No warning.
+ } else {
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+ }
+}
+
+void derefConditionOnNullPtrTrueBranch() {
+ std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
+ if (!P) { // expected-note {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+ }
+}
+
+void derefConditionOnValidPtrTrueBranch() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
+ if (P) { // expected-note {{Taking true branch}}
+PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'PNull'}}
+ } else {
+PNull->foo(); // No warning
+ }
+}
+
+void derefConditionOnValidPtrFalseBranch() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
+ if (!P) { // expected-note {{Taking false branch}}
+PNull->foo(); // No warning
+ }
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
NoQ added inline comments.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:483
+llvm::raw_ostream ) {
+ BR.markInteresting(ThisRegion);
+ OS << "Smart pointer";
Wait, why are we marking the region interesting here? Not every null smart
pointer is interesting, only the ones that are actually dereferenced.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:497-499
+ OS << "Smart pointer";
+ checkAndPrettyPrintRegion(OS, ThisRegion);
+ OS << " is non-null";
Also i'm starting to suspect that these notes aren't actually needed when
there's no "Assuming..."; in particular, we don't emit such notes for raw
pointers, so we probably shouldn't emit them for smart pointers either. There's
anyway going to be a note about "Taking true branch..." or something like that,
which is sufficient to understand what was the smart pointer.
I.e., this note is unnecessary:
```lang=c++
void foo() {
std::unique_ptr P = nullptr;
if (P) {// Smart pointer 'P' is null
// Taking false branch
}
}
```
The second note, "Taking false branch", is sufficient for explaining to the
user that the smart pointer is known to be null. The user may ask why the smart
pointer is null, but the note isn't explaining it. The way you marked the
region as interesting (as i noticed in the above inline comment) would have
indeed explained why it's null, but at this point we draw the line and believe
that if the region isn't already interesting then the user most likely doesn't
need to know why it's null (and if it's already interesting then there's no
need to mark it again).
But this note is necessary:
```lang=c++
void foo(std::unique_ptr P) {
if (P) {// Assuming smart pointer 'P' is null
// Taking false branch
}
}
```
This note conveys the extra information that we don't know for a fact that the
smart pointer is null on the current path based on previous analysis, but
instead the analyzer is *assuming* that it's possible that it's null due to the
presence of the check in the code. That's a big deal.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
vrnithinkumar marked an inline comment as done.
vrnithinkumar added inline comments.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:494-496
+ OS << "Smart pointer";
+ checkAndPrettyPrintRegion(OS, ThisRegion);
+ OS << " is non-null";
NoQ wrote:
> Because these note tags aren't specific to the bug report at hand, they have
> to be marked as //prunable// (i.e., the optional `getNoteTag()`'s parameter
> "`IsPrunable`" should be set to true). That'd reduce bug report clutter by
> not bringing in stack frames that aren't otherwise interesting for the bug
> report.
>
> Something like this may act as a test (i didn't try):
> ```lang=c++
> struct S {
> std::unique_ptr P;
>
> void foo() {
> if (!P) { // no-note because foo() is pruned
> return;
> }
> }
>
> int bar() {
> P = new int(0);
> foo();
> return 1 / *P; // warning: division by zero
> }
>
> }
>
> ```
Marked these tags as prunable and added the tests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
vrnithinkumar updated this revision to Diff 287641.
vrnithinkumar added a comment.
- Making the note tags prunable
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
Files:
clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
clang/test/Analysis/smart-ptr-text-output.cpp
clang/test/Analysis/smart-ptr.cpp
Index: clang/test/Analysis/smart-ptr.cpp
===
--- clang/test/Analysis/smart-ptr.cpp
+++ clang/test/Analysis/smart-ptr.cpp
@@ -8,12 +8,13 @@
void clang_analyzer_warnIfReached();
void clang_analyzer_numTimesReached();
void clang_analyzer_eval(bool);
+void clang_analyzer_warnOnDeadSymbol(int *);
void derefAfterMove(std::unique_ptr P) {
std::unique_ptr Q = std::move(P);
if (Q)
clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
- *Q.get() = 1; // no-warning
+ *Q.get() = 1; // expected-warning {{Dereference of null pointer [core.NullDereference]}}
if (P)
clang_analyzer_warnIfReached(); // no-warning
// TODO: Report a null dereference (instead).
@@ -276,3 +277,69 @@
Y->foo(); // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}}
}
}
+
+void derefConditionOnNullPtr() {
+ std::unique_ptr P;
+ if (P)
+P->foo(); // No warning.
+ else
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnNotNullPtr() {
+ std::unique_ptr P;
+ if (!P)
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnValidPtr() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull;
+ if (P)
+PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnNotValidPtr() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull;
+ if (!P)
+PNull->foo(); // No warning.
+}
+
+void derefConditionOnUnKnownPtr(std::unique_ptr P) {
+ if (P)
+P->foo(); // No warning.
+ else
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnValidPtrAfterReset(std::unique_ptr P) {
+ P.reset(new A());
+ if (!P)
+P->foo(); // No warning.
+ else
+P->foo(); // No warning.
+}
+
+void innerPointerSymbolLiveness() {
+ std::unique_ptr P(new int());
+ clang_analyzer_warnOnDeadSymbol(P.get());
+ int *RP = P.release();
+} // expected-warning{{SYMBOL DEAD}}
+
+void boolOpCreatedConjuredSymbolLiveness(std::unique_ptr P) {
+ if (P) {
+int *X = P.get();
+clang_analyzer_warnOnDeadSymbol(X);
+ }
+} // expected-warning{{SYMBOL DEAD}}
+
+void getCreatedConjuredSymbolLiveness(std::unique_ptr P) {
+ int *X = P.get();
+ clang_analyzer_warnOnDeadSymbol(X);
+ int Y;
+ if (!P) {
+Y = *P.get(); // expected-warning {{Dereference of null pointer [core.NullDereference]}}
+// expected-warning@-1 {{SYMBOL DEAD}}
+ }
+}
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
@@ -131,3 +131,112 @@
void derefOnRawPtrFromGetOnUnknownPtr(std::unique_ptr P) {
P.get()->foo(); // No warning.
}
+
+void derefConditionOnNullPtrFalseBranch() {
+ std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
+ if (P) { // expected-note {{Taking false branch}}
+// expected-note@-1{{Smart pointer 'P' is nul}}
+P->foo(); // No warning.
+ } else {
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+ }
+}
+
+void derefConditionOnNullPtrTrueBranch() {
+ std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
+ if (!P) { // expected-note {{Taking true branch}}
+// expected-note@-1{{Smart pointer 'P' is nul}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+ }
+}
+
+void derefConditionOnValidPtrTrueBranch() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
+ if (P) { // expected-note {{Taking true branch}}
+// expected-note@-1{{Smart pointer 'P' is non-nul}}
+PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'PNull'}}
+ } else {
+PNull->foo(); // No warning
+ }
+}
+
+void derefConditionOnValidPtrFalseBranch() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
+ if (!P) { //
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
NoQ added inline comments.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:315-318
+SVal Val = I->second;
+for (auto si = Val.symbol_begin(), se = Val.symbol_end(); si != se; ++si) {
+ SR.markLive(*si);
+}
Yes, this looks correct, thanks!
In the ideal world we'd have checked that `I->first` is live before marking
`I->second` live. Unfortunately, currently `checkLiveSymbols` is invoked before
RegionStore's live symbols detection, so we can't rely on that and it's a bug;
in order to be correct, RegionStore's live symbols detection and checker's live
symbols callback should engage in fixpoint iteration.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:494-496
+ OS << "Smart pointer";
+ checkAndPrettyPrintRegion(OS, ThisRegion);
+ OS << " is non-null";
Because these note tags aren't specific to the bug report at hand, they have to
be marked as //prunable// (i.e., the optional `getNoteTag()`'s parameter
"`IsPrunable`" should be set to true). That'd reduce bug report clutter by not
bringing in stack frames that aren't otherwise interesting for the bug report.
Something like this may act as a test (i didn't try):
```lang=c++
struct S {
std::unique_ptr P;
void foo() {
if (!P) { // no-note because foo() is pruned
return;
}
}
int bar() {
P = new int(0);
foo();
return 1 / *P; // warning: division by zero
}
}
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
vrnithinkumar updated this revision to Diff 287512.
vrnithinkumar marked 11 inline comments as done.
vrnithinkumar added a comment.
- Adding checkLiveSymbols and review comments changes
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
Files:
clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
clang/test/Analysis/smart-ptr-text-output.cpp
clang/test/Analysis/smart-ptr.cpp
Index: clang/test/Analysis/smart-ptr.cpp
===
--- clang/test/Analysis/smart-ptr.cpp
+++ clang/test/Analysis/smart-ptr.cpp
@@ -8,12 +8,13 @@
void clang_analyzer_warnIfReached();
void clang_analyzer_numTimesReached();
void clang_analyzer_eval(bool);
+void clang_analyzer_warnOnDeadSymbol(int *);
void derefAfterMove(std::unique_ptr P) {
std::unique_ptr Q = std::move(P);
if (Q)
clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
- *Q.get() = 1; // no-warning
+ *Q.get() = 1; // expected-warning {{Dereference of null pointer [core.NullDereference]}}
if (P)
clang_analyzer_warnIfReached(); // no-warning
// TODO: Report a null dereference (instead).
@@ -276,3 +277,69 @@
Y->foo(); // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}}
}
}
+
+void derefConditionOnNullPtr() {
+ std::unique_ptr P;
+ if (P)
+P->foo(); // No warning.
+ else
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnNotNullPtr() {
+ std::unique_ptr P;
+ if (!P)
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnValidPtr() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull;
+ if (P)
+PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnNotValidPtr() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull;
+ if (!P)
+PNull->foo(); // No warning.
+}
+
+void derefConditionOnUnKnownPtr(std::unique_ptr P) {
+ if (P)
+P->foo(); // No warning.
+ else
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnValidPtrAfterReset(std::unique_ptr P) {
+ P.reset(new A());
+ if (!P)
+P->foo(); // No warning.
+ else
+P->foo(); // No warning.
+}
+
+void innerPointerSymbolLiveness() {
+ std::unique_ptr P(new int());
+ clang_analyzer_warnOnDeadSymbol(P.get());
+ int *RP = P.release();
+} // expected-warning{{SYMBOL DEAD}}
+
+void boolOpCreatedConjuredSymbolLiveness(std::unique_ptr P) {
+ if (P) {
+int *X = P.get();
+clang_analyzer_warnOnDeadSymbol(X);
+ }
+} // expected-warning{{SYMBOL DEAD}}
+
+void getCreatedConjuredSymbolLiveness(std::unique_ptr P) {
+ int *X = P.get();
+ clang_analyzer_warnOnDeadSymbol(X);
+ int Y;
+ if (!P) {
+Y = *P.get(); // expected-warning {{Dereference of null pointer [core.NullDereference]}}
+// expected-warning@-1 {{SYMBOL DEAD}}
+ }
+}
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
@@ -131,3 +131,80 @@
void derefOnRawPtrFromGetOnUnknownPtr(std::unique_ptr P) {
P.get()->foo(); // No warning.
}
+
+void derefConditionOnNullPtrFalseBranch() {
+ std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
+ if (P) { // expected-note {{Taking false branch}}
+// expected-note@-1{{Smart pointer 'P' is nul}}
+P->foo(); // No warning.
+ } else {
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+ }
+}
+
+void derefConditionOnNullPtrTrueBranch() {
+ std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
+ if (!P) { // expected-note {{Taking true branch}}
+// expected-note@-1{{Smart pointer 'P' is nul}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+ }
+}
+
+void derefConditionOnValidPtrTrueBranch() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
+ if (P) { // expected-note {{Taking true branch}}
+// expected-note@-1{{Smart pointer 'P' is non-nul}}
+PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'PNull'}}
+ } else {
+PNull->foo(); // No warning
+ }
+}
+
+void derefConditionOnValidPtrFalseBranch() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull; // expected-note
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
vrnithinkumar added inline comments.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:137
+ const auto *RecordDecl = MethodDecl->getParent();
+ if (!RecordDecl || !RecordDecl->getDeclContext()->isStdNamespace())
+return InnerType;
xazax.hun wrote:
> I'd rather use `Decl::isInStdNamespace` instead of querying the DeclContext
> of the decl. The former is more robust with inline namespaces.
Changed to use `Decl::isInStdNamespace`
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:141
+ const auto *TSD = dyn_cast(RecordDecl);
+ if (TSD) {
+auto TemplateArgs = TSD->getTemplateArgs().asArray();
xazax.hun wrote:
> Inverting this condition would reduce the indentation in the rest of the
> function.
inverted the if condition
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:145
+TemplateArgs.size() > 0 &&
+"Smart pointer should have specialized with atleast one template
type");
+auto InnerValueType = TemplateArgs[0].getAsType();
NoQ wrote:
> That's pretty fundamental, right? If it's a specialization, it must have
> something specialized. It isn't specific to unique pointers, right?
>
> Because unique pointers aren't special; technically anybody can define an
> arbitrary class with name `std::unique_ptr` and any properties they'd like.
> It's going to be undefined behavior according to the standard (because
> namespace `std` is explicitly reserved for the standard library) but if the
> compiler *crashes* it'll still be our fault.
>
>
added a check for `TemplateArgs.size() == 0` before accessing the first
Template argument.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:147
+auto InnerValueType = TemplateArgs[0].getAsType();
+InnerType =
+C.getASTContext().getPointerType(InnerValueType.getCanonicalType());
xazax.hun wrote:
> You could return the real inner type here and replace all other returns with
> `return {};` making the code a bit cleaner.
Updated to return with {}
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:404
+void SmartPtrModeling::handleBoolOperation(const CallEvent ,
+ CheckerContext ) const {
vsavchenko wrote:
> I suggest `BoolConversion`
yeah thats sounds better.
Changed.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:413
+ if (InnerPointVal) {
+bool IsInnerPtrNull = InnerPointVal->isZeroConstant();
+State = State->BindExpr(CallExpr, C.getLocationContext(),
xazax.hun wrote:
> Is this actually correct? What if the InnerPtr is an unconstrained symbol. In
> that case, it is not a zero constant so we will assume that it is constrained
> to non-zero. As far as I understand.
Fixed as you suggested in the below comments
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:432
+return;
+ } else {
+// In case of inner pointer SVal is not available we create
xazax.hun wrote:
> I'd do it the other way around as we discussed during the call.
> * Move the task of conjuring a new symbol to the beginning of the method.
> * Start by calling this function at the beginning of modeling operator bool.
> * The rest of the function could assume that there always is a symbol. It
> could be constrained to be non-null, it could be the zero constant, or it
> could be a completely unconstrained symbol. The latter will not work as
> expected with your current implementation, see my comment above.
Made the changes as suggested above
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:448
+
+if (NullState) {
+ auto NullVal = C.getSValBuilder().makeNull();
xazax.hun wrote:
> NoQ wrote:
> > There's no need to check. You just conjured a brand new symbol out of thin
> > air; you can be sure that it's completely unconstrained and both states are
> > feasible. You can instead `assert()` that they're both feasible.
> I think instead of removing this check, this method should be reworked. I
> think it might have some bugs, see my comment at the beginning of this method.
Refactored the method as suggested.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:458-460
+OS << "Assuming smart pointer ";
+ThisRegion->printPretty(OS);
+OS << " is null";
NoQ wrote:
> Wait, what happens when the region can't be pretty-printed? Does it print two
> spaces between "pointer" and "is"?
Yes.
Created `checkAndPrettyPrintRegion` to check if the region can be pretty
printed or not to avoid two spaces.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
vsavchenko added inline comments.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:404
+void SmartPtrModeling::handleBoolOperation(const CallEvent ,
+ CheckerContext ) const {
I suggest `BoolConversion`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
xazax.hun requested changes to this revision.
xazax.hun added a comment.
This revision now requires changes to proceed.
Please add a test case, where the unique_ptr is initialized from a pointer
parameter that has no assumptions. I think that case is not handled correctly.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:137
+ const auto *RecordDecl = MethodDecl->getParent();
+ if (!RecordDecl || !RecordDecl->getDeclContext()->isStdNamespace())
+return InnerType;
I'd rather use `Decl::isInStdNamespace` instead of querying the DeclContext of
the decl. The former is more robust with inline namespaces.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:141
+ const auto *TSD = dyn_cast(RecordDecl);
+ if (TSD) {
+auto TemplateArgs = TSD->getTemplateArgs().asArray();
Inverting this condition would reduce the indentation in the rest of the
function.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:147
+auto InnerValueType = TemplateArgs[0].getAsType();
+InnerType =
+C.getASTContext().getPointerType(InnerValueType.getCanonicalType());
You could return the real inner type here and replace all other returns with
`return {};` making the code a bit cleaner.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:413
+ if (InnerPointVal) {
+bool IsInnerPtrNull = InnerPointVal->isZeroConstant();
+State = State->BindExpr(CallExpr, C.getLocationContext(),
Is this actually correct? What if the InnerPtr is an unconstrained symbol. In
that case, it is not a zero constant so we will assume that it is constrained
to non-zero. As far as I understand.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:432
+return;
+ } else {
+// In case of inner pointer SVal is not available we create
I'd do it the other way around as we discussed during the call.
* Move the task of conjuring a new symbol to the beginning of the method.
* Start by calling this function at the beginning of modeling operator bool.
* The rest of the function could assume that there always is a symbol. It could
be constrained to be non-null, it could be the zero constant, or it could be a
completely unconstrained symbol. The latter will not work as expected with your
current implementation, see my comment above.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:448
+
+if (NullState) {
+ auto NullVal = C.getSValBuilder().makeNull();
NoQ wrote:
> There's no need to check. You just conjured a brand new symbol out of thin
> air; you can be sure that it's completely unconstrained and both states are
> feasible. You can instead `assert()` that they're both feasible.
I think instead of removing this check, this method should be reworked. I think
it might have some bugs, see my comment at the beginning of this method.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
NoQ added inline comments.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:130
+// Returns empty type if not found valid inner pointer type.
+static QualType getInnerPointerType(const CallEvent , CheckerContext ) {
+ QualType InnerType{};
vrnithinkumar wrote:
> It seems like a long shot to me.
> I am not sure is there any direct or easy way to get inner pointer type from
> a smart pointer
That's about right. You're doing exactly what you're asked: grab the template
parameter of the class. The problem is indeed that complicated!
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:145
+TemplateArgs.size() > 0 &&
+"Smart pointer should have specialized with atleast one template
type");
+auto InnerValueType = TemplateArgs[0].getAsType();
That's pretty fundamental, right? If it's a specialization, it must have
something specialized. It isn't specific to unique pointers, right?
Because unique pointers aren't special; technically anybody can define an
arbitrary class with name `std::unique_ptr` and any properties they'd like.
It's going to be undefined behavior according to the standard (because
namespace `std` is explicitly reserved for the standard library) but if the
compiler *crashes* it'll still be our fault.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:448
+
+if (NullState) {
+ auto NullVal = C.getSValBuilder().makeNull();
There's no need to check. You just conjured a brand new symbol out of thin air;
you can be sure that it's completely unconstrained and both states are
feasible. You can instead `assert()` that they're both feasible.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:458-460
+OS << "Assuming smart pointer ";
+ThisRegion->printPretty(OS);
+OS << " is null";
Wait, what happens when the region can't be pretty-printed? Does it print two
spaces between "pointer" and "is"?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
vrnithinkumar added inline comments.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:130
+// Returns empty type if not found valid inner pointer type.
+static QualType getInnerPointerType(const CallEvent , CheckerContext ) {
+ QualType InnerType{};
It seems like a long shot to me.
I am not sure is there any direct or easy way to get inner pointer type from a
smart pointer
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
vrnithinkumar updated this revision to Diff 286632.
vrnithinkumar marked 5 inline comments as done.
vrnithinkumar added a comment.
Changes to use conjureSymbolVal if the inner pointer value is missing
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
Files:
clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
clang/test/Analysis/smart-ptr-text-output.cpp
clang/test/Analysis/smart-ptr.cpp
Index: clang/test/Analysis/smart-ptr.cpp
===
--- clang/test/Analysis/smart-ptr.cpp
+++ clang/test/Analysis/smart-ptr.cpp
@@ -252,3 +252,46 @@
P->foo(); // No warning.
PValid->foo(); // No warning.
}
+
+void derefConditionOnNullPtr() {
+ std::unique_ptr P;
+ if (P)
+P->foo(); // No warning.
+ else
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnNotNullPtr() {
+ std::unique_ptr P;
+ if (!P)
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnValidPtr() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull;
+ if (P)
+PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnNotValidPtr() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull;
+ if (!P)
+PNull->foo(); // No warning.
+}
+
+void derefConditionOnUnKnownPtr(std::unique_ptr P) {
+ if (P)
+P->foo(); // No warning.
+ else
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnValidPtrAfterReset(std::unique_ptr P) {
+ P.reset(new A());
+ if (!P)
+P->foo(); // No warning.
+ else
+P->foo(); // No warning.
+}
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
@@ -116,3 +116,80 @@
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' of type 'std::unique_ptr' [cplusplus.Move]}}
// expected-note@-1 {{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
}
+
+void derefConditionOnNullPtrFalseBranch() {
+ std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
+ if (P) { // expected-note {{Taking false branch}}
+// expected-note@-1{{Smart pointer 'P' is nul}}
+P->foo(); // No warning.
+ } else {
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+ }
+}
+
+void derefConditionOnNullPtrTrueBranch() {
+ std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
+ if (!P) { // expected-note {{Taking true branch}}
+// expected-note@-1{{Smart pointer 'P' is nul}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+ }
+}
+
+void derefConditionOnValidPtrTrueBranch() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
+ if (P) { // expected-note {{Taking true branch}}
+// expected-note@-1{{Smart pointer 'P' is non-nul}}
+PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'PNull'}}
+ } else {
+PNull->foo(); // No warning
+ }
+}
+
+void derefConditionOnValidPtrFalseBranch() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
+ if (!P) { // expected-note {{Taking false branch}}
+// expected-note@-1{{Smart pointer 'P' is non-nul}}
+PNull->foo(); // No warning
+ } else {
+PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'PNull'}}
+ }
+}
+
+void derefConditionOnNotValidPtr() {
+ std::unique_ptr P(new A());
+ std::unique_ptr PNull;
+ if (!P)
+PNull->foo(); // No warning.
+}
+
+void derefConditionOnUnKnownPtrAssumeNull(std::unique_ptr P) {
+ std::unique_ptr PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
+ if (!P) { // expected-note {{Taking true branch}}
+// expected-note@-1{{Assuming smart pointer 'P' is null}}
+PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'PNull'}}
+ }
+}
+
+void derefConditionOnUnKnownPtrAssumeNonNull(std::unique_ptr P) {
+ std::unique_ptr PNull; // expected-note {{Default
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
vrnithinkumar added inline comments.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:147
-if (!move::isMovedFrom(State, ThisR)) {
- // TODO: Model this case as well. At least, avoid invalidation of
- // globals.
- return false;
+if (ModelSmartPtrDereference) {
+ handleBoolOperation(Call, C);
xazax.hun wrote:
> vrnithinkumar wrote:
> > This seems little messy here.
> > I guess once we model the `std::move` for smart ptr it will be less messy
> I agree. `isNullAfterMoveMethod` is especially confusing as it does not do
> what the name of the function says. It checks if the `Call` is a boolean
> conversion operator. I'd suggest renaming that method to make this code a bit
> more sensible.
>
> Moreover, when `ModelSmartPtrDereference` is true, we no longer model moves
> below right? I think a comment that this logic is duplicated
> `handleBoolOperation` might make the code cleaner.
>
> But yeah, this needs to be cleaned up, hopefully really soon.
- Renamed the method to `isBooleanOpMethod`
- Added comments and FIXME
Hopefully we can clean up once `std::move` is modeled.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:389
+}
+C.addTransition(State);
+ } else if (move::isMovedFrom(State, ThisRegion)) {
xazax.hun wrote:
> This looks fine for now, but we often prefer adding a return after each case
> to avoid executing multiple `addTransition`s accidentally after refactoring.
Added return
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:411
+ if (NotNullState) {
+auto NonNullVal = C.getSValBuilder().makeTruthVal(true);
+NotNullState =
NoQ wrote:
> vrnithinkumar wrote:
> > Since the inner pointer value can be any non-null value, I am not sure what
> > should be the value to be added to the map for tracking.
> >
> Don't add values, constrain existing values.
Made changes to create the conjured symbol and use that to constrain.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
xazax.hun added inline comments.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:147
-if (!move::isMovedFrom(State, ThisR)) {
- // TODO: Model this case as well. At least, avoid invalidation of
- // globals.
- return false;
+if (ModelSmartPtrDereference) {
+ handleBoolOperation(Call, C);
vrnithinkumar wrote:
> This seems little messy here.
> I guess once we model the `std::move` for smart ptr it will be less messy
I agree. `isNullAfterMoveMethod` is especially confusing as it does not do what
the name of the function says. It checks if the `Call` is a boolean conversion
operator. I'd suggest renaming that method to make this code a bit more
sensible.
Moreover, when `ModelSmartPtrDereference` is true, we no longer model moves
below right? I think a comment that this logic is duplicated
`handleBoolOperation` might make the code cleaner.
But yeah, this needs to be cleaned up, hopefully really soon.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:389
+}
+C.addTransition(State);
+ } else if (move::isMovedFrom(State, ThisRegion)) {
This looks fine for now, but we often prefer adding a return after each case to
avoid executing multiple `addTransition`s accidentally after refactoring.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
NoQ added inline comments.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:400
+ ProgramStateRef NotNullState, NullState;
+ std::tie(NotNullState, NullState) =
State->assume(CallExprVal.getValue());
+
vrnithinkumar wrote:
> NoQ wrote:
> > It's always `UnknownVal` because the call has not been evaluated yet. This
> > `assume()` does nothing.
> Okay.
> So in that case the `NotNullState` and `NullState` will be same as the
> original state?
Yup.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:411
+ if (NotNullState) {
+auto NonNullVal = C.getSValBuilder().makeTruthVal(true);
+NotNullState =
vrnithinkumar wrote:
> Since the inner pointer value can be any non-null value, I am not sure what
> should be the value to be added to the map for tracking.
>
Don't add values, constrain existing values.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
vrnithinkumar added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:400 + ProgramStateRef NotNullState, NullState; + std::tie(NotNullState, NullState) = State->assume(CallExprVal.getValue()); + NoQ wrote: > It's always `UnknownVal` because the call has not been evaluated yet. This > `assume()` does nothing. Okay. So in that case the `NotNullState` and `NullState` will be same as the original state? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86027/new/ https://reviews.llvm.org/D86027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
vrnithinkumar added inline comments.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:411
+ if (NotNullState) {
+auto NonNullVal = C.getSValBuilder().makeTruthVal(true);
+NotNullState =
Since the inner pointer value can be any non-null value, I am not sure what
should be the value to be added to the map for tracking.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:400 + ProgramStateRef NotNullState, NullState; + std::tie(NotNullState, NullState) = State->assume(CallExprVal.getValue()); + It's always `UnknownVal` because the call has not been evaluated yet. This `assume()` does nothing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86027/new/ https://reviews.llvm.org/D86027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
vrnithinkumar added inline comments.
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:147
-if (!move::isMovedFrom(State, ThisR)) {
- // TODO: Model this case as well. At least, avoid invalidation of
- // globals.
- return false;
+if (ModelSmartPtrDereference) {
+ handleBoolOperation(Call, C);
This seems little messy here.
I guess once we model the `std::move` for smart ptr it will be less messy
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr
vrnithinkumar created this revision.
Herald added subscribers: cfe-commits, steakhal, ASDenysPetrov, martong,
Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet,
baloghadamsoftware, xazax.hun.
Herald added a project: clang.
vrnithinkumar requested review of this revision.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D86027
Files:
clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
clang/test/Analysis/smart-ptr.cpp
Index: clang/test/Analysis/smart-ptr.cpp
===
--- clang/test/Analysis/smart-ptr.cpp
+++ clang/test/Analysis/smart-ptr.cpp
@@ -87,6 +87,7 @@
void derefAfterResetWithNonNull() {
std::unique_ptr P;
P.reset(new A());
+ clang_analyzer_numTimesReached(); // expected-warning {{1}}
P->foo(); // No warning.
}
@@ -117,36 +118,39 @@
void pass_smart_ptr_by_const_ptr(const std::unique_ptr *a);
void regioninvalidationTest() {
- {
-std::unique_ptr P;
-pass_smart_ptr_by_ref(P);
-P->foo(); // no-warning
- }
- {
-std::unique_ptr P;
-pass_smart_ptr_by_const_ref(P);
-P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
- }
- {
-std::unique_ptr P;
-pass_smart_ptr_by_rvalue_ref(std::move(P));
-P->foo(); // no-warning
- }
- {
-std::unique_ptr P;
-pass_smart_ptr_by_const_rvalue_ref(std::move(P));
-P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
- }
- {
-std::unique_ptr P;
-pass_smart_ptr_by_ptr();
-P->foo();
- }
- {
-std::unique_ptr P;
-pass_smart_ptr_by_const_ptr();
-P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
- }
+ std::unique_ptr P;
+ pass_smart_ptr_by_ref(P);
+ P->foo(); // no-warning
+}
+
+void regioninvalidationTest1() {
+ std::unique_ptr P;
+ pass_smart_ptr_by_const_ref(P);
+ P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void regioninvalidationTest2() {
+ std::unique_ptr P;
+ pass_smart_ptr_by_rvalue_ref(std::move(P));
+ P->foo(); // no-warning
+}
+
+void regioninvalidationTest3() {
+ std::unique_ptr P;
+ pass_smart_ptr_by_const_rvalue_ref(std::move(P));
+ P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void regioninvalidationTest4() {
+ std::unique_ptr P;
+ pass_smart_ptr_by_ptr();
+ P->foo();
+}
+
+void regioninvalidationTest5() {
+ std::unique_ptr P;
+ pass_smart_ptr_by_const_ptr();
+ P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
}
struct StructWithSmartPtr {
@@ -161,36 +165,39 @@
void pass_struct_with_smart_ptr_by_const_ptr(const StructWithSmartPtr *a);
void regioninvalidationTestWithinStruct() {
- {
-StructWithSmartPtr S;
-pass_struct_with_smart_ptr_by_ref(S);
-S.P->foo(); // no-warning
- }
- {
-StructWithSmartPtr S;
-pass_struct_with_smart_ptr_by_const_ref(S);
-S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
- }
- {
-StructWithSmartPtr S;
-pass_struct_with_smart_ptr_by_rvalue_ref(std::move(S));
-S.P->foo(); // no-warning
- }
- {
-StructWithSmartPtr S;
-pass_struct_with_smart_ptr_by_const_rvalue_ref(std::move(S));
-S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
- }
- {
-StructWithSmartPtr S;
-pass_struct_with_smart_ptr_by_ptr();
-S.P->foo();
- }
- {
-StructWithSmartPtr S;
-pass_struct_with_smart_ptr_by_const_ptr();
-S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
- }
+ StructWithSmartPtr S;
+ pass_struct_with_smart_ptr_by_ref(S);
+ S.P->foo(); // no-warning
+}
+
+void regioninvalidationTestWithinStruct2() {
+ StructWithSmartPtr S;
+ pass_struct_with_smart_ptr_by_const_ref(S);
+ S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void regioninvalidationTestWithinStruct3() {
+ StructWithSmartPtr S;
+ pass_struct_with_smart_ptr_by_rvalue_ref(std::move(S));
+ S.P->foo(); // no-warning
+}
+
+void regioninvalidationTestWithinStruct4() {
+ StructWithSmartPtr S;
+ pass_struct_with_smart_ptr_by_const_rvalue_ref(std::move(S));
+ S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void regioninvalidationTestWithinStruct5() {
+ StructWithSmartPtr S;
+ pass_struct_with_smart_ptr_by_ptr();
+ S.P->foo(); // no-warning
+}
+
+void regioninvalidationTestWithinStruct6() {
+ StructWithSmartPtr S;
+ pass_struct_with_smart_ptr_by_const_ptr();
+ S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
}
void derefAfterAssignment() {
@@ -217,14
