Skip to content

Commit

Permalink
[analyzer][UninitializedObjectChecker] Fixed dereferencing
Browse files Browse the repository at this point in the history 
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

llvm-svn: 342213
  • Loading branch information
@Szelethus
Szelethus committed Sep 14, 2018
1 parent 89fc264 commit f0dd101
Show file tree
Hide file tree
Showing 6 changed files with 207 additions and 91 deletions.
10 changes: 8 additions & 2 deletions clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
View file
Expand Up @@ -87,7 +87,7 @@ class FieldNode {

/// 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
Expand Down Expand Up @@ -255,7 +255,13 @@ class FindUninitializedFields {
/// 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.
Expand Down
25 changes: 18 additions & 7 deletions clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
View file
Expand Up @@ -252,9 +252,12 @@ bool FindUninitializedFields::isNonUnionUninit(const TypedValueRegion *R,
!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;

Expand Down Expand Up @@ -292,8 +295,7 @@ bool FindUninitializedFields::isNonUnionUninit(const TypedValueRegion *R,
continue;
}

if (T->isAnyPointerType() || T->isReferenceType() ||
T->isBlockPointerType()) {
if (isDereferencableType(T)) {
if (isPointerOrReferenceUninit(FR, LocalChain))
ContainsUninitField = true;
continue;
Expand Down Expand Up @@ -487,7 +489,7 @@ static bool willObjectBeAnalyzedLater(const CXXConstructorDecl *Ctor,
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.
Expand All @@ -496,7 +498,16 @@ StringRef clang::ento::getVariableName(const FieldDecl *Field) {
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();
Expand Down
128 changes: 60 additions & 68 deletions clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
View file
Expand Up @@ -95,11 +95,13 @@ class NeedsCastLocField final : public FieldNode {
/// 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.
Expand All @@ -110,10 +112,8 @@ dereference(ProgramStateRef State, const FieldRegion *FR);
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);

Expand All @@ -134,54 +134,47 @@ bool FindUninitializedFields::isPointerOrReferenceUninit(

// 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)));
Expand All @@ -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!");

// 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;
}
assert(V.getAsRegion() && "V must have an underlying region!");

DynTInfo = getDynamicTypeInfo(State, Tmp->getRegion());
if (!DynTInfo.isValid()) {
return None;
}
// The region we'd like to acquire.
const auto *R = V.getAsRegion()->getAs<TypedValueRegion>();
if (!R)
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);
}
92 changes: 87 additions & 5 deletions clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
View file
Expand Up @@ -45,6 +45,50 @@ void fNullPtrTest() {
NullPtrTest();
}

//===----------------------------------------------------------------------===//
// 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.
//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -470,6 +510,39 @@ void fMultiPointerTest3() {
MultiPointerTest3(mptr, int()); // '**mptr' uninitialized
}

//===----------------------------------------------------------------------===//
// 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.
//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -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.
//===----------------------------------------------------------------------===//
Expand Down

0 comments on commit f0dd101

Please sign in to comment.