NoQ updated this revision to Diff 127549.
NoQ added a comment.
Rebase.
> I also noticed that `evalCast` from `void *` to `T *` is uncomfortable to use
> because sometimes it transforms `&SymRegion{$x}` into `&element{T, 0S32b,
> SymRegion{$x}}` even when `$x` is already of type `T *`. The form
> `&SymRegion{$x}` seems to be the canonical form of this symbolic pointer
> value in the rest of the analyzer, so i decided to change `evalCast` to
> preserve it.
Suddenly it turns out that this is not needed anymore. I'm struggling quite a
bit to get the casts right, and still failing to identify the actual system
we're trying to follow when representing pointer casts. I'd love to get to the
bottom of it eventually.
https://reviews.llvm.org/D41250
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
test/Analysis/new-ctor-conservative.cpp
test/Analysis/new-ctor-inlined.cpp
Index: test/Analysis/new-ctor-inlined.cpp
===================================================================
--- test/Analysis/new-ctor-inlined.cpp
+++ test/Analysis/new-ctor-inlined.cpp
@@ -27,3 +27,18 @@
// Check that bindings are correct (and also not dying).
clang_analyzer_eval(s->x == 1); // expected-warning{{TRUE}}
}
+
+void checkNewPOD() {
+ int *i = new int;
+ clang_analyzer_eval(*i == 0); // expected-warning{{UNKNOWN}}
+ int *j = new int();
+ clang_analyzer_eval(*j == 0); // expected-warning{{TRUE}}
+ int *k = new int(5);
+ clang_analyzer_eval(*k == 5); // expected-warning{{TRUE}}
+}
+
+void checkTrivialCopy() {
+ S s;
+ S *t = new S(s); // no-crash
+ clang_analyzer_eval(t->x == 1); // expected-warning{{TRUE}}
+}
Index: test/Analysis/new-ctor-conservative.cpp
===================================================================
--- test/Analysis/new-ctor-conservative.cpp
+++ test/Analysis/new-ctor-conservative.cpp
@@ -12,3 +12,12 @@
S *s = new S;
clang_analyzer_eval(s->x == 1); // expected-warning{{TRUE}}
}
+
+void checkNewPOD() {
+ int *i = new int;
+ clang_analyzer_eval(*i == 0); // expected-warning{{UNKNOWN}}
+ int *j = new int();
+ clang_analyzer_eval(*j == 0); // expected-warning{{TRUE}}
+ int *k = new int(5);
+ clang_analyzer_eval(*k == 5); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -277,11 +277,12 @@
state = state->BindExpr(CCE, callerCtx, ThisV);
}
- if (isa<CXXNewExpr>(CE)) {
+ if (const auto *CNE = dyn_cast<CXXNewExpr>(CE)) {
// We are currently evaluating a CXXNewAllocator CFGElement. It takes a
// while to reach the actual CXXNewExpr element from here, so keep the
// region for later use.
- state = pushCXXNewAllocatorValue(state, state->getSVal(CE, callerCtx));
+ state = pushCXXNewAllocatorValue(state, state->getSVal(CE, callerCtx),
+ CNE->getType());
}
}
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -463,7 +463,8 @@
for (auto I : DstPostCall) {
ProgramStateRef State = I->getState();
ValueBldr.generateNode(
- CNE, I, pushCXXNewAllocatorValue(State, State->getSVal(CNE, LCtx)));
+ CNE, I, pushCXXNewAllocatorValue(State, State->getSVal(CNE, LCtx),
+ CNE->getType()));
}
getCheckerManager().runCheckersForPostCall(Dst, DstPostValue,
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -314,7 +314,12 @@
}
ProgramStateRef ExprEngine::pushCXXNewAllocatorValue(ProgramStateRef State,
- SVal V) {
+ SVal V, QualType T) {
+ // Because operator new returns void *, we need to perform the cast here,
+ // which is implied by the semantics of operator new.
+ V = svalBuilder.evalCast(V, T,
+ getContext().getPointerType(getContext().VoidTy));
+
return State->add<CXXNewAllocatorValueStack>(V);
}
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -659,8 +659,10 @@
/// Store the region returned by operator new() so that the constructor
/// that follows it knew what location to initialize. The value should be
/// cleared once the respective CXXNewExpr CFGStmt element is processed.
+ /// \p T is the type of the newly allocated object, which is not necessarily
+ /// obvious from \p V because the return type of operator new is void *.
ProgramStateRef pushCXXNewAllocatorValue(ProgramStateRef State,
- SVal V);
+ SVal V, QualType T);
/// Retrieve the location returned by the current operator new().
SVal peekCXXNewAllocatorValue(ProgramStateRef State);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
