[PATCH] D87043: [Analyzer] Fix for dereferece of smart pointer after branching on unknown inner pointer

[Nithin VR via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
vrnithinkumar marked an inline comment as done.
Closed by commit rG0b4fe8086f03: [Analyzer] Fix for dereferece of smart pointer 
after branching on unknown inner… (authored by vrnithinkumar).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87043/new/

https://reviews.llvm.org/D87043

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
@@ -333,7 +333,7 @@
 void drefOnAssignedNullFromMethodPtrValidSmartPtr() {
   std::unique_ptr P(new A());
   P = returnRValRefOfUniquePtr();
-  P->foo(); // No warning. 
+  P->foo(); // No warning.
 }
 
 void derefMoveConstructedWithValidPtr() {
@@ -374,7 +374,7 @@
 
 void derefMoveConstructedWithRValueRefReturn() {
   std::unique_ptr P(functionReturnsRValueRef());
-  P->foo();  // No warning.
+  P->foo(); // No warning.
 }
 
 void derefConditionOnNullPtr() {
@@ -450,3 +450,10 @@
   else
 return *P; // expected-warning {{Dereference of null smart pointer 'P' 
[alpha.cplusplus.SmartPtr]}}
 }
+
+void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr P) {
+  A *RP = P.get();
+  if (!RP) {
+P->foo(); // 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
@@ -304,3 +304,12 @@
 // expected-note@-1 {{Division by zero}}
   }
 };
+
+void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr P) {
+  A *RP = P.get();
+  if (!RP) { // expected-note {{Assuming 'RP' is null}}
+// expected-note@-1 {{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'}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -103,7 +103,8 @@
 
 bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegion) {
   const auto *InnerPointVal = State->get(ThisRegion);
-  return InnerPointVal && InnerPointVal->isZeroConstant();
+  return InnerPointVal &&
+ !State->assume(InnerPointVal->castAs(), true);
 }
 } // namespace smartptr
 } // namespace ento


Index: clang/test/Analysis/smart-ptr.cpp
===
--- clang/test/Analysis/smart-ptr.cpp
+++ clang/test/Analysis/smart-ptr.cpp
@@ -333,7 +333,7 @@
 void drefOnAssignedNullFromMethodPtrValidSmartPtr() {
   std::unique_ptr P(new A());
   P = returnRValRefOfUniquePtr();
-  P->foo(); // No warning. 
+  P->foo(); // No warning.
 }
 
 void derefMoveConstructedWithValidPtr() {
@@ -374,7 +374,7 @@
 
 void derefMoveConstructedWithRValueRefReturn() {
   std::unique_ptr P(functionReturnsRValueRef());
-  P->foo();  // No warning.
+  P->foo(); // No warning.
 }
 
 void derefConditionOnNullPtr() {
@@ -450,3 +450,10 @@
   else
 return *P; // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
+
+void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr P) {
+  A *RP = P.get();
+  if (!RP) {
+P->foo(); // 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
@@ -304,3 +304,12 @@
 // expected-note@-1 {{Division by zero}}
   }
 };
+
+void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr P) {
+  A *RP = P.get();
+  if (!RP) { // expected-note {{Assuming 'RP' is null}}
+// expected-note@-1 {{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'}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -103,7 +103,8 @@
 
 bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegion) {
   const auto *InnerPointVal = State->get(ThisRegion);
-  return InnerPointVal && 

[PATCH] D87043: [Analyzer] Fix for dereferece of smart pointer after branching on unknown inner pointer

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Looks great, thank you! I think you can commit it :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87043/new/

https://reviews.llvm.org/D87043

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87043: [Analyzer] Fix for dereferece of smart pointer after branching on unknown inner pointer

[Nithin VR via Phabricator via cfe-commits]

vrnithinkumar marked an inline comment as done.
vrnithinkumar added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:106
   const auto *InnerPointVal = State->get(ThisRegion);
-  return InnerPointVal && InnerPointVal->isZeroConstant();
+  return InnerPointVal && State->isNull(*InnerPointVal).isConstrainedTrue();
 }

NoQ wrote:
> The intended ultimate test is `!State->assume(X)` and it's also much easier 
> to write. I strongly suspect that we should remove these other APIs; they're 
> only confusing.
Sorry for addressing the pending comment so late. :(
I was busy with my postponed exams. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87043/new/

https://reviews.llvm.org/D87043

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87043: [Analyzer] Fix for dereferece of smart pointer after branching on unknown inner pointer

[Nithin VR via Phabricator via cfe-commits]

vrnithinkumar updated this revision to Diff 296785.
vrnithinkumar added a comment.

- Addressing review comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87043/new/

https://reviews.llvm.org/D87043

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
@@ -333,7 +333,7 @@
 void drefOnAssignedNullFromMethodPtrValidSmartPtr() {
   std::unique_ptr P(new A());
   P = returnRValRefOfUniquePtr();
-  P->foo(); // No warning. 
+  P->foo(); // No warning.
 }
 
 void derefMoveConstructedWithValidPtr() {
@@ -374,7 +374,7 @@
 
 void derefMoveConstructedWithRValueRefReturn() {
   std::unique_ptr P(functionReturnsRValueRef());
-  P->foo();  // No warning.
+  P->foo(); // No warning.
 }
 
 void derefConditionOnNullPtr() {
@@ -450,3 +450,10 @@
   else
 return *P; // expected-warning {{Dereference of null smart pointer 'P' 
[alpha.cplusplus.SmartPtr]}}
 }
+
+void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr P) {
+  A *RP = P.get();
+  if (!RP) {
+P->foo(); // 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
@@ -304,3 +304,12 @@
 // expected-note@-1 {{Division by zero}}
   }
 };
+
+void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr P) {
+  A *RP = P.get();
+  if (!RP) { // expected-note {{Assuming 'RP' is null}}
+// expected-note@-1 {{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'}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -103,7 +103,8 @@
 
 bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegion) {
   const auto *InnerPointVal = State->get(ThisRegion);
-  return InnerPointVal && InnerPointVal->isZeroConstant();
+  return InnerPointVal &&
+ !State->assume(InnerPointVal->castAs(), true);
 }
 } // namespace smartptr
 } // namespace ento


Index: clang/test/Analysis/smart-ptr.cpp
===
--- clang/test/Analysis/smart-ptr.cpp
+++ clang/test/Analysis/smart-ptr.cpp
@@ -333,7 +333,7 @@
 void drefOnAssignedNullFromMethodPtrValidSmartPtr() {
   std::unique_ptr P(new A());
   P = returnRValRefOfUniquePtr();
-  P->foo(); // No warning. 
+  P->foo(); // No warning.
 }
 
 void derefMoveConstructedWithValidPtr() {
@@ -374,7 +374,7 @@
 
 void derefMoveConstructedWithRValueRefReturn() {
   std::unique_ptr P(functionReturnsRValueRef());
-  P->foo();  // No warning.
+  P->foo(); // No warning.
 }
 
 void derefConditionOnNullPtr() {
@@ -450,3 +450,10 @@
   else
 return *P; // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
+
+void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr P) {
+  A *RP = P.get();
+  if (!RP) {
+P->foo(); // 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
@@ -304,3 +304,12 @@
 // expected-note@-1 {{Division by zero}}
   }
 };
+
+void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr P) {
+  A *RP = P.get();
+  if (!RP) { // expected-note {{Assuming 'RP' is null}}
+// expected-note@-1 {{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'}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -103,7 +103,8 @@
 
 bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegion) {
   const auto *InnerPointVal = State->get(ThisRegion);
-  return InnerPointVal && InnerPointVal->isZeroConstant();
+  return InnerPointVal &&
+ !State->assume(InnerPointVal->castAs(), true);
 }
 } // namespace smartptr
 } // namespace ento
___
cfe-commits 

[PATCH] D87043: [Analyzer] Fix for dereferece of smart pointer after branching on unknown inner pointer

[Nithin VR via Phabricator via cfe-commits]

vrnithinkumar added a comment.

In D87043#2294531 , @Szelethus wrote:

> Congrats on your GSoC! Unless I missed it, it seems like you haven't posted 
> your final report on cfe-dev, even though its an amazing looking document 
> with a lot of examples and explanations. I think it would be great to spread 
> the message, its a work to be proud of!

I am sorry,  I took a break and I forgot about sharing the final report. 
Thanks for the reminder.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87043/new/

https://reviews.llvm.org/D87043

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87043: [Analyzer] Fix for dereferece of smart pointer after branching on unknown inner pointer

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Congrats on your GSoC! Unless I missed it, it seems like you haven't posted 
your final evaluation on cfe-dev, even though its an amazing looking document 
with a lot of examples and explanations. I think it would be great to spread 
the message, its a work to be proud of!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87043/new/

https://reviews.llvm.org/D87043

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87043: [Analyzer] Fix for dereferece of smart pointer after branching on unknown inner pointer

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Nice catch, thx!




Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:106
   const auto *InnerPointVal = State->get(ThisRegion);
-  return InnerPointVal && InnerPointVal->isZeroConstant();
+  return InnerPointVal && State->isNull(*InnerPointVal).isConstrainedTrue();
 }

The intended ultimate test is `!State->assume(X)` and it's also much easier to 
write. I strongly suspect that we should remove these other APIs; they're only 
confusing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87043/new/

https://reviews.llvm.org/D87043

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87043: [Analyzer] Fix for dereferece of smart pointer after branching on unknown inner pointer

[Nithin VR via Phabricator via cfe-commits]

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/D87043

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
@@ -333,7 +333,7 @@
 void drefOnAssignedNullFromMethodPtrValidSmartPtr() {
   std::unique_ptr P(new A());
   P = returnRValRefOfUniquePtr();
-  P->foo(); // No warning. 
+  P->foo(); // No warning.
 }
 
 void derefMoveConstructedWithValidPtr() {
@@ -374,7 +374,7 @@
 
 void derefMoveConstructedWithRValueRefReturn() {
   std::unique_ptr P(functionReturnsRValueRef());
-  P->foo();  // No warning.
+  P->foo(); // No warning.
 }
 
 void derefConditionOnNullPtr() {
@@ -450,3 +450,10 @@
   else
 return *P; // expected-warning {{Dereference of null smart pointer 'P' 
[alpha.cplusplus.SmartPtr]}}
 }
+
+void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr P) {
+  A *RP = P.get();
+  if (!RP) {
+P->foo(); // 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
@@ -304,3 +304,12 @@
 // expected-note@-1 {{Division by zero}}
   }
 };
+
+void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr P) {
+  A *RP = P.get();
+  if (!RP) { // expected-note {{Assuming 'RP' is null}}
+// expected-note@-1 {{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'}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -103,7 +103,7 @@
 
 bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegion) {
   const auto *InnerPointVal = State->get(ThisRegion);
-  return InnerPointVal && InnerPointVal->isZeroConstant();
+  return InnerPointVal && State->isNull(*InnerPointVal).isConstrainedTrue();
 }
 } // namespace smartptr
 } // namespace ento


Index: clang/test/Analysis/smart-ptr.cpp
===
--- clang/test/Analysis/smart-ptr.cpp
+++ clang/test/Analysis/smart-ptr.cpp
@@ -333,7 +333,7 @@
 void drefOnAssignedNullFromMethodPtrValidSmartPtr() {
   std::unique_ptr P(new A());
   P = returnRValRefOfUniquePtr();
-  P->foo(); // No warning. 
+  P->foo(); // No warning.
 }
 
 void derefMoveConstructedWithValidPtr() {
@@ -374,7 +374,7 @@
 
 void derefMoveConstructedWithRValueRefReturn() {
   std::unique_ptr P(functionReturnsRValueRef());
-  P->foo();  // No warning.
+  P->foo(); // No warning.
 }
 
 void derefConditionOnNullPtr() {
@@ -450,3 +450,10 @@
   else
 return *P; // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
+
+void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr P) {
+  A *RP = P.get();
+  if (!RP) {
+P->foo(); // 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
@@ -304,3 +304,12 @@
 // expected-note@-1 {{Division by zero}}
   }
 };
+
+void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr P) {
+  A *RP = P.get();
+  if (!RP) { // expected-note {{Assuming 'RP' is null}}
+// expected-note@-1 {{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'}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -103,7 +103,7 @@
 
 bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegion) {
   const auto *InnerPointVal = State->get(ThisRegion);
-  return InnerPointVal && InnerPointVal->isZeroConstant();
+  return InnerPointVal && State->isNull(*InnerPointVal).isConstrainedTrue();
 }
 }