Re: [PATCH] D29303: In VirtualCallChecker, handle indirect calls

2017-02-25 Thread Anna Zaks via cfe-commits 
Thank you!

On Friday, February 24, 2017, Hans Wennborg  wrote:

> Yes, this looks very straight-forward. Merged in r296154.
>
> On Fri, Feb 24, 2017 at 4:29 AM, Sam McCall via cfe-commits
> > wrote:
> > Thanks Anna, I'm new to the release process here.
> >
> > Hans: this is a simple fix for a null-dereference in the static analyzer.
> > Does it make sense to cherrypick?
> >
> > On Sat, Feb 18, 2017 at 2:46 AM, Anna Zaks via Phabricator
> > > wrote:
> >>
> >> zaks.anna added a comment.
> >>
> >> Has this been cherry-picked into the clang 4.0 release branch? If not,
> we
> >> should definitely do that!
> >>
> >>
> >> Repository:
> >>   rL LLVM
> >>
> >> https://reviews.llvm.org/D29303
> >>
> >>
> >>
> >
> >
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org 
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D29303: In VirtualCallChecker, handle indirect calls

2017-02-24 Thread Hans Wennborg via cfe-commits 
Yes, this looks very straight-forward. Merged in r296154.

On Fri, Feb 24, 2017 at 4:29 AM, Sam McCall via cfe-commits
 wrote:
> Thanks Anna, I'm new to the release process here.
>
> Hans: this is a simple fix for a null-dereference in the static analyzer.
> Does it make sense to cherrypick?
>
> On Sat, Feb 18, 2017 at 2:46 AM, Anna Zaks via Phabricator
>  wrote:
>>
>> zaks.anna added a comment.
>>
>> Has this been cherry-picked into the clang 4.0 release branch? If not, we
>> should definitely do that!
>>
>>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D29303
>>
>>
>>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D29303: In VirtualCallChecker, handle indirect calls

2017-02-24 Thread Sam McCall via cfe-commits 
Thanks Anna, I'm new to the release process here.

Hans: this is a simple fix for a null-dereference in the static analyzer.
Does it make sense to cherrypick?

On Sat, Feb 18, 2017 at 2:46 AM, Anna Zaks via Phabricator <
revi...@reviews.llvm.org> wrote:

> zaks.anna added a comment.
>
> Has this been cherry-picked into the clang 4.0 release branch? If not, we
> should definitely do that!
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D29303
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29303: In VirtualCallChecker, handle indirect calls

2017-02-17 Thread Anna Zaks via Phabricator via cfe-commits 
zaks.anna added a comment.

Has this been cherry-picked into the clang 4.0 release branch? If not, we 
should definitely do that!


Repository:
  rL LLVM

https://reviews.llvm.org/D29303



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

[PATCH] D29303: In VirtualCallChecker, handle indirect calls

2017-01-30 Thread Sam McCall via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL293604: In VirtualCallChecker, handle indirect calls 
(authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D29303?vs=86342&id=86385#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D29303

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  cfe/trunk/test/Analysis/virtualcall.cpp


Index: cfe/trunk/test/Analysis/virtualcall.cpp
===
--- cfe/trunk/test/Analysis/virtualcall.cpp
+++ cfe/trunk/test/Analysis/virtualcall.cpp
@@ -115,12 +115,23 @@
   int foo() override;
 };
 
+// Regression test: don't crash when there's no direct callee.
+class F {
+public:
+  F() {
+void (F::* ptr)() = &F::foo;
+(this->*ptr)();
+  }
+  void foo();
+};
+
 int main() {
   A *a;
   B *b;
   C *c;
   D *d;
   E *e;
+  F *f;
 }
 
 #include "virtualcall.h"
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
@@ -179,7 +179,8 @@
   }
 
   // Get the callee.
-  const CXXMethodDecl *MD = dyn_cast(CE->getDirectCallee());
+  const CXXMethodDecl *MD =
+  dyn_cast_or_null(CE->getDirectCallee());
   if (MD && MD->isVirtual() && !callIsNonVirtual && !MD->hasAttr() 
&&
   !MD->getParent()->hasAttr())
 ReportVirtualCall(CE, MD->isPure());


Index: cfe/trunk/test/Analysis/virtualcall.cpp
===
--- cfe/trunk/test/Analysis/virtualcall.cpp
+++ cfe/trunk/test/Analysis/virtualcall.cpp
@@ -115,12 +115,23 @@
   int foo() override;
 };
 
+// Regression test: don't crash when there's no direct callee.
+class F {
+public:
+  F() {
+void (F::* ptr)() = &F::foo;
+(this->*ptr)();
+  }
+  void foo();
+};
+
 int main() {
   A *a;
   B *b;
   C *c;
   D *d;
   E *e;
+  F *f;
 }
 
 #include "virtualcall.h"
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
@@ -179,7 +179,8 @@
   }
 
   // Get the callee.
-  const CXXMethodDecl *MD = dyn_cast(CE->getDirectCallee());
+  const CXXMethodDecl *MD =
+  dyn_cast_or_null(CE->getDirectCallee());
   if (MD && MD->isVirtual() && !callIsNonVirtual && !MD->hasAttr() &&
   !MD->getParent()->hasAttr())
 ReportVirtualCall(CE, MD->isPure());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29303: In VirtualCallChecker, handle indirect calls

2017-01-30 Thread Benjamin Kramer via Phabricator via cfe-commits 
bkramer accepted this revision.
bkramer added a comment.
This revision is now accepted and ready to land.

lg


https://reviews.llvm.org/D29303



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

[PATCH] D29303: In VirtualCallChecker, handle indirect calls

2017-01-30 Thread Sam McCall via Phabricator via cfe-commits 
sammccall updated this revision to Diff 86342.
sammccall added a comment.

Add regression test.


https://reviews.llvm.org/D29303

Files:
  lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  test/Analysis/virtualcall.cpp


Index: test/Analysis/virtualcall.cpp
===
--- test/Analysis/virtualcall.cpp
+++ test/Analysis/virtualcall.cpp
@@ -115,12 +115,23 @@
   int foo() override;
 };
 
+// Regression test: don't crash when there's no direct callee.
+class F {
+public:
+  F() {
+void (F::* ptr)() = &F::foo;
+(this->*ptr)();
+  }
+  void foo();
+};
+
 int main() {
   A *a;
   B *b;
   C *c;
   D *d;
   E *e;
+  F *f;
 }
 
 #include "virtualcall.h"
Index: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
+++ lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
@@ -179,7 +179,8 @@
   }
 
   // Get the callee.
-  const CXXMethodDecl *MD = dyn_cast(CE->getDirectCallee());
+  const CXXMethodDecl *MD =
+  dyn_cast_or_null(CE->getDirectCallee());
   if (MD && MD->isVirtual() && !callIsNonVirtual && !MD->hasAttr() 
&&
   !MD->getParent()->hasAttr())
 ReportVirtualCall(CE, MD->isPure());


Index: test/Analysis/virtualcall.cpp
===
--- test/Analysis/virtualcall.cpp
+++ test/Analysis/virtualcall.cpp
@@ -115,12 +115,23 @@
   int foo() override;
 };
 
+// Regression test: don't crash when there's no direct callee.
+class F {
+public:
+  F() {
+void (F::* ptr)() = &F::foo;
+(this->*ptr)();
+  }
+  void foo();
+};
+
 int main() {
   A *a;
   B *b;
   C *c;
   D *d;
   E *e;
+  F *f;
 }
 
 #include "virtualcall.h"
Index: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
+++ lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
@@ -179,7 +179,8 @@
   }
 
   // Get the callee.
-  const CXXMethodDecl *MD = dyn_cast(CE->getDirectCallee());
+  const CXXMethodDecl *MD =
+  dyn_cast_or_null(CE->getDirectCallee());
   if (MD && MD->isVirtual() && !callIsNonVirtual && !MD->hasAttr() &&
   !MD->getParent()->hasAttr())
 ReportVirtualCall(CE, MD->isPure());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29303: In VirtualCallChecker, handle indirect calls

2017-01-30 Thread Benjamin Kramer via Phabricator via cfe-commits 
bkramer added a comment.

Your test case is fine, it crashes with assertions enabled.


https://reviews.llvm.org/D29303



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

[PATCH] D29303: In VirtualCallChecker, handle indirect calls

2017-01-30 Thread Sam McCall via Phabricator via cfe-commits 
sammccall added a comment.

I couldn't work out how to add a test for this, advice appreciated.

Closest I could get was adding something like this to 
test/Analysis/virtualcall.cpp:

  class F { public: F(); void foo(); };
  F::F() { void (F::* ptr) = &F::foo; (this->*ptr)(); }

which crashes, but only if I add extra logging :\


https://reviews.llvm.org/D29303



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

[PATCH] D29303: In VirtualCallChecker, handle indirect calls

2017-01-30 Thread Sam McCall via Phabricator via cfe-commits 
sammccall updated this revision to Diff 86337.
sammccall added a comment.

Oops, reverted noise :(


https://reviews.llvm.org/D29303

Files:
  lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp


Index: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
+++ lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
@@ -179,7 +179,8 @@
   }
 
   // Get the callee.
-  const CXXMethodDecl *MD = dyn_cast(CE->getDirectCallee());
+  const CXXMethodDecl *MD =
+  dyn_cast_or_null(CE->getDirectCallee());
   if (MD && MD->isVirtual() && !callIsNonVirtual && !MD->hasAttr() 
&&
   !MD->getParent()->hasAttr())
 ReportVirtualCall(CE, MD->isPure());


Index: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
+++ lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
@@ -179,7 +179,8 @@
   }
 
   // Get the callee.
-  const CXXMethodDecl *MD = dyn_cast(CE->getDirectCallee());
+  const CXXMethodDecl *MD =
+  dyn_cast_or_null(CE->getDirectCallee());
   if (MD && MD->isVirtual() && !callIsNonVirtual && !MD->hasAttr() &&
   !MD->getParent()->hasAttr())
 ReportVirtualCall(CE, MD->isPure());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29303: In VirtualCallChecker, handle indirect calls

2017-01-30 Thread Sam McCall via Phabricator via cfe-commits 
sammccall created this revision.

In VirtualCallChecker, handle indirect calls.

getDirectCallee() can be nullptr, and dyn_cast(nullptr) is UB


https://reviews.llvm.org/D29303

Files:
  lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp

Index: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
+++ lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
@@ -1,3 +1,4 @@
+#include 
 //===- VirtualCallChecker.cpp *- C++ -*-==//
 //
 // The LLVM Compiler Infrastructure
@@ -44,7 +45,7 @@
   /// may result in unexpected behavior).
   bool ReportPureOnly = false;
 
-  typedef const CallExpr * WorkListUnit;
+  typedef const CallExpr *WorkListUnit;
   typedef SmallVector DFSWorkList;
 
   /// A vector representing the worklist which has a chain of CallExprs.
@@ -54,12 +55,13 @@
   // body has not been visited yet.
   // PostVisited : A CallExpr to this FunctionDecl is in the worklist, and the
   // body has been visited.
-  enum Kind { NotVisited,
-  PreVisited,  /**< A CallExpr to this FunctionDecl is in the
-worklist, but the body has not yet been
-visited. */
-  PostVisited  /**< A CallExpr to this FunctionDecl is in the
-worklist, and the body has been visited. */
+  enum Kind {
+NotVisited,
+PreVisited, /**< A CallExpr to this FunctionDecl is in the
+ worklist, but the body has not yet been
+ visited. */
+PostVisited /**< A CallExpr to this FunctionDecl is in the
+ worklist, and the body has been visited. */
   };
 
   /// A DenseMap that records visited states of FunctionDecls.
@@ -135,7 +137,6 @@
   void VisitChildren(Stmt *S);
 
   void ReportVirtualCall(const CallExpr *CE, bool isPure);
-
 };
 } // end anonymous namespace
 
@@ -179,7 +180,8 @@
   }
 
   // Get the callee.
-  const CXXMethodDecl *MD = dyn_cast(CE->getDirectCallee());
+  const CXXMethodDecl *MD =
+  dyn_cast_or_null(CE->getDirectCallee());
   if (MD && MD->isVirtual() && !callIsNonVirtual && !MD->hasAttr() &&
   !MD->getParent()->hasAttr())
 ReportVirtualCall(CE, MD->isPure());
@@ -208,8 +210,9 @@
   os << " <-- " << *visitingCallExpr->getDirectCallee();
 // Names of FunctionDecls in worklist with state PostVisited.
 for (SmallVectorImpl::iterator I = WList.end(),
- E = WList.begin(); I != E; --I) {
-  const FunctionDecl *FD = (*(I-1))->getDirectCallee();
+ E = WList.begin();
+ I != E; --I) {
+  const FunctionDecl *FD = (*(I - 1))->getDirectCallee();
   assert(FD);
   if (VisitedFunctions[FD] == PostVisited)
 os << " <-- " << *FD;
@@ -219,7 +222,7 @@
   }
 
   PathDiagnosticLocation CELoc =
-PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
+  PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
   SourceRange R = CE->getCallee()->getSourceRange();
 
   os << "Call to ";
@@ -249,12 +252,12 @@
 //===--===//
 
 namespace {
-class VirtualCallChecker : public Checker > {
+class VirtualCallChecker : public Checker> {
 public:
   DefaultBool isInterprocedural;
   DefaultBool isPureOnly;
 
-  void checkASTDecl(const CXXRecordDecl *RD, AnalysisManager& mgr,
+  void checkASTDecl(const CXXRecordDecl *RD, AnalysisManager &mgr,
 BugReporter &BR) const {
 AnalysisDeclContext *ADC = mgr.getAnalysisDeclContext(RD);
 
@@ -281,11 +284,9 @@
 
 void ento::registerVirtualCallChecker(CheckerManager &mgr) {
   VirtualCallChecker *checker = mgr.registerChecker();
-  checker->isInterprocedural =
-  mgr.getAnalyzerOptions().getBooleanOption("Interprocedural", false,
-checker);
+  checker->isInterprocedural = mgr.getAnalyzerOptions().getBooleanOption(
+  "Interprocedural", false, checker);
 
   checker->isPureOnly =
-  mgr.getAnalyzerOptions().getBooleanOption("PureOnly", false,
-checker);
+  mgr.getAnalyzerOptions().getBooleanOption("PureOnly", false, checker);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits