[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

This revision was automatically updated to reflect the committed changes. Closed by commit rL311877: [analyzer][GSoC] Re-implemente current virtual calls checker in a path… (authored by xazax). Changed prior to commit: https://reviews.llvm.org/D34275?vs=112196=112857#toc Repository: rL LLVM

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. All right then, i approve! > There are 105 alarms running the checker on the LibreOffice, 92 True > positive, 13 not sure. That's impressively loud. I guess you can try reporting some of the bugs

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb added a comment. There are 105 alarms running the checker on the LibreOffice, 92 True positive, 13 not sure. https://reviews.llvm.org/D34275 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb updated this revision to Diff 112196. wangxindsb added a comment. - Highlight pure virtual even in non-pure-only mode; - Add change to the header. https://reviews.llvm.org/D34275 Files: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp test/Analysis/virtualcall.cpp

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

NoQ added a comment. Thanks, works now! Apart from the minor comment on the hanging header file in the tests, this looks good and i have no further nits :) //*summons @dcoughlin to have a look at English in the warning messages*// Comment at:

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb added a comment. @NoQ In https://reviews.llvm.org/D34275#847434, @NoQ wrote: > Most importantly, do you think we can enable the checker by default now? Was > this checker evaluated on any large codebase, and if so, how many true/false > positives did it find, probably compared to

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb updated this revision to Diff 112110. wangxindsb added a comment. - Fix the errors of the tests; - Add `-analyzer-output=text` to the run-line; - Change the message of the visitor's diagnostic piece. https://reviews.llvm.org/D34275 Files:

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

NoQ added a comment. Most importantly, do you think we can enable the checker by default now? Was this checker evaluated on any large codebase, and if so, how many true/false positives did it find, probably compared to the old checker? https://reviews.llvm.org/D34275

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

NoQ added a comment. Tests are still not working - they pass now, but they don't actually test anything. Please make sure that tests actually work. Which means that 1. Tests pass when you run `make -j4 check-clang-analysis`; 2. Tests start failing when you change your code or expected-warning

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb added inline comments. Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:31 +} // end namespace + // FIXME: Ascending over StackFrameContext maybe another method. + Add the FIXME https://reviews.llvm.org/D34275

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb updated this revision to Diff 111862. wangxindsb added a comment. - Rename reportbug(); - Change message "Pure function" to "pure virtual function"; - Fixing: expected-warning; https://reviews.llvm.org/D34275 Files: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb updated this revision to Diff 111806. wangxindsb added a comment. +enum class ObjectState : bool { CtorCalled, DtorCalled }; +} // end namespace Add namespace closing comment. https://reviews.llvm.org/D34275 Files: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

xazax.hun added a subscriber: NoQ. xazax.hun added a comment. @NoQ , @dcoughlin could either of you review this patch as well? The end of GSoC is closing and it would be awesome to be able to commit this before it ends. :) Comment at:

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb updated this revision to Diff 110899. wangxindsb added a comment. Fix the Assertion Failed when run the checker to check the building of LibreOffice. https://reviews.llvm.org/D34275 Files: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp test/Analysis/virtualcall.cpp Index:

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb updated this revision to Diff 108236. wangxindsb added a comment. - Change the bugtype, just like the older checker. https://reviews.llvm.org/D34275 Files: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp test/Analysis/virtualcall.cpp Index: test/Analysis/virtualcall.cpp

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb updated this revision to Diff 106654. wangxindsb marked an inline comment as done. wangxindsb added a comment. - Change two maps to one map. - Move the declarations of PSM and SVB after the next early return. - Delete the variable std::string DeclName. - Delete last return in

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb marked 5 inline comments as done. wangxindsb added inline comments. Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:72 +REGISTER_MAP_WITH_PROGRAMSTATE(CtorMap, const MemRegion *, bool) +REGISTER_MAP_WITH_PROGRAMSTATE(DtorMap, const MemRegion *, bool) +

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:72 +REGISTER_MAP_WITH_PROGRAMSTATE(CtorMap, const MemRegion *, bool) +REGISTER_MAP_WITH_PROGRAMSTATE(DtorMap, const MemRegion *, bool) + I was wondering if there is

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb updated this revision to Diff 106243. wangxindsb added a comment. - Change function name to start with a lower case letter. - Use `StringRef` instead of `const char * ' for ReportBug() const. - Correct the braces for single statement blocks. https://reviews.llvm.org/D34275 Files:

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

xazax.hun added a comment. Thank you! I think we can start to run this check on real world code bases and evaluate the results. Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:41 + void checkPreCall(const CallEvent , CheckerContext ) const; + void

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb updated this revision to Diff 106204. wangxindsb added a comment. - Change IsVirtualCall(const CallExpr *CE) to be a free static function. - Rename some variables. - Improve the BugReporterVisitor, enclose the declaration names in single quotes. - Hoist getSValBuilder() from the if

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

xazax.hun added a comment. You are making a pretty good progress! I think right now there is some code duplication that could be reduced, but otherwise, the checker starts to look really good. Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:43 +private: + bool

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb added a comment. Look forward to your review. https://reviews.llvm.org/D34275 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb updated this revision to Diff 105760. wangxindsb marked an inline comment as done. wangxindsb added a comment. - Use the map of object to ctor/dtor to check the virtual call. - Use CXXInstanceCall and getCXXThisVal method to get the 'this' instead of getThisSVal(). - Correct some

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb updated this revision to Diff 105737. wangxindsb added a comment. - Change the two bugtype to one. - Use the generateErrorNode() for the pure virtual call. - Change the test case for the new expression. https://reviews.llvm.org/D34275 Files:

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb updated this revision to Diff 105738. wangxindsb added a comment. - Change the two bugtype to one. - Use the generateErrorNode() for the pure virtual call. - Change the test case for the new expression. https://reviews.llvm.org/D34275 Files:

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb added inline comments. Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:332 +// Check the base of the callexpr is equal to the this of the ctor +bool VirtualCallChecker::isCalledbyCtor(const CallExpr *CE,ProgramStateRef State,const LocationContext

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb updated this revision to Diff 105440. wangxindsb added a comment. - Rename the BugType. - Correct the code style to be clang format. - Rename the arguments to start with uppercase letter. - Add the support to check for the constructor called by the New expr. You don't need to put

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:31 +class VirtualCallChecker: public Checker { + mutable std::unique_ptr BT_CT; + mutable std::unique_ptr BT_DT; Could you find

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb updated this revision to Diff 104201. wangxindsb marked an inline comment as done. wangxindsb added a comment. -Fix the bug of the virtual call during a function call of the object. -Add flag for the PUREONLY. https://reviews.llvm.org/D34275 Files:

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb updated this revision to Diff 103832. wangxindsb added a comment. Add license to VirtualCallChecker.cpp https://reviews.llvm.org/D34275 Files: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp test/Analysis/virtualcall.cpp Index: test/Analysis/virtualcall.cpp

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb added a comment. > Oh, I think I see how it works now. How about: > > > struct A; > struct X { > void callFooOfA(A*); > }; > struct A { > A() { > X x; > x.virtualMethod(); // this virtual call is ok > x.callFooOfA(this) > } > virtual foo();

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb updated this revision to Diff 103214. wangxindsb added a comment. Correct some error in VirtualCallChecker.cpp. Complete the test case. https://reviews.llvm.org/D34275 Files: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp test/Analysis/virtualcall.cpp Index:

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb marked 5 inline comments as done. wangxindsb added inline comments. Comment at: VirtualCallChecker.cpp:260 + if (SFC->inTopFrame()) { + const FunctionDecl *FD = SFC->getDecl()->getAsFunction(); +if (!FD) xazax.hun wrote: > The formatting seems

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb updated this revision to Diff 103199. wangxindsb added a comment. Add run line in the test case. https://reviews.llvm.org/D34275 Files: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp test/Analysis/virtualcall.cpp Index: test/Analysis/virtualcall.cpp

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb updated this revision to Diff 103196. wangxindsb added a comment. Add test case for the virtual call checker. https://reviews.llvm.org/D34275 Files: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp test/Analysis/virtualcall.cpp Index: test/Analysis/virtualcall.cpp

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb added a comment. > What about: > > struct A { > A() { > X x; > x.virtualMethod(); // this virtual call is ok > foo(); // should warn here > } > virtual foo(); > } > int main() { > A a; > } > > > Does the checker warn on the second call?

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

xazax.hun added a comment. Note that when you update the differential revision you need to upload the whole diff. Your diff now only contains the tests but not the code. In https://reviews.llvm.org/D34275#785189, @wangxindsb wrote: > > How do you handle the following case? > > > > struct A

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb added a comment. > I do not see any test cases for this patch. Could you add them as well? I add the test case just now. > How do you handle the following case? > > struct A { > A() { > X x; > x.virtualMethod(); // this virtual call is ok > } > } > int

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb updated this revision to Diff 103180. wangxindsb added a comment. Add test case for the patch https://reviews.llvm.org/D34275 Files: virtualcall.cpp Index: virtualcall.cpp === --- virtualcall.cpp +++ virtualcall.cpp

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

xazax.hun added a comment. I do not see any test cases for this patch. Could you add them as well? Are you sure that this representation is ok? How do you handle the following case? struct A { A() { X x; x.virtualMethod(); // this virtual call is ok } } int main() {

[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

wangxindsb created this revision. This implement a path-sensitive checker that warns if virtual calls are made from constructors and destructors. The checker use the GDM (generic data map) to store three integers in the program state for constructors, destructors and objects. Once enter one of