NoQ created this revision.
In `ProgramState::getSVal(Loc, QualType)`, the `QualType` parameter, which
represents the type of the expected value, is optional. If it is not supplied,
it is auto-detected by looking at the memory region in `Loc`. For typed-value
regions such autodetection is trivial. For other kinds of regions (most
importantly, for symbolic regions) it apparently never worked correctly: it
detected the location type (pointer type), not the value type in this location
(pointee type).
Our tests contain numerous cases when such autodetection was working
incorrectly, but for existing tests it never mattered. There are three notable
places where the type was regularly auto-detected incorrectly:
1. `ExprEngine::performTrivialCopy()`. Trivial copy from symbolic references
never worked - test case attached.
2. `CallAndMessageChecker::uninitRefOrPointer()`. Here the code only cares if
the value is `Undefined` or not, so autodetected type didn't matter.
3. `GTestChecker::modelAssertionResultBoolConstructor()`. This is how the issue
was found in https://bugs.llvm.org/show_bug.cgi?id=34305 - test case attached.
I added a few more sanity checks - we'd now also fail with an assertion if we
are unable to autodetect the pointer type, warning the author of the checker to
pass the type explicitly.
It is sometimes indeed handy to put a void-pointer-typed `Loc` into
`getSVal(Loc)`, as demonstrated in the `exercise-ps.c`'s `f2()` test through
`CallAndMessageChecker` (case '2.' above). I handled this on the API side in
order to simplify checker API, implicitly turning `void` into `char`, though i
don't mind modifying `CallAndMessageChecker` to pass `CharTy` explicitly
instead.
https://reviews.llvm.org/D38358
Files:
lib/StaticAnalyzer/Core/RegionStore.cpp
test/Analysis/ctor.mm
test/Analysis/gtest.cpp
Index: test/Analysis/gtest.cpp
===================================================================
--- test/Analysis/gtest.cpp
+++ test/Analysis/gtest.cpp
@@ -151,3 +151,9 @@
ASSERT_TRUE(false);
clang_analyzer_warnIfReached(); // no-warning
}
+
+void testAssertSymbolic(const bool &b) {
+ ASSERT_TRUE(b);
+
+ clang_analyzer_eval(b); // expected-warning{{UNKNOWN}}
+}
Index: test/Analysis/ctor.mm
===================================================================
--- test/Analysis/ctor.mm
+++ test/Analysis/ctor.mm
@@ -199,7 +199,7 @@
Inner p;
};
- void testPOD() {
+ void testPOD(const POD &pp) {
POD p;
p.x = 1;
POD p2 = p; // no-warning
@@ -210,6 +210,15 @@
// Use rvalues as well.
clang_analyzer_eval(POD(p3).x == 1); // expected-warning{{TRUE}}
+ // Copy from symbolic references correctly.
+ POD p4 = pp;
+ // Make sure that p4.x contains a symbol after copy.
+ if (p4.x > 0)
+ clang_analyzer_eval(p4.x > 0); // expected-warning{{TRUE}}
+ // FIXME: Element region gets in the way, so these aren't the same symbols
+ // as they should be.
+ clang_analyzer_eval(pp.x == p4.x); // expected-warning{{UNKNOWN}}
+
PODWrapper w;
w.p.y = 1;
PODWrapper w2 = w; // no-warning
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1393,17 +1393,20 @@
return UnknownVal();
}
- if (isa<AllocaRegion>(MR) ||
- isa<SymbolicRegion>(MR) ||
- isa<CodeTextRegion>(MR)) {
+ if (!isa<TypedValueRegion>(MR)) {
if (T.isNull()) {
- if (const TypedRegion *TR = dyn_cast<TypedRegion>(MR))
- T = TR->getLocationType();
- else {
- const SymbolicRegion *SR = cast<SymbolicRegion>(MR);
- T = SR->getSymbol()->getType();
+ if (const TypedRegion *TR = dyn_cast<TypedRegion>(MR)) {
+ T = TR->getLocationType()->getPointeeType();
+ } else if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(MR)) {
+ T = SR->getSymbol()->getType()->getPointeeType();
+ if (T->isVoidType()) {
+ // When trying to dereference a void pointer, read the first byte.
+ T = Ctx.CharTy;
+ }
}
}
+ assert(!T.isNull() && "Unable to auto-detect binding type!");
+ assert(!T->isVoidType() && "Attempted to retrieve a void value!");
MR = GetElementZeroRegion(cast<SubRegion>(MR), T);
}
Index: test/Analysis/gtest.cpp
===================================================================
--- test/Analysis/gtest.cpp
+++ test/Analysis/gtest.cpp
@@ -151,3 +151,9 @@
ASSERT_TRUE(false);
clang_analyzer_warnIfReached(); // no-warning
}
+
+void testAssertSymbolic(const bool &b) {
+ ASSERT_TRUE(b);
+
+ clang_analyzer_eval(b); // expected-warning{{UNKNOWN}}
+}
Index: test/Analysis/ctor.mm
===================================================================
--- test/Analysis/ctor.mm
+++ test/Analysis/ctor.mm
@@ -199,7 +199,7 @@
Inner p;
};
- void testPOD() {
+ void testPOD(const POD &pp) {
POD p;
p.x = 1;
POD p2 = p; // no-warning
@@ -210,6 +210,15 @@
// Use rvalues as well.
clang_analyzer_eval(POD(p3).x == 1); // expected-warning{{TRUE}}
+ // Copy from symbolic references correctly.
+ POD p4 = pp;
+ // Make sure that p4.x contains a symbol after copy.
+ if (p4.x > 0)
+ clang_analyzer_eval(p4.x > 0); // expected-warning{{TRUE}}
+ // FIXME: Element region gets in the way, so these aren't the same symbols
+ // as they should be.
+ clang_analyzer_eval(pp.x == p4.x); // expected-warning{{UNKNOWN}}
+
PODWrapper w;
w.p.y = 1;
PODWrapper w2 = w; // no-warning
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1393,17 +1393,20 @@
return UnknownVal();
}
- if (isa<AllocaRegion>(MR) ||
- isa<SymbolicRegion>(MR) ||
- isa<CodeTextRegion>(MR)) {
+ if (!isa<TypedValueRegion>(MR)) {
if (T.isNull()) {
- if (const TypedRegion *TR = dyn_cast<TypedRegion>(MR))
- T = TR->getLocationType();
- else {
- const SymbolicRegion *SR = cast<SymbolicRegion>(MR);
- T = SR->getSymbol()->getType();
+ if (const TypedRegion *TR = dyn_cast<TypedRegion>(MR)) {
+ T = TR->getLocationType()->getPointeeType();
+ } else if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(MR)) {
+ T = SR->getSymbol()->getType()->getPointeeType();
+ if (T->isVoidType()) {
+ // When trying to dereference a void pointer, read the first byte.
+ T = Ctx.CharTy;
+ }
}
}
+ assert(!T.isNull() && "Unable to auto-detect binding type!");
+ assert(!T->isVoidType() && "Attempted to retrieve a void value!");
MR = GetElementZeroRegion(cast<SubRegion>(MR), T);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
