NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs, mehdi_amini.
Similarly to how we allow (since https://reviews.llvm.org/D40560) inlining the
constructor after `operator new` which isn't `operator new[]`, even if the
target region is an `ElementRegion` (which doesn't necessarily represent an
array element - it may represent a result of pointer arithmetic or a cast), we
should allow inlining the destructors for non-array-but-still-element regions,
but not when they are part of `operator delete[]`. There aren't any known
issues in this situation. We still aren't attempting to model array new/delete
because it requires an unknown amount of constructor calls to be modeled
symbolically.
Before the patch, in `new.cpp` tests `testCallToDestructor()` and
`test_delete_dtor_Arg()` started failing under `-analyzer-config
c++-allocator-inlining=true`, because the new behavior of `operator new` is to
return an `ElementRegion` surrounding the void pointer, which disables
destructor inlining; the old behavior is to call the destructor over a raw void
pointer, which kind of worked. Additionally, some fixmes in `new.cpp` were
fixed in the new mode. The change in `testPlacementNew()` also seems correct,
even if it wasn't marked as a FIXME.
Repository:
rC Clang
https://reviews.llvm.org/D41795
Files:
lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
test/Analysis/new.cpp
Index: test/Analysis/new.cpp
===================================================================
--- test/Analysis/new.cpp
+++ test/Analysis/new.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_analyze_cc1
-analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store region
-std=c++11 -verify %s
+// RUN: %clang_analyze_cc1
-analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store region
-analyzer-config c++-allocator-inlining=true -DALLOCATOR_INLINING=1 -std=c++11
-verify %s
#include "Inputs/system-header-simulator-cxx.h"
void clang_analyzer_eval(bool);
@@ -34,7 +35,12 @@
void *y = new (x) int;
clang_analyzer_eval(x == y); // expected-warning{{TRUE}};
- clang_analyzer_eval(*x == 1); // expected-warning{{UNKNOWN}};
+ clang_analyzer_eval(*x == 1);
+#if ALLOCATOR_INLINING
+ // expected-warning@-2{{TRUE}};
+#else
+ // expected-warning@-4{{UNKNOWN}};
+#endif
return y;
}
@@ -201,7 +207,10 @@
new (&n) int;
// Should warn that n is uninitialized.
- if (n) { // no-warning
+ if (n) {
+#if ALLOCATOR_INLINING
+ // expected-warning@-2{{Branch condition evaluates to a garbage value}}
+#endif
return 0;
}
return 1;
@@ -311,8 +320,7 @@
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}}
+ clang_analyzer_eval(true);
}
// Invalidate Region even in case of default destructor
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -687,8 +687,20 @@
// FIXME: We don't handle constructors or destructors for arrays properly.
const MemRegion *Target = Dtor.getCXXThisVal().getAsRegion();
- if (Target && isa<ElementRegion>(Target))
- return CIP_DisallowedOnce;
+ if (Target && isa<ElementRegion>(Target)) {
+ if (const Stmt *DtorExpr = Dtor.getOriginExpr())
+ if (const Stmt *ParentExpr =
+ CurLC->getParentMap().getParent(DtorExpr))
+ if (const CXXDeleteExpr *DeleteExpr =
+ dyn_cast<CXXDeleteExpr>(ParentExpr))
+ if (DeleteExpr->isArrayForm())
+ return CIP_DisallowedOnce;
+
+ if (const TypedValueRegion *TR = dyn_cast<TypedValueRegion>(
+ cast<SubRegion>(Target)->getSuperRegion()))
+ if (TR->getValueType()->isArrayType())
+ return CIP_DisallowedOnce;
+ }
break;
}
Index: test/Analysis/new.cpp
===================================================================
--- test/Analysis/new.cpp
+++ test/Analysis/new.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store region -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store region -analyzer-config c++-allocator-inlining=true -DALLOCATOR_INLINING=1 -std=c++11 -verify %s
#include "Inputs/system-header-simulator-cxx.h"
void clang_analyzer_eval(bool);
@@ -34,7 +35,12 @@
void *y = new (x) int;
clang_analyzer_eval(x == y); // expected-warning{{TRUE}};
- clang_analyzer_eval(*x == 1); // expected-warning{{UNKNOWN}};
+ clang_analyzer_eval(*x == 1);
+#if ALLOCATOR_INLINING
+ // expected-warning@-2{{TRUE}};
+#else
+ // expected-warning@-4{{UNKNOWN}};
+#endif
return y;
}
@@ -201,7 +207,10 @@
new (&n) int;
// Should warn that n is uninitialized.
- if (n) { // no-warning
+ if (n) {
+#if ALLOCATOR_INLINING
+ // expected-warning@-2{{Branch condition evaluates to a garbage value}}
+#endif
return 0;
}
return 1;
@@ -311,8 +320,7 @@
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}}
+ clang_analyzer_eval(true);
}
// Invalidate Region even in case of default destructor
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -687,8 +687,20 @@
// FIXME: We don't handle constructors or destructors for arrays properly.
const MemRegion *Target = Dtor.getCXXThisVal().getAsRegion();
- if (Target && isa<ElementRegion>(Target))
- return CIP_DisallowedOnce;
+ if (Target && isa<ElementRegion>(Target)) {
+ if (const Stmt *DtorExpr = Dtor.getOriginExpr())
+ if (const Stmt *ParentExpr =
+ CurLC->getParentMap().getParent(DtorExpr))
+ if (const CXXDeleteExpr *DeleteExpr =
+ dyn_cast<CXXDeleteExpr>(ParentExpr))
+ if (DeleteExpr->isArrayForm())
+ return CIP_DisallowedOnce;
+
+ if (const TypedValueRegion *TR = dyn_cast<TypedValueRegion>(
+ cast<SubRegion>(Target)->getSuperRegion()))
+ if (TR->getValueType()->isArrayType())
+ return CIP_DisallowedOnce;
+ }
break;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
