Szelethus updated this revision to Diff 155913.
Szelethus added a comment.
Rebased to the latest trunk.
https://reviews.llvm.org/D49199
Files:
lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
test/Analysis/cxx-uninitialized-object-inheritance.cpp
test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
Index: test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
===================================================================
--- test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
+++ test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
@@ -191,7 +191,7 @@
struct CyclicPointerTest {
int *ptr;
- CyclicPointerTest() : ptr(reinterpret_cast<int*>(&ptr)) {}
+ CyclicPointerTest() : ptr(reinterpret_cast<int *>(&ptr)) {}
};
void fCyclicPointerTest() {
@@ -280,13 +280,62 @@
void *vptr; // no-crash
CyclicVoidPointerTest() : vptr(&vptr) {}
-
};
void fCyclicVoidPointerTest() {
CyclicVoidPointerTest();
}
+struct IntDynTypedVoidPointerTest1 {
+ void *vptr; // expected-note{{uninitialized pointee 'this->vptr'}}
+ int dontGetFilteredByNonPedanticMode = 0;
+
+ IntDynTypedVoidPointerTest1(void *vptr) : vptr(vptr) {} // expected-warning{{1 uninitialized field}}
+};
+
+void fIntDynTypedVoidPointerTest1() {
+ int a;
+ IntDynTypedVoidPointerTest1 tmp(&a);
+}
+
+struct RecordDynTypedVoidPointerTest {
+ struct RecordType {
+ int x; // expected-note{{uninitialized field 'this->vptr->x'}}
+ int y; // expected-note{{uninitialized field 'this->vptr->y'}}
+ };
+
+ void *vptr;
+ int dontGetFilteredByNonPedanticMode = 0;
+
+ RecordDynTypedVoidPointerTest(void *vptr) : vptr(vptr) {} // expected-warning{{2 uninitialized fields}}
+};
+
+void fRecordDynTypedVoidPointerTest() {
+ RecordDynTypedVoidPointerTest::RecordType a;
+ RecordDynTypedVoidPointerTest tmp(&a);
+}
+
+struct NestedNonVoidDynTypedVoidPointerTest {
+ struct RecordType {
+ int x; // expected-note{{uninitialized field 'this->vptr->x'}}
+ int y; // expected-note{{uninitialized field 'this->vptr->y'}}
+ void *vptr; // expected-note{{uninitialized pointee 'this->vptr->vptr'}}
+ };
+
+ void *vptr;
+ int dontGetFilteredByNonPedanticMode = 0;
+
+ NestedNonVoidDynTypedVoidPointerTest(void *vptr, void *c) : vptr(vptr) {
+ static_cast<RecordType *>(vptr)->vptr = c; // expected-warning{{3 uninitialized fields}}
+ }
+};
+
+void fNestedNonVoidDynTypedVoidPointerTest() {
+ NestedNonVoidDynTypedVoidPointerTest::RecordType a;
+ char c;
+ NestedNonVoidDynTypedVoidPointerTest tmp(&a, &c);
+}
+
//===----------------------------------------------------------------------===//
// Multipointer tests.
//===----------------------------------------------------------------------===//
Index: test/Analysis/cxx-uninitialized-object-inheritance.cpp
===================================================================
--- test/Analysis/cxx-uninitialized-object-inheritance.cpp
+++ test/Analysis/cxx-uninitialized-object-inheritance.cpp
@@ -773,3 +773,26 @@
void fVirtualDiamondInheritanceTest3() {
VirtualDiamondInheritanceTest3();
}
+
+//===----------------------------------------------------------------------===//
+// Dynamic type test.
+//===----------------------------------------------------------------------===//
+
+struct DynTBase {};
+struct DynTDerived : DynTBase {
+ // TODO: we'd expect the note: {{uninitialized field 'this->x'}}
+ int x; // no-note
+};
+
+struct DynamicTypeTest {
+ DynTBase *bptr;
+ int i = 0;
+
+ // TODO: we'd expect the warning: {{1 uninitialized field}}
+ DynamicTypeTest(DynTBase *bptr) : bptr(bptr) {} // no-warning
+};
+
+void f() {
+ DynTDerived d;
+ DynamicTypeTest t(&d);
+};
Index: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
@@ -30,7 +30,7 @@
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
-#include <algorithm>
+#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h"
using namespace clang;
using namespace clang::ento;
@@ -214,10 +214,10 @@
/// constructor.
static bool isCalledByConstructor(const CheckerContext &Context);
-/// Returns whether FD can be (transitively) dereferenced to a void pointer type
+/// Returns whether T can be (transitively) dereferenced to a void pointer type
/// (void*, void**, ...). The type of the region behind a void pointer isn't
/// known, and thus FD can not be analyzed.
-static bool isVoidPointer(const FieldDecl *FD);
+static bool isVoidPointer(QualType T);
/// Returns true if T is a primitive type. We defined this type so that for
/// objects that we'd only like analyze as much as checking whether their
@@ -459,75 +459,99 @@
SVal V = State->getSVal(FR);
- if (V.isUnknown() || V.isZeroConstant()) {
+ if (V.isUnknown() || V.getAs<loc::ConcreteInt>()) {
IsAnyFieldInitialized = true;
return false;
}
if (V.isUndef()) {
return addFieldToUninits({LocalChain, FR});
}
- const FieldDecl *FD = FR->getDecl();
+ assert(V.getAs<loc::MemRegionVal>() &&
+ "At this point V must be loc::MemRegionVal!");
+ auto L = V.castAs<loc::MemRegionVal>();
- // TODO: The dynamic type of a void pointer may be retrieved with
- // `getDynamicTypeInfo`.
- if (isVoidPointer(FD)) {
+ // We can't reason about symbolic regions, assume its initialized.
+ // Note that this also avoids a potential infinite recursion, because
+ // constructors for list-like classes are checked without being called, and
+ // the Static Analyzer will construct a symbolic region for Node *next; or
+ // similar code snippets.
+ if (L.getRegion()->getSymbolicBase()) {
IsAnyFieldInitialized = true;
return false;
}
- assert(V.getAs<Loc>() && "V should be Loc at this point!");
+ DynamicTypeInfo DynTInfo = getDynamicTypeInfo(State, L.getRegion());
+ if (!DynTInfo.isValid()) {
+ IsAnyFieldInitialized = true;
+ return false;
+ }
- // At this point the pointer itself is initialized and points to a valid
- // location, we'll now check the pointee.
- SVal DerefdV = State->getSVal(V.castAs<Loc>());
+ QualType DynT = DynTInfo.getType();
- // TODO: Dereferencing should be done according to the dynamic type.
- while (Optional<Loc> L = DerefdV.getAs<Loc>()) {
- DerefdV = State->getSVal(*L);
+ if (isVoidPointer(DynT)) {
+ IsAnyFieldInitialized = true;
+ return false;
}
- // If V is a pointer pointing to a record type.
- if (Optional<nonloc::LazyCompoundVal> RecordV =
- DerefdV.getAs<nonloc::LazyCompoundVal>()) {
+ // At this point the pointer itself is initialized and points to a valid
+ // location, we'll now check the pointee.
+ SVal DerefdV = State->getSVal(V.castAs<Loc>(), DynT);
- const TypedValueRegion *R = RecordV->getRegion();
+ // If DerefdV is still a pointer value, we'll dereference it again (e.g.:
+ // int** -> int*).
+ while (auto Tmp = DerefdV.getAs<loc::MemRegionVal>()) {
+ if (Tmp->getRegion()->getSymbolicBase()) {
+ IsAnyFieldInitialized = true;
+ return false;
+ }
+
+ DynTInfo = getDynamicTypeInfo(State, Tmp->getRegion());
+ if (!DynTInfo.isValid()) {
+ IsAnyFieldInitialized = true;
+ return false;
+ }
- // We can't reason about symbolic regions, assume its initialized.
- // Note that this also avoids a potential infinite recursion, because
- // constructors for list-like classes are checked without being called, and
- // the Static Analyzer will construct a symbolic region for Node *next; or
- // similar code snippets.
- if (R->getSymbolicBase()) {
+ DynT = DynTInfo.getType();
+ if (isVoidPointer(DynT)) {
IsAnyFieldInitialized = true;
return false;
}
- const QualType T = R->getValueType();
+ DerefdV = State->getSVal(*Tmp, DynT);
+ }
- if (T->isStructureOrClassType())
+ // If FR is a pointer pointing to a non-primitive type.
+ if (Optional<nonloc::LazyCompoundVal> RecordV =
+ DerefdV.getAs<nonloc::LazyCompoundVal>()) {
+
+ const TypedValueRegion *R = RecordV->getRegion();
+
+ if (DynT->getPointeeType()->isStructureOrClassType())
return isNonUnionUninit(R, {LocalChain, FR});
- if (T->isUnionType()) {
+ if (DynT->getPointeeType()->isUnionType()) {
if (isUnionUninit(R)) {
return addFieldToUninits({LocalChain, FR, /*IsDereferenced*/ true});
} else {
IsAnyFieldInitialized = true;
return false;
}
}
- if (T->isArrayType()) {
+ if (DynT->getPointeeType()->isArrayType()) {
IsAnyFieldInitialized = true;
return false;
}
llvm_unreachable("All cases are handled!");
}
- // TODO: If possible, it should be asserted that the DerefdV at this point is
- // primitive.
+ assert((isPrimitiveType(DynT->getPointeeType()) || DynT->isPointerType() ||
+ DynT->isReferenceType()) &&
+ "At this point FR must either have a primitive dynamic type, or it "
+ "must be a null, undefined, unknown or concrete pointer!");
if (isPrimitiveUninit(DerefdV))
return addFieldToUninits({LocalChain, FR, /*IsDereferenced*/ true});
@@ -571,6 +595,25 @@
return (*Chain.begin())->getDecl();
}
+// TODO: This function constructs an incorrect string if a void pointer is a
+// part of the chain:
+//
+// struct B { int x; }
+//
+// struct A {
+// void *vptr;
+// A(void* vptr) : vptr(vptr) {}
+// };
+//
+// void f() {
+// B b;
+// A a(&b);
+// }
+//
+// The note message will be "uninitialized field 'this->vptr->x'", even though
+// void pointers can't be dereferenced. This should be changed to "uninitialized
+// field 'static_cast<B*>(this->vptr)->x'".
+//
// TODO: This function constructs an incorrect fieldchain string in the
// following case:
//
@@ -611,9 +654,7 @@
// Utility functions.
//===----------------------------------------------------------------------===//
-static bool isVoidPointer(const FieldDecl *FD) {
- QualType T = FD->getType();
-
+static bool isVoidPointer(QualType T) {
while (!T.isNull()) {
if (T->isVoidPointerType())
return true;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
