r191381 - [analyzer] Handle destructors for the argument to C++ 'delete'.
Jordan Rose
jordan_rose at apple.com
Wed Sep 25 09:06:18 PDT 2013
Author: jrose
Date: Wed Sep 25 11:06:17 2013
New Revision: 191381
URL: http://llvm.org/viewvc/llvm-project?rev=191381&view=rev
Log:
[analyzer] Handle destructors for the argument to C++ 'delete'.
Now that the CFG includes nodes for the destructors in a delete-expression,
process them in the analyzer using the same common destructor interface
currently used for local, member, and base destructors. Also, check for when
the value is known to be null, in which case no destructor is actually run.
This does not yet handle destructors for deleted /arrays/, which may need
more CFG work. It also causes a slight regression in the location of
double delete warnings; the double delete is detected at the destructor
call, which is implicit, and so is reported on the first access within the
destructor instead of at the 'delete' statement. This will be fixed soon.
Patch by Karthik Bhat!
Modified:
cfe/trunk/include/clang/Analysis/CFG.h
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
cfe/trunk/test/Analysis/NewDelete-checker-test.cpp
cfe/trunk/test/Analysis/new.cpp
Modified: cfe/trunk/include/clang/Analysis/CFG.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/CFG.h?rev=191381&r1=191380&r2=191381&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/CFG.h (original)
+++ cfe/trunk/include/clang/Analysis/CFG.h Wed Sep 25 11:06:17 2013
@@ -200,7 +200,7 @@ public:
}
// Get Delete expression which triggered the destructor call.
- const CXXDeleteExpr *getDeleteExpr() {
+ const CXXDeleteExpr *getDeleteExpr() const {
return static_cast<CXXDeleteExpr *>(Data2.getPointer());
}
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=191381&r1=191380&r2=191381&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Wed Sep 25 11:06:17 2013
@@ -1803,7 +1803,8 @@ bool MallocChecker::isReleased(SymbolRef
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;
}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp?rev=191381&r1=191380&r2=191381&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp Wed Sep 25 11:06:17 2013
@@ -964,6 +964,8 @@ CallEventManager::getCaller(const StackF
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();
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=191381&r1=191380&r2=191381&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Wed Sep 25 11:06:17 2013
@@ -569,7 +569,30 @@ void ExprEngine::ProcessAutomaticObjDtor
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 = DE->getArgument();
+ SVal ArgVal = State->getSVal(Arg, LCtx);
+
+ // If the argument to delete is known to be a null value,
+ // don't run destructor.
+ if (State->isNull(ArgVal).isConstrainedTrue()) {
+ QualType DTy = DE->getDestroyedType();
+ QualType BTy = getContext().getBaseElementType(DTy);
+ const CXXRecordDecl *RD = BTy->getAsCXXRecordDecl();
+ const CXXDestructorDecl *Dtor = RD->getDestructor();
+
+ PostImplicitCall PP(Dtor, 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,
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=191381&r1=191380&r2=191381&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Wed Sep 25 11:06:17 2013
@@ -296,7 +296,9 @@ void ExprEngine::VisitCXXDestructor(Qual
// 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();
Modified: cfe/trunk/test/Analysis/NewDelete-checker-test.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/NewDelete-checker-test.cpp?rev=191381&r1=191380&r2=191381&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/NewDelete-checker-test.cpp (original)
+++ cfe/trunk/test/Analysis/NewDelete-checker-test.cpp Wed Sep 25 11:06:17 2013
@@ -333,3 +333,28 @@ namespace reference_count {
}
}
+// 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}}
+}
Modified: cfe/trunk/test/Analysis/new.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/new.cpp?rev=191381&r1=191380&r2=191381&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/new.cpp (original)
+++ cfe/trunk/test/Analysis/new.cpp Wed Sep 25 11:06:17 2013
@@ -206,3 +206,148 @@ int testNoInitializationPlacement() {
}
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
+}
+
+void testArrayNull() {
+ NoReturnDtor *fooArray = 0;
+ delete[] fooArray; // should not call destructor, checked below
+ clang_analyzer_eval(true); // expected-warning{{TRUE}}
+}
+
+void testArrayDestr() {
+ NoReturnDtor *p = new NoReturnDtor[2];
+ delete[] p; // Calls the base destructor which aborts, checked below
+ //TODO: clang_analyzer_eval should not be called
+ clang_analyzer_eval(true); // expected-warning{{TRUE}}
+}
+
+// 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;
+}
More information about the cfe-commits
mailing list