Oops thanks for catching the crash. I was not able to see it in release mode
when i tested.
The crash is due to the type of node we are creating.
We are trying to use StmtNodeBuilder to create a node when actually we do not
have a Stmt associated with it!
What i think we need here is to create a node with PostImplicitCall.
When we enqueue it onto worklist we do not create extra nodes corresponding
to this implicit call stmt and move to the next CFG element. This fixes the
crash found in regression on debug mode.
Thanks!
Hi jordan_rose, zaks.anna,
http://llvm-reviews.chandlerc.com/D1594
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D1594?vs=4320&id=4392#toc
Files:
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/CallEvent.cpp
test/Analysis/new.cpp
test/Analysis/NewDelete-checker-test.cpp
include/clang/Analysis/CFG.h
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1803,7 +1803,8 @@
bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext &C,
const Stmt *S) const {
- if (isReleased(Sym, C)) {
+ // FIXME: Handle destructor called from delete more precisely.
+ if (isReleased(Sym, C) && S) {
ReportUseAfterFree(C, S->getSourceRange(), Sym);
return true;
}
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -296,7 +296,9 @@
// 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).
- SVal DestVal = loc::MemRegionVal(Dest);
+ SVal DestVal = UnknownVal();
+ if (Dest)
+ DestVal = loc::MemRegionVal(Dest);
DestVal = makeZeroElementRegion(State, DestVal, ObjectType);
Dest = DestVal.getAsRegion();
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -569,7 +569,26 @@
void ExprEngine::ProcessDeleteDtor(const CFGDeleteDtor Dtor,
ExplodedNode *Pred,
ExplodedNodeSet &Dst) {
- //TODO: Handle DeleteDtor
+ ProgramStateRef State = Pred->getState();
+ const LocationContext *LCtx = Pred->getLocationContext();
+ const CXXDeleteExpr *DE = Dtor.getDeleteExpr();
+ const Stmt *Arg = cast<Stmt>(DE->getArgument());
+ SVal ArgVal = Pred->getState()->getSVal(Arg, LCtx);
+ // If it is known to be bound to a null value
+ // dont run destructor.
+ if (State->isNull(ArgVal).isConstrainedTrue()) {
+ QualType DTy = DE->getDestroyedType();
+ DTy = DTy.getNonReferenceType();
+ CXXRecordDecl *RD = getContext().getBaseElementType(DTy)->getAsCXXRecordDecl();
+ PostImplicitCall PP(RD, DE->getLocStart(), LCtx);
+ NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+ Bldr.generateNode(PP, Pred->getState(), Pred);
+ return;
+ }
+ 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
@@ -963,6 +963,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: 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 testDestCall(DerefClass *arg) {
+ delete arg;
+}
+
+void test_delete_dtor_Arg() {
+ DerefClass *pair = new DerefClass();
+ pair->x = 0;
+ testDestCall(pair);
+}
+
+//Deleting the address of a local variable, null pointer
+void abort(void) __attribute__((noreturn));
+
+class NoReturnDtor {
+public:
+ NoReturnDtor() {}
+ ~NoReturnDtor() {abort();}
+};
+
+void test_delete_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 deleteArg(NoReturnDtor *test) {
+ delete test;
+}
+
+void testNulldtorArg() {
+ NoReturnDtor *p = 0;
+ deleteArg(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: test/Analysis/NewDelete-checker-test.cpp
===================================================================
--- test/Analysis/NewDelete-checker-test.cpp
+++ test/Analysis/NewDelete-checker-test.cpp
@@ -333,3 +333,29 @@
}
}
+// Test double delete
+class DerefClass{
+public:
+ int *x;
+ DerefClass() {}
+ ~DerefClass() {*x = 1;} //expected-warning {{Use of memory after it is freed}}
+};
+
+void testDoubleDeleteClassInstance() {
+ DerefClass *foo = new DerefClass();
+ delete foo;
+ delete foo; // FIXME: We should ideally report warning here instead of inside the destructor.
+}
+
+class EmptyClass{
+public:
+ EmptyClass() {}
+ ~EmptyClass() {}
+};
+
+void testDoubleDeleteEmptyClass() {
+ EmptyClass *foo = new EmptyClass();
+ delete foo;
+ delete foo; //expected-warning {{Attempt to free released memory}}
+}
+
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());
}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
