From 388574a33bb45e36292c8884b8985841f0f2c8fd Mon Sep 17 00:00:00 2001 From: Kristof Umann Date: Thu, 11 Oct 2018 11:58:53 +0000 Subject: [PATCH] [analyzer][UninitializedObjectChecker] Reports Loc fields pointing to themselves I've added a new functionality, the checker is now able to detect and report fields pointing to themselves. I figured this would fit well into the checker as there's no reason for a pointer to point to itself instead of being nullptr. Differential Revision: https://reviews.llvm.org/D51305 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@344242 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../UninitializedPointee.cpp | 68 +++++++++++++------ .../cxx-uninitialized-object-ptr-ref.cpp | 17 +++-- 2 files changed, 60 insertions(+), 25 deletions(-) diff --git a/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp b/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp index 623ba6b3ff..c99dac5adf 100644 --- a/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp +++ b/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp @@ -89,15 +89,39 @@ public: } }; +/// Represents a Loc field that points to itself. +class CyclicLocField final : public FieldNode { + +public: + CyclicLocField(const FieldRegion *FR) : FieldNode(FR) {} + + virtual void printNoteMsg(llvm::raw_ostream &Out) const override { + Out << "object references itself "; + } + + virtual void printPrefix(llvm::raw_ostream &Out) const override {} + + virtual void printNode(llvm::raw_ostream &Out) const override { + Out << getVariableName(getDecl()); + } + + virtual void printSeparator(llvm::raw_ostream &Out) const override { + llvm_unreachable("CyclicLocField objects must be the last node of the " + "fieldchain!"); + } +}; + } // end of anonymous namespace // Utility function declarations. -/// Returns whether \p T can be (transitively) dereferenced to a void pointer -/// type (void*, void**, ...). -static bool isVoidPointer(QualType T); - -using DereferenceInfo = std::pair; +struct DereferenceInfo { + const TypedValueRegion *R; + const bool NeedsCastBack; + const bool IsCyclic; + DereferenceInfo(const TypedValueRegion *R, bool NCB, bool IC) + : R(R), NeedsCastBack(NCB), IsCyclic(IC) {} +}; /// 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 @@ -105,6 +129,10 @@ using DereferenceInfo = std::pair; static llvm::Optional dereference(ProgramStateRef State, const FieldRegion *FR); +/// Returns whether \p T can be (transitively) dereferenced to a void pointer +/// type (void*, void**, ...). +static bool isVoidPointer(QualType T); + //===----------------------------------------------------------------------===// // Methods for FindUninitializedFields. //===----------------------------------------------------------------------===// @@ -141,8 +169,11 @@ bool FindUninitializedFields::isDereferencableUninit( return false; } - const TypedValueRegion *R = DerefInfo->first; - const bool NeedsCastBack = DerefInfo->second; + if (DerefInfo->IsCyclic) + return addFieldToUninits(LocalChain.add(CyclicLocField(FR))); + + const TypedValueRegion *R = DerefInfo->R; + const bool NeedsCastBack = DerefInfo->NeedsCastBack; QualType DynT = R->getLocationType(); QualType PointeeT = DynT->getPointeeType(); @@ -189,15 +220,6 @@ bool FindUninitializedFields::isDereferencableUninit( // Utility functions. //===----------------------------------------------------------------------===// -static bool isVoidPointer(QualType T) { - while (!T.isNull()) { - if (T->isVoidPointerType()) - return true; - T = T->getPointeeType(); - } - return false; -} - static llvm::Optional dereference(ProgramStateRef State, const FieldRegion *FR) { @@ -229,9 +251,8 @@ static llvm::Optional dereference(ProgramStateRef State, return None; // We found a cyclic pointer, like int *ptr = (int *)&ptr. - // TODO: Should we report these fields too? if (!VisitedRegions.insert(R).second) - return None; + return DereferenceInfo{R, NeedsCastBack, /*IsCyclic*/ true}; DynT = R->getLocationType(); // In order to ensure that this loop terminates, we're also checking the @@ -248,5 +269,14 @@ static llvm::Optional dereference(ProgramStateRef State, R = R->getSuperRegion()->getAs(); } - return std::make_pair(R, NeedsCastBack); + return DereferenceInfo{R, NeedsCastBack, /*IsCyclic*/ false}; +} + +static bool isVoidPointer(QualType T) { + while (!T.isNull()) { + if (T->isVoidPointerType()) + return true; + T = T->getPointeeType(); + } + return false; } diff --git a/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp b/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp index 4f95f09cc0..c222f3e6a8 100644 --- a/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp +++ b/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp @@ -257,8 +257,10 @@ void fCharPointerTest() { } struct CyclicPointerTest1 { - int *ptr; - CyclicPointerTest1() : ptr(reinterpret_cast(&ptr)) {} + int *ptr; // expected-note{{object references itself 'this->ptr'}} + int dontGetFilteredByNonPedanticMode = 0; + + CyclicPointerTest1() : ptr(reinterpret_cast(&ptr)) {} // expected-warning{{1 uninitialized field}} }; void fCyclicPointerTest1() { @@ -266,8 +268,10 @@ void fCyclicPointerTest1() { } struct CyclicPointerTest2 { - int **pptr; // no-crash - CyclicPointerTest2() : pptr(reinterpret_cast(&pptr)) {} + int **pptr; // expected-note{{object references itself 'this->pptr'}} + int dontGetFilteredByNonPedanticMode = 0; + + CyclicPointerTest2() : pptr(reinterpret_cast(&pptr)) {} // expected-warning{{1 uninitialized field}} }; void fCyclicPointerTest2() { @@ -353,9 +357,10 @@ void fVoidPointerLRefTest() { } struct CyclicVoidPointerTest { - void *vptr; // no-crash + void *vptr; // expected-note{{object references itself 'this->vptr'}} + int dontGetFilteredByNonPedanticMode = 0; - CyclicVoidPointerTest() : vptr(&vptr) {} + CyclicVoidPointerTest() : vptr(&vptr) {} // expected-warning{{1 uninitialized field}} }; void fCyclicVoidPointerTest() { -- 2.40.0