NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
In the `c++-allocator-inlining=true` mode, we need to make the assumption that
the conservatively evaluated operator new() has returned a non-null value.
Previously we did this on `CXXNewExpr`, but now we have to do that before
calling the constructor, because some clever constructors are sometimes
assuming that their `this` is null and doing weird stuff. We would also crash
upon evaluating `CXXNewExpr` when the allocator was inlined and returned null
and had a throw specification; this is UB even for custom allocators, but we
still need not to crash.
Added more FIXME tests to ensure that eventually we fix calling the constructor
for null return values.
Repository:
rC Clang
https://reviews.llvm.org/D42192
Files:
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
test/Analysis/new-ctor-conservative.cpp
test/Analysis/new-ctor-null-throw.cpp
test/Analysis/new-ctor-null.cpp
Index: test/Analysis/new-ctor-null.cpp
===================================================================
--- test/Analysis/new-ctor-null.cpp
+++ test/Analysis/new-ctor-null.cpp
@@ -1,6 +1,7 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
void clang_analyzer_eval(bool);
+void clang_analyzer_warnIfReached();
typedef __typeof__(sizeof(int)) size_t;
@@ -13,11 +14,23 @@
struct S {
int x;
- S() : x(1) {}
+ S() : x(1) {
+ // FIXME: Constructor should not be called with null this, even if it was
+ // returned by operator new().
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+ }
~S() {}
};
void testArrays() {
S *s = new S[10]; // no-crash
s[0].x = 2; // expected-warning{{Dereference of null pointer}}
}
+
+int global;
+void testInvalidationOnConstructionIntoNull() {
+ global = 0;
+ S *s = new S();
+ // FIXME: Should be FALSE - we should not invalidate globals.
+ clang_analyzer_eval(global); // expected-warning{{UNKNOWN}}
+}
Index: test/Analysis/new-ctor-null-throw.cpp
===================================================================
--- /dev/null
+++ test/Analysis/new-ctor-null-throw.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
+
+void clang_analyzer_eval(bool);
+
+typedef __typeof__(sizeof(int)) size_t;
+
+
+// These are ill-formed. One cannot return nullptr from a throwing version of an
+// operator new.
+void *operator new(size_t size) {
+ return nullptr;
+}
+void *operator new[](size_t size) {
+ return nullptr;
+}
+
+struct S {
+ int x;
+ S() : x(1) {}
+ ~S() {}
+};
+
+void testArrays() {
+ S *s = new S[10]; // no-crash
+ s[0].x = 2; // expected-warning{{Dereference of null pointer}}
+}
Index: test/Analysis/new-ctor-conservative.cpp
===================================================================
--- test/Analysis/new-ctor-conservative.cpp
+++ test/Analysis/new-ctor-conservative.cpp
@@ -1,6 +1,7 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
void clang_analyzer_eval(bool);
+void clang_analyzer_warnIfReached();
struct S {
int x;
@@ -27,3 +28,19 @@
// FIXME: Should be true once we inline array constructors.
clang_analyzer_eval(s[0].x == 1); // expected-warning{{UNKNOWN}}
}
+
+struct NullS {
+ NullS() {
+ if (this) {}
+ }
+ NullS(int x) {
+ if (!this) {
+ clang_analyzer_warnIfReached(); // no-warning
+ }
+ }
+};
+
+void checkNullThis() {
+ NullS *nulls = new NullS(); // no-crash
+ NullS *nulls2 = new NullS(0);
+}
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -500,9 +500,24 @@
// is breaking too much to evaluate the no-op symbolic cast over it, so we
// skip it for now.
ProgramStateRef State = I->getState();
- ValueBldr.generateNode(
- CNE, I,
- setCXXNewAllocatorValue(State, CNE, LCtx, State->getSVal(CNE, LCtx)));
+ SVal RetVal = State->getSVal(CNE, LCtx);
+
+ // If this allocation function is not declared as non-throwing, failures
+ // /must/ be signalled by exceptions, and thus the return value will never
+ // be NULL. -fno-exceptions does not influence this semantics.
+ // FIXME: GCC has a -fcheck-new option, which forces it to consider the case
+ // where new can return NULL. If we end up supporting that option, we can
+ // consider adding a check for it here.
+ // C++11 [basic.stc.dynamic.allocation]p3.
+ if (const FunctionDecl *FD = CNE->getOperatorNew()) {
+ QualType Ty = FD->getType();
+ if (const FunctionProtoType *ProtoType = Ty->getAs<FunctionProtoType>())
+ if (!ProtoType->isNothrow(getContext()))
+ State = State->assume(RetVal.castAs<DefinedOrUnknownSVal>(), true);
+ }
+
+ ValueBldr.generateNode(CNE, I,
+ setCXXNewAllocatorValue(State, CNE, LCtx, RetVal));
}
ExplodedNodeSet DstPostPostCallCallback;
@@ -559,21 +574,21 @@
State = Call->invalidateRegions(blockCount);
if (!State)
return;
- }
- // If this allocation function is not declared as non-throwing, failures
- // /must/ be signalled by exceptions, and thus the return value will never be
- // NULL. -fno-exceptions does not influence this semantics.
- // FIXME: GCC has a -fcheck-new option, which forces it to consider the case
- // where new can return NULL. If we end up supporting that option, we can
- // consider adding a check for it here.
- // C++11 [basic.stc.dynamic.allocation]p3.
- if (FD) {
- QualType Ty = FD->getType();
- if (const FunctionProtoType *ProtoType = Ty->getAs<FunctionProtoType>())
- if (!ProtoType->isNothrow(getContext()))
- if (auto dSymVal = symVal.getAs<DefinedOrUnknownSVal>())
- State = State->assume(*dSymVal, true);
+ // If this allocation function is not declared as non-throwing, failures
+ // /must/ be signalled by exceptions, and thus the return value will never
+ // be NULL. -fno-exceptions does not influence this semantics.
+ // FIXME: GCC has a -fcheck-new option, which forces it to consider the case
+ // where new can return NULL. If we end up supporting that option, we can
+ // consider adding a check for it here.
+ // C++11 [basic.stc.dynamic.allocation]p3.
+ if (FD) {
+ QualType Ty = FD->getType();
+ if (const FunctionProtoType *ProtoType = Ty->getAs<FunctionProtoType>())
+ if (!ProtoType->isNothrow(getContext()))
+ if (auto dSymVal = symVal.getAs<DefinedOrUnknownSVal>())
+ State = State->assume(*dSymVal, true);
+ }
}
StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
