Hi Jordan,
Implemented review comments.
1) Handled UnknownVal in VisitCXXDestructor.
2) Added more test cases (testNullDtorDerived, testNullAssigneddtor,
testNulldtorArg)
I found one more issue during testing. In case of double delete; there is no
actual stmt corresponding to
classInstance->~myDestructor call. This was resulting in a crash in
MallocChecker. Handled the same and added a test case.
Please let me know your input on the same.
Thanks!
Hi jordan_rose,
http://llvm-reviews.chandlerc.com/D1594
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D1594?vs=4090&id=4222#toc
Files:
test/Analysis/NewDelete-checker-test.cpp
test/Analysis/new.cpp
include/clang/Analysis/CFG.h
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/CallEvent.cpp
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
Index: test/Analysis/NewDelete-checker-test.cpp
===================================================================
--- test/Analysis/NewDelete-checker-test.cpp
+++ test/Analysis/NewDelete-checker-test.cpp
@@ -333,3 +333,16 @@
}
}
+// Test Deleting a value that's passed as an argument.
+class DerefClass{
+ public:
+ int *x;
+ DerefClass() {};
+ ~DerefClass() {*x = 1;}; //expected-warning {{Use of memory after it is freed}}
+};
+// make sure double delete doesn't crash
+void testDoubleDeleteClassInstance() {
+ DerefClass *foo = new DerefClass();
+ delete foo;
+ delete foo; //no crash
+}
Index: test/Analysis/new.cpp
===================================================================
--- test/Analysis/new.cpp
+++ test/Analysis/new.cpp
@@ -206,3 +206,135 @@
}
return 1;
}
+
+// Test modelling destructor call on call to delete
+class IntPair{
+ public:
+ int x;
+ int y;
+ IntPair() {};
+ ~IntPair() {x = x/y;}; //expected-warning {{Division by zero}}
+};
+
+void testCallToDestructor() {
+ IntPair *b = new IntPair();
+ b->x = 1;
+ b->y = 0;
+ delete b; // This results in divide by zero in destructor
+}
+
+// Test Deleting a value that's passed as an argument.
+class DerefClass{
+ public:
+ int *x;
+ DerefClass() {};
+ ~DerefClass() {*x = 1;}; //expected-warning {{Dereference of null pointer (loaded from field 'x')}}
+};
+
+void Fn(DerefClass *arg) {
+ delete arg;
+}
+
+void test_delte_dtor_Arg() {
+ DerefClass *pair = new DerefClass();
+ pair->x = 0;
+ Fn(pair);
+}
+
+//Deleting the address of a local variable, null pointer
+void abort(void) __attribute__((noreturn));
+
+class NoReturnDtor {
+ public:
+ NoReturnDtor() {}
+ ~NoReturnDtor() {abort();}
+};
+
+void test_delte_dtor_LocalVar() {
+ NoReturnDtor test;
+ delete &test; // no warn or crash
+}
+
+class DerivedNoReturn:public NoReturnDtor {
+ public:
+ DerivedNoReturn() {};
+ ~DerivedNoReturn() {};
+};
+
+void testNullDtorDerived() {
+ DerivedNoReturn *p = new DerivedNoReturn();
+ delete p; // Calls the base destructor which aborts, checked below
+ clang_analyzer_eval(true); // no warn
+}
+
+//Deleting a non class pointer should not crash/warn
+void test_var_delete() {
+ int *v = new int;
+ delete v; // no crash/warn
+ clang_analyzer_eval(true); // expected-warning{{TRUE}}
+}
+
+void testDeleteNull() {
+ NoReturnDtor *foo = 0;
+ delete foo; // should not call destructor, checked below
+ clang_analyzer_eval(true); // expected-warning{{TRUE}}
+}
+
+void testNullAssigneddtor() {
+ NoReturnDtor *p = 0;
+ NoReturnDtor *s = p;
+ delete s; // should not call destructor, checked below
+ clang_analyzer_eval(true); // expected-warning{{TRUE}}
+}
+
+void delteArg(NoReturnDtor *test) {
+ delete test;
+}
+
+void testNulldtorArg() {
+ NoReturnDtor *p = 0;
+ delteArg(p);
+ clang_analyzer_eval(true); // expected-warning{{TRUE}}
+}
+
+void testDeleteUnknown(NoReturnDtor *foo) {
+ delete foo; // should assume non-null and call noreturn destructor
+ clang_analyzer_eval(true); // no-warning
+}
+
+// Invalidate Region even in case of default destructor
+class InvalidateDestTest {
+ public:
+ int x;
+ int *y;
+ ~InvalidateDestTest();
+};
+
+int test_member_invalidation() {
+
+ //test invalidation of member variable
+ InvalidateDestTest *test = new InvalidateDestTest();
+ test->x = 5;
+ int *k = &(test->x);
+ clang_analyzer_eval(*k == 5); // expected-warning{{TRUE}}
+ delete test;
+ clang_analyzer_eval(*k == 5); // expected-warning{{UNKNOWN}}
+
+ //test invalidation of member pointer
+ int localVar = 5;
+ test = new InvalidateDestTest();
+ test->y = &localVar;
+ delete test;
+ clang_analyzer_eval(localVar == 5); // expected-warning{{UNKNOWN}}
+
+ // Test aray elements are invalidated.
+ int Var1 = 5;
+ int Var2 = 5;
+ InvalidateDestTest *a = new InvalidateDestTest[2];
+ a[0].y = &Var1;
+ a[1].y = &Var2;
+ delete[] a;
+ clang_analyzer_eval(Var1 == 5); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(Var2 == 5); // expected-warning{{UNKNOWN}}
+ return 0;
+}
Index: include/clang/Analysis/CFG.h
===================================================================
--- include/clang/Analysis/CFG.h
+++ include/clang/Analysis/CFG.h
@@ -200,7 +200,7 @@
}
// Get Delete expression which triggered the destructor call.
- const CXXDeleteExpr *getDeleteExpr() {
+ const CXXDeleteExpr *getDeleteExpr() const {
return static_cast<CXXDeleteExpr *>(Data2.getPointer());
}
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1790,7 +1790,7 @@
bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext &C,
const Stmt *S) const {
- if (isReleased(Sym, C)) {
+ if (isReleased(Sym, C) && S) {
ReportUseAfterFree(C, S->getSourceRange(), Sym);
return true;
}
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -569,7 +569,14 @@
void ExprEngine::ProcessDeleteDtor(const CFGDeleteDtor Dtor,
ExplodedNode *Pred,
ExplodedNodeSet &Dst) {
- //TODO: Handle DeleteDtor
+ const LocationContext *LCtx = Pred->getLocationContext();
+ const CXXDeleteExpr *DE = Dtor.getDeleteExpr();
+ const Stmt *Arg = cast<Stmt>(DE->getArgument());
+ SVal ArgVal = Pred->getState()->getSVal(Arg, LCtx);
+ VisitCXXDestructor(DE->getDestroyedType(),
+ ArgVal.getAsRegion(),
+ DE, /*IsBase=*/ false,
+ Pred, Dst);
}
void ExprEngine::ProcessBaseDtor(const CFGBaseDtor D,
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===================================================================
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -957,6 +957,8 @@
const Stmt *Trigger;
if (Optional<CFGAutomaticObjDtor> AutoDtor = E.getAs<CFGAutomaticObjDtor>())
Trigger = AutoDtor->getTriggerStmt();
+ else if (Optional<CFGDeleteDtor> DeleteDtor = E.getAs<CFGDeleteDtor>())
+ Trigger = cast<Stmt>(DeleteDtor->getDeleteExpr());
else
Trigger = Dtor->getBody();
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -296,10 +296,28 @@
// FIXME: We need to run the same destructor on every element of the array.
// This workaround will just run the first destructor (which will still
// invalidate the entire array).
+
+ // If the memory region is null do not call the
+ // destructors. Return a node with state same as previous node.
+ // Do not process further.
+ if (!Dest) {
+ StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+ Bldr.generateNode(S, Pred, State);
+ return;
+ }
SVal DestVal = loc::MemRegionVal(Dest);
DestVal = makeZeroElementRegion(State, DestVal, ObjectType);
+ // If retured SVal is Unknown or the SVal is null
+ // return a node with same state.
+ // Do not process further.
+ if (DestVal.isUnknown() ||
+ State->isNull(DestVal).isConstrainedTrue()) {
+ StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+ Bldr.generateNode(S, Pred, State);
+ return;
+ }
+
Dest = DestVal.getAsRegion();
-
const CXXRecordDecl *RecordDecl = ObjectType->getAsCXXRecordDecl();
assert(RecordDecl && "Only CXXRecordDecls should have destructors");
const CXXDestructorDecl *DtorDecl = RecordDecl->getDestructor();
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
