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&id=112857#toc
Repository:
rL LL
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 y
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
http://lists.llvm.org/cgi-bin/mailman/li
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
test/Analys
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: test/Analysis/virtualcall.cpp:15-
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 th
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:
lib/StaticAnalyzer/Checkers/VirtualCallChec
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
___
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 di
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
__
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
test/Analysis
wangxindsb marked 3 inline comments as done.
wangxindsb added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:29
namespace {
+enum class ObjectState : bool { CtorCalled, DtorCalled };
+} // end namespace
NoQ wrote:
> Please remind
NoQ added a comment.
First of all, thanks everybody for working on this. I'd see what i can do.
Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:29
namespace {
+enum class ObjectState : bool { CtorCalled, DtorCalled };
+} // end namespace
Please
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
test/Analysis
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: lib/StaticAnalyzer/Checkers/VirtualCallCh
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: t
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
=
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 reportbug(
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)
+
-
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 an
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:
l
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 &Call, CheckerContext &C) const;
+ void Chang
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 sta
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
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
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 forma
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:
lib/StaticAnalyzer/Checkers/VirtualCallC
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:
lib/StaticAnalyzer/Checkers/VirtualCallC
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 *LCtx)
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 effor
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 more descriptive names for these BugT
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:
lib/StaticAnalyzer/Checkers/Virtu
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
==
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();
xazax.hun added a comment.
In https://reviews.llvm.org/D34275#785294, @wangxindsb wrote:
> > What about:
> >
> > struct A {
> > A() {
> > X x;
> > x.virtualMethod(); // this virtual call is ok
> > foo(); // should warn here
> > }
> > virtual foo();
> > }
> > i
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: test/Analysis/vi
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 to
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
=
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
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?
Yes,
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 {
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 main(
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
@
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() {
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
45 matches
Mail list logo