r342213 - [analyzer][UninitializedObjectChecker] Fixed dereferencing
Kristof Umann via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 14 01:58:21 PDT 2018
Author: szelethus
Date: Fri Sep 14 01:58:21 2018
New Revision: 342213
URL: http://llvm.org/viewvc/llvm-project?rev=342213&view=rev
Log:
[analyzer][UninitializedObjectChecker] Fixed dereferencing
iThis patch aims to fix derefencing, which has been debated for months now.
Instead of working with SVals, the function now relies on TypedValueRegion.
Differential Revision: https://reviews.llvm.org/D51057
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp
cfe/trunk/test/Analysis/objcpp-uninitialized-object.mm
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h?rev=342213&r1=342212&r2=342213&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h Fri Sep 14 01:58:21 2018
@@ -87,7 +87,7 @@ public:
/// Returns with Field's name. This is a helper function to get the correct name
/// even if Field is a captured lambda variable.
-StringRef getVariableName(const FieldDecl *Field);
+std::string getVariableName(const FieldDecl *Field);
/// Represents a field chain. A field chain is a vector of fields where the
/// first element of the chain is the object under checking (not stored), and
@@ -255,7 +255,13 @@ private:
/// ease. This also helps ensuring that every special field type is handled
/// correctly.
inline bool isPrimitiveType(const QualType &T) {
- return T->isBuiltinType() || T->isEnumeralType() || T->isMemberPointerType();
+ return T->isBuiltinType() || T->isEnumeralType() ||
+ T->isMemberPointerType() || T->isBlockPointerType() ||
+ T->isFunctionType();
+}
+
+inline bool isDereferencableType(const QualType &T) {
+ return T->isAnyPointerType() || T->isReferenceType();
}
// Template method definitions.
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp?rev=342213&r1=342212&r2=342213&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp Fri Sep 14 01:58:21 2018
@@ -252,9 +252,12 @@ bool FindUninitializedFields::isNonUnion
!R->getValueType()->isUnionType() &&
"This method only checks non-union record objects!");
- const RecordDecl *RD =
- R->getValueType()->getAs<RecordType>()->getDecl()->getDefinition();
- assert(RD && "Referred record has no definition");
+ const RecordDecl *RD = R->getValueType()->getAsRecordDecl()->getDefinition();
+
+ if (!RD) {
+ IsAnyFieldInitialized = true;
+ return true;
+ }
bool ContainsUninitField = false;
@@ -292,8 +295,7 @@ bool FindUninitializedFields::isNonUnion
continue;
}
- if (T->isAnyPointerType() || T->isReferenceType() ||
- T->isBlockPointerType()) {
+ if (isDereferencableType(T)) {
if (isPointerOrReferenceUninit(FR, LocalChain))
ContainsUninitField = true;
continue;
@@ -487,7 +489,7 @@ static bool willObjectBeAnalyzedLater(co
return false;
}
-StringRef clang::ento::getVariableName(const FieldDecl *Field) {
+std::string clang::ento::getVariableName(const FieldDecl *Field) {
// If Field is a captured lambda variable, Field->getName() will return with
// an empty string. We can however acquire it's name from the lambda's
// captures.
@@ -496,7 +498,16 @@ StringRef clang::ento::getVariableName(c
if (CXXParent && CXXParent->isLambda()) {
assert(CXXParent->captures_begin());
auto It = CXXParent->captures_begin() + Field->getFieldIndex();
- return It->getCapturedVar()->getName();
+
+ if (It->capturesVariable())
+ return llvm::Twine("/*captured variable*/" +
+ It->getCapturedVar()->getName())
+ .str();
+
+ if (It->capturesThis())
+ return "/*'this' capture*/";
+
+ llvm_unreachable("No other capture type is expected!");
}
return Field->getName();
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp?rev=342213&r1=342212&r2=342213&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp Fri Sep 14 01:58:21 2018
@@ -95,11 +95,13 @@ public:
/// known, and thus FD can not be analyzed.
static bool isVoidPointer(QualType T);
-/// Dereferences \p V and returns the value and dynamic type of the pointee, as
-/// well as whether \p FR needs to be casted back to that type. If for whatever
-/// reason dereferencing fails, returns with None.
-static llvm::Optional<std::tuple<SVal, QualType, bool>>
-dereference(ProgramStateRef State, const FieldRegion *FR);
+using DereferenceInfo = std::pair<const TypedValueRegion *, bool>;
+
+/// Dereferences \p FR and returns with the pointee's region, and whether it
+/// needs to be casted back to it's location type. If for whatever reason
+/// dereferencing fails, returns with None.
+static llvm::Optional<DereferenceInfo> dereference(ProgramStateRef State,
+ const FieldRegion *FR);
//===----------------------------------------------------------------------===//
// Methods for FindUninitializedFields.
@@ -110,10 +112,8 @@ dereference(ProgramStateRef State, const
bool FindUninitializedFields::isPointerOrReferenceUninit(
const FieldRegion *FR, FieldChainInfo LocalChain) {
- assert((FR->getDecl()->getType()->isAnyPointerType() ||
- FR->getDecl()->getType()->isReferenceType() ||
- FR->getDecl()->getType()->isBlockPointerType()) &&
- "This method only checks pointer/reference objects!");
+ assert(isDereferencableType(FR->getDecl()->getType()) &&
+ "This method only checks dereferencable objects!");
SVal V = State->getSVal(FR);
@@ -134,54 +134,47 @@ bool FindUninitializedFields::isPointerO
// At this point the pointer itself is initialized and points to a valid
// location, we'll now check the pointee.
- llvm::Optional<std::tuple<SVal, QualType, bool>> DerefInfo =
- dereference(State, FR);
+ llvm::Optional<DereferenceInfo> DerefInfo = dereference(State, FR);
if (!DerefInfo) {
IsAnyFieldInitialized = true;
return false;
}
- V = std::get<0>(*DerefInfo);
- QualType DynT = std::get<1>(*DerefInfo);
- bool NeedsCastBack = std::get<2>(*DerefInfo);
+ const TypedValueRegion *R = DerefInfo->first;
+ const bool NeedsCastBack = DerefInfo->second;
- // If FR is a pointer pointing to a non-primitive type.
- if (Optional<nonloc::LazyCompoundVal> RecordV =
- V.getAs<nonloc::LazyCompoundVal>()) {
+ QualType DynT = R->getLocationType();
+ QualType PointeeT = DynT->getPointeeType();
- const TypedValueRegion *R = RecordV->getRegion();
+ if (PointeeT->isStructureOrClassType()) {
+ if (NeedsCastBack)
+ return isNonUnionUninit(R, LocalChain.add(NeedsCastLocField(FR, DynT)));
+ return isNonUnionUninit(R, LocalChain.add(LocField(FR)));
+ }
- if (DynT->getPointeeType()->isStructureOrClassType()) {
+ if (PointeeT->isUnionType()) {
+ if (isUnionUninit(R)) {
if (NeedsCastBack)
- return isNonUnionUninit(R, LocalChain.add(NeedsCastLocField(FR, DynT)));
- return isNonUnionUninit(R, LocalChain.add(LocField(FR)));
- }
-
- if (DynT->getPointeeType()->isUnionType()) {
- if (isUnionUninit(R)) {
- if (NeedsCastBack)
- return addFieldToUninits(LocalChain.add(NeedsCastLocField(FR, DynT)));
- return addFieldToUninits(LocalChain.add(LocField(FR)));
- } else {
- IsAnyFieldInitialized = true;
- return false;
- }
- }
-
- if (DynT->getPointeeType()->isArrayType()) {
+ return addFieldToUninits(LocalChain.add(NeedsCastLocField(FR, DynT)));
+ return addFieldToUninits(LocalChain.add(LocField(FR)));
+ } else {
IsAnyFieldInitialized = true;
return false;
}
+ }
- llvm_unreachable("All cases are handled!");
+ if (PointeeT->isArrayType()) {
+ IsAnyFieldInitialized = true;
+ return false;
}
- assert((isPrimitiveType(DynT->getPointeeType()) || DynT->isAnyPointerType() ||
- DynT->isReferenceType()) &&
+ assert((isPrimitiveType(PointeeT) || isDereferencableType(PointeeT)) &&
"At this point FR must either have a primitive dynamic type, or it "
"must be a null, undefined, unknown or concrete pointer!");
- if (isPrimitiveUninit(V)) {
+ SVal PointeeV = State->getSVal(R);
+
+ if (isPrimitiveUninit(PointeeV)) {
if (NeedsCastBack)
return addFieldToUninits(LocalChain.add(NeedsCastLocField(FR, DynT)));
return addFieldToUninits(LocalChain.add(LocField(FR)));
@@ -204,47 +197,46 @@ static bool isVoidPointer(QualType T) {
return false;
}
-static llvm::Optional<std::tuple<SVal, QualType, bool>>
-dereference(ProgramStateRef State, const FieldRegion *FR) {
+static llvm::Optional<DereferenceInfo> dereference(ProgramStateRef State,
+ const FieldRegion *FR) {
- DynamicTypeInfo DynTInfo;
- QualType DynT;
+ llvm::SmallSet<const TypedValueRegion *, 5> VisitedRegions;
// If the static type of the field is a void pointer, we need to cast it back
// to the dynamic type before dereferencing.
bool NeedsCastBack = isVoidPointer(FR->getDecl()->getType());
SVal V = State->getSVal(FR);
- assert(V.getAs<loc::MemRegionVal>() && "V must be loc::MemRegionVal!");
+ assert(V.getAsRegion() && "V must have an underlying region!");
- // If V is multiple pointer value, we'll dereference it again (e.g.: int** ->
- // int*).
- // TODO: Dereference according to the dynamic type to avoid infinite loop for
- // these kind of fields:
- // int **ptr = reinterpret_cast<int **>(&ptr);
- while (auto Tmp = V.getAs<loc::MemRegionVal>()) {
- // 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 (Tmp->getRegion()->getSymbolicBase()) {
- return None;
- }
+ // The region we'd like to acquire.
+ const auto *R = V.getAsRegion()->getAs<TypedValueRegion>();
+ if (!R)
+ return None;
- DynTInfo = getDynamicTypeInfo(State, Tmp->getRegion());
- if (!DynTInfo.isValid()) {
- return None;
- }
+ VisitedRegions.insert(R);
- DynT = DynTInfo.getType();
+ // We acquire the dynamic type of R,
+ QualType DynT = R->getLocationType();
- if (isVoidPointer(DynT)) {
+ while (const MemRegion *Tmp = State->getSVal(R, DynT).getAsRegion()) {
+
+ R = Tmp->getAs<TypedValueRegion>();
+
+ if (!R)
+ return None;
+
+ // We found a cyclic pointer, like int *ptr = (int *)&ptr.
+ // TODO: Report these fields too.
+ if (!VisitedRegions.insert(R).second)
return None;
- }
- V = State->getSVal(*Tmp, DynT);
+ DynT = R->getLocationType();
+ // In order to ensure that this loop terminates, we're also checking the
+ // dynamic type of R, since type hierarchy is finite.
+ if (isDereferencableType(DynT->getPointeeType()))
+ break;
}
- return std::make_tuple(V, DynT, NeedsCastBack);
+ return std::make_pair(R, NeedsCastBack);
}
Modified: cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp?rev=342213&r1=342212&r2=342213&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp (original)
+++ cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp Fri Sep 14 01:58:21 2018
@@ -46,6 +46,50 @@ void fNullPtrTest() {
}
//===----------------------------------------------------------------------===//
+// Alloca tests.
+//===----------------------------------------------------------------------===//
+
+struct UntypedAllocaTest {
+ void *allocaPtr;
+ int dontGetFilteredByNonPedanticMode = 0;
+
+ UntypedAllocaTest() : allocaPtr(__builtin_alloca(sizeof(int))) {
+ // All good!
+ }
+};
+
+void fUntypedAllocaTest() {
+ UntypedAllocaTest();
+}
+
+struct TypedAllocaTest1 {
+ int *allocaPtr; // expected-note{{uninitialized pointee 'this->allocaPtr'}}
+ int dontGetFilteredByNonPedanticMode = 0;
+
+ TypedAllocaTest1() // expected-warning{{1 uninitialized field}}
+ : allocaPtr(static_cast<int *>(__builtin_alloca(sizeof(int)))) {}
+};
+
+void fTypedAllocaTest1() {
+ TypedAllocaTest1();
+}
+
+struct TypedAllocaTest2 {
+ int *allocaPtr;
+ int dontGetFilteredByNonPedanticMode = 0;
+
+ TypedAllocaTest2()
+ : allocaPtr(static_cast<int *>(__builtin_alloca(sizeof(int)))) {
+ *allocaPtr = 55555;
+ // All good!
+ }
+};
+
+void fTypedAllocaTest2() {
+ TypedAllocaTest2();
+}
+
+//===----------------------------------------------------------------------===//
// Heap pointer tests.
//===----------------------------------------------------------------------===//
@@ -203,18 +247,14 @@ void fCyclicPointerTest1() {
CyclicPointerTest1();
}
-// TODO: Currently, the checker ends up in an infinite loop for the following
-// test case.
-/*
struct CyclicPointerTest2 {
- int **pptr;
+ int **pptr; // no-crash
CyclicPointerTest2() : pptr(reinterpret_cast<int **>(&pptr)) {}
};
void fCyclicPointerTest2() {
CyclicPointerTest2();
}
-*/
//===----------------------------------------------------------------------===//
// Void pointer tests.
@@ -471,6 +511,39 @@ void fMultiPointerTest3() {
}
//===----------------------------------------------------------------------===//
+// Incomplete pointee tests.
+//===----------------------------------------------------------------------===//
+
+class IncompleteType;
+
+struct IncompletePointeeTypeTest {
+ IncompleteType *pImpl; //no-crash
+ int dontGetFilteredByNonPedanticMode = 0;
+
+ IncompletePointeeTypeTest(IncompleteType *A) : pImpl(A) {}
+};
+
+void fIncompletePointeeTypeTest(void *ptr) {
+ IncompletePointeeTypeTest(reinterpret_cast<IncompleteType *>(ptr));
+}
+
+//===----------------------------------------------------------------------===//
+// Function pointer tests.
+//===----------------------------------------------------------------------===//
+
+struct FunctionPointerWithDifferentDynTypeTest {
+ using Func1 = void *(*)();
+ using Func2 = int *(*)();
+
+ Func1 f; // no-crash
+ FunctionPointerWithDifferentDynTypeTest(Func2 f) : f((Func1)f) {}
+};
+
+// Note that there isn't a function calling the constructor of
+// FunctionPointerWithDifferentDynTypeTest, because a crash could only be
+// reproduced without it.
+
+//===----------------------------------------------------------------------===//
// Member pointer tests.
//===----------------------------------------------------------------------===//
@@ -645,6 +718,15 @@ void fCyclicList() {
CyclicList(&n1, int());
}
+struct RingListTest {
+ RingListTest *next; // no-crash
+ RingListTest() : next(this) {}
+};
+
+void fRingListTest() {
+ RingListTest();
+}
+
//===----------------------------------------------------------------------===//
// Tests for classes containing references.
//===----------------------------------------------------------------------===//
Modified: cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp?rev=342213&r1=342212&r2=342213&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp (original)
+++ cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp Fri Sep 14 01:58:21 2018
@@ -1,11 +1,11 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject \
// RUN: -analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true -DPEDANTIC \
// RUN: -analyzer-config alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=true \
-// RUN: -std=c++11 -verify %s
+// RUN: -std=c++14 -verify %s
// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject \
// RUN: -analyzer-config alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=true \
-// RUN: -std=c++11 -verify %s
+// RUN: -std=c++14 -verify %s
//===----------------------------------------------------------------------===//
// Default constructor test.
@@ -781,7 +781,7 @@ struct LambdaTest2 {
void fLambdaTest2() {
int b;
- auto equals = [&b](int a) { return a == b; }; // expected-note{{uninitialized pointee 'this->functor.b'}}
+ auto equals = [&b](int a) { return a == b; }; // expected-note{{uninitialized pointee 'this->functor./*captured variable*/b'}}
LambdaTest2<decltype(equals)>(equals, int());
}
#else
@@ -803,8 +803,8 @@ void fLambdaTest2() {
namespace LT3Detail {
struct RecordType {
- int x; // expected-note{{uninitialized field 'this->functor.rec1.x'}}
- int y; // expected-note{{uninitialized field 'this->functor.rec1.y'}}
+ int x; // expected-note{{uninitialized field 'this->functor./*captured variable*/rec1.x'}}
+ int y; // expected-note{{uninitialized field 'this->functor./*captured variable*/rec1.y'}}
};
} // namespace LT3Detail
@@ -857,8 +857,8 @@ struct MultipleLambdaCapturesTest1 {
void fMultipleLambdaCapturesTest1() {
int b1, b2 = 3, b3;
- auto equals = [&b1, &b2, &b3](int a) { return a == b1 == b2 == b3; }; // expected-note{{uninitialized pointee 'this->functor.b1'}}
- // expected-note at -1{{uninitialized pointee 'this->functor.b3'}}
+ auto equals = [&b1, &b2, &b3](int a) { return a == b1 == b2 == b3; }; // expected-note{{uninitialized pointee 'this->functor./*captured variable*/b1'}}
+ // expected-note at -1{{uninitialized pointee 'this->functor./*captured variable*/b3'}}
MultipleLambdaCapturesTest1<decltype(equals)>(equals, int());
}
@@ -872,10 +872,35 @@ struct MultipleLambdaCapturesTest2 {
void fMultipleLambdaCapturesTest2() {
int b1, b2 = 3, b3;
- auto equals = [b1, &b2, &b3](int a) { return a == b1 == b2 == b3; }; // expected-note{{uninitialized pointee 'this->functor.b3'}}
+ auto equals = [b1, &b2, &b3](int a) { return a == b1 == b2 == b3; }; // expected-note{{uninitialized pointee 'this->functor./*captured variable*/b3'}}
MultipleLambdaCapturesTest2<decltype(equals)>(equals, int());
}
+struct LambdaWrapper {
+ void *func; // no-crash
+ int dontGetFilteredByNonPedanticMode = 0;
+
+ LambdaWrapper(void *ptr) : func(ptr) {} // expected-warning{{1 uninitialized field}}
+};
+
+struct ThisCapturingLambdaFactory {
+ int a; // expected-note{{uninitialized field 'static_cast<decltype(a.ret()) *>(this->func)->/*'this' capture*/->a'}}
+
+ auto ret() {
+ return [this] { (void)this; };
+ }
+};
+
+void fLambdaFieldWithInvalidThisCapture() {
+ void *ptr;
+ {
+ ThisCapturingLambdaFactory a;
+ decltype(a.ret()) lambda = a.ret();
+ ptr = λ
+ }
+ LambdaWrapper t(ptr);
+}
+
//===----------------------------------------------------------------------===//
// System header tests.
//===----------------------------------------------------------------------===//
Modified: cfe/trunk/test/Analysis/objcpp-uninitialized-object.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/objcpp-uninitialized-object.mm?rev=342213&r1=342212&r2=342213&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/objcpp-uninitialized-object.mm (original)
+++ cfe/trunk/test/Analysis/objcpp-uninitialized-object.mm Fri Sep 14 01:58:21 2018
@@ -4,7 +4,7 @@ typedef void (^myBlock) ();
struct StructWithBlock {
int a;
- myBlock z; // expected-note{{uninitialized pointer 'this->z'}}
+ myBlock z; // expected-note{{uninitialized field 'this->z'}}
StructWithBlock() : a(0), z(^{}) {}
More information about the cfe-commits
mailing list