kromanenkov updated this revision to Diff 75020. kromanenkov added a comment.
Null pointer-to-member dereference is now modeled as UndefinedVal. Fixing this also releases us from loading from nonloc SVal. I delete an assertion suppression in ExprEngine::performTrivialCopy because I can't reproduce its violation. https://reviews.llvm.org/D25475 Files: include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h lib/StaticAnalyzer/Core/ExprEngineC.cpp lib/StaticAnalyzer/Core/SValBuilder.cpp lib/StaticAnalyzer/Core/SVals.cpp lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp test/Analysis/pointer-to-member.cpp
Index: test/Analysis/pointer-to-member.cpp =================================================================== --- test/Analysis/pointer-to-member.cpp +++ test/Analysis/pointer-to-member.cpp @@ -35,8 +35,7 @@ clang_analyzer_eval(&A::getPtr == &A::getPtr); // expected-warning{{TRUE}} clang_analyzer_eval(&A::getPtr == 0); // expected-warning{{FALSE}} - // FIXME: Should be TRUE. - clang_analyzer_eval(&A::m_ptr == &A::m_ptr); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(&A::m_ptr == &A::m_ptr); // expected-warning{{TRUE}} } namespace PR15742 { @@ -62,21 +61,70 @@ } } -// --------------- -// FALSE NEGATIVES -// --------------- - bool testDereferencing() { A obj; obj.m_ptr = 0; A::MemberPointer member = &A::m_ptr; - // FIXME: Should be TRUE. - clang_analyzer_eval(obj.*member == 0); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(obj.*member == 0); // expected-warning{{TRUE}} member = 0; - // FIXME: Should emit a null dereference. - return obj.*member; // no-warning + return obj.*member; // expected-warning{{}} } + +namespace testPointerToMemberFunction { + struct A { + virtual int foo() { return 1; } + int bar() { return 2; } + }; + + struct B : public A { + virtual int foo() { return 3; } + }; + + typedef int (A::*AFnPointer)(); + typedef int (B::*BFnPointer)(); + + void testPointerToMemberCasts() { + AFnPointer AFP = &A::bar; + BFnPointer StaticCastedBase2Derived = static_cast<BFnPointer>(&A::bar), + CCastedBase2Derived = (BFnPointer) (&A::bar); + A a; + B b; + + clang_analyzer_eval((a.*AFP)() == 2); // expected-warning{{TRUE}} + clang_analyzer_eval((b.*StaticCastedBase2Derived)() == 2); // expected-warning{{TRUE}} + clang_analyzer_eval(((b.*CCastedBase2Derived)() == 2)); // expected-warning{{TRUE}} + } + + void testPointerToMemberVirtualCall() { + A a; + B b; + A *APtr = &a; + AFnPointer AFP = &A::foo; + + clang_analyzer_eval((APtr->*AFP)() == 1); // expected-warning{{TRUE}} + + APtr = &b; + + clang_analyzer_eval((APtr->*AFP)() == 3); // expected-warning{{TRUE}} + } +} // end of testPointerToMemberFunction namespace + +namespace testPointerToMemberData { + struct A { + int i; + }; + + void testPointerToMemberData() { + int A::*AMdPointer = &A::i; + A a; + + a.i = 42; + a.*AMdPointer += 1; + + clang_analyzer_eval(a.i == 43); // expected-warning{{TRUE}} + } +} // end of testPointerToMemberData namespace Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp =================================================================== --- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -69,6 +69,9 @@ bool isLocType = Loc::isLocType(castTy); + if (val.getAs<nonloc::PointerToMember>()) + return val; + if (Optional<nonloc::LocAsInteger> LI = val.getAs<nonloc::LocAsInteger>()) { if (isLocType) return LI->getLoc(); @@ -335,6 +338,21 @@ switch (lhs.getSubKind()) { default: return makeSymExprValNN(state, op, lhs, rhs, resultTy); + case nonloc::PointerToMemberKind: { + assert(rhs.getSubKind() == nonloc::PointerToMemberKind && + "Both SVals should have pointer-to-member-type"); + const DeclaratorDecl *LDD = + lhs.castAs<nonloc::PointerToMember>().getDecl(), + *RDD = rhs.castAs<nonloc::PointerToMember>().getDecl(); + switch (op) { + case BO_EQ: + return makeTruthVal(LDD == RDD, resultTy); + case BO_NE: + return makeTruthVal(LDD != RDD, resultTy); + default: + return UnknownVal(); + } + } case nonloc::LocAsIntegerKind: { Loc lhsL = lhs.castAs<nonloc::LocAsInteger>().getLoc(); switch (rhs.getSubKind()) { @@ -857,6 +875,17 @@ SVal SimpleSValBuilder::evalBinOpLN(ProgramStateRef state, BinaryOperator::Opcode op, Loc lhs, NonLoc rhs, QualType resultTy) { + if (op >= BO_PtrMemD && op <= BO_PtrMemI) { + if (auto PTMSV = rhs.getAs<nonloc::PointerToMember>()) { + if (PTMSV->isNullMemberPointer()) + return UndefinedVal(); + if (const FieldDecl *FD = PTMSV->getDeclAs<FieldDecl>()) + return state->getLValue(FD, lhs); + } + + return rhs; + } + assert(!BinaryOperator::isComparisonOp(op) && "arguments to comparison ops must be of the same type"); Index: lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp =================================================================== --- lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp +++ lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp @@ -184,6 +184,12 @@ return isFeasible ? state : nullptr; } + case nonloc::PointerToMemberKind: { + bool IsNull = !Cond.castAs<nonloc::PointerToMember>().isNullMemberPointer(); + bool IsFeasible = IsNull ? Assumption : !Assumption; + return IsFeasible ? state : nullptr; + } + case nonloc::LocAsIntegerKind: return assume(state, Cond.castAs<nonloc::LocAsInteger>().getLoc(), Assumption); Index: lib/StaticAnalyzer/Core/SVals.cpp =================================================================== --- lib/StaticAnalyzer/Core/SVals.cpp +++ lib/StaticAnalyzer/Core/SVals.cpp @@ -16,6 +16,7 @@ #include "clang/AST/ExprObjC.h" #include "clang/Basic/IdentifierTable.h" #include "llvm/Support/raw_ostream.h" +#include "clang/AST/DeclCXX.h" using namespace clang; using namespace ento; using llvm::APSInt; @@ -56,6 +57,10 @@ return FD; } + if (auto X = getAs<nonloc::PointerToMember>()) + if (const CXXMethodDecl *MD = X->getDeclAs<CXXMethodDecl>()) + return MD; + return nullptr; } @@ -299,6 +304,15 @@ << '}'; break; } + case nonloc::PointerToMemberKind: { + os << "pointerToMember{"; + const nonloc::PointerToMember &CastRes = + castAs<nonloc::PointerToMember>(); + if (CastRes.getDecl()) + os << CastRes.getDecl()->getQualifiedNameAsString(); + os << '}'; + break; + } default: assert (false && "Pretty-printed not implemented for this NonLoc."); break; Index: lib/StaticAnalyzer/Core/SValBuilder.cpp =================================================================== --- lib/StaticAnalyzer/Core/SValBuilder.cpp +++ lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -214,6 +214,10 @@ return nonloc::SymbolVal(sym); } +DefinedSVal SValBuilder::getMemberPointer(const DeclaratorDecl* DD) { + return nonloc::PointerToMember(DD); +} + DefinedSVal SValBuilder::getFunctionPointer(const FunctionDecl *func) { return loc::MemRegionVal(MemMgr.getFunctionCodeRegion(func)); } @@ -291,6 +295,18 @@ case Stmt::CXXNullPtrLiteralExprClass: return makeNull(); + case Stmt::UnaryOperatorClass: { + const UnaryOperator *UO = dyn_cast<UnaryOperator>(E); + if (const DeclRefExpr *DRE = + dyn_cast<DeclRefExpr>(UO->getSubExpr()->IgnoreParenCasts())) { + if (const DeclaratorDecl *DD = + dyn_cast_or_null<DeclaratorDecl>(DRE->getDecl())) + if (isa<CXXMethodDecl>(DD) || isa<FieldDecl>(DD)) + return getMemberPointer(DD); + } + return None; + } + case Stmt::ImplicitCastExprClass: { const CastExpr *CE = cast<CastExpr>(E); switch (CE->getCastKind()) { Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -12,6 +12,7 @@ //===----------------------------------------------------------------------===// #include "clang/AST/ExprCXX.h" +#include "clang/AST/DeclCXX.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" @@ -310,17 +311,32 @@ continue; } case CK_MemberPointerToBoolean: - // FIXME: For now, member pointers are represented by void *. - // FALLTHROUGH + case CK_PointerToBoolean: { + SVal V = state->getSVal(Ex, LCtx); + auto PTMSV = V.getAs<nonloc::PointerToMember>(); + if (PTMSV) + V = svalBuilder.makeTruthVal(!PTMSV->isNullMemberPointer(), ExTy); + if (V.isUndef() || PTMSV) { + state = state->BindExpr(CastE, LCtx, V); + Bldr.generateNode(CastE, Pred, state); + continue; + } + } case CK_Dependent: case CK_ArrayToPointerDecay: case CK_BitCast: case CK_AddressSpaceConversion: case CK_BooleanToSignedIntegral: case CK_NullToPointer: case CK_IntegralToPointer: - case CK_PointerToIntegral: - case CK_PointerToBoolean: + case CK_PointerToIntegral: { + SVal V = state->getSVal(Ex, LCtx); + if (V.getAs<nonloc::PointerToMember>()) { + state = state->BindExpr(CastE, LCtx, UnknownVal()); + Bldr.generateNode(CastE, Pred, state); + continue; + } + } case CK_IntegralToBoolean: case CK_IntegralToFloating: case CK_FloatingToIntegral: @@ -435,17 +451,37 @@ continue; } case CK_NullToMemberPointer: { - // FIXME: For now, member pointers are represented by void *. - SVal V = svalBuilder.makeNull(); + SVal V = svalBuilder.getMemberPointer(nullptr); state = state->BindExpr(CastE, LCtx, V); Bldr.generateNode(CastE, Pred, state); continue; } + case CK_DerivedToBaseMemberPointer: + case CK_BaseToDerivedMemberPointer: + case CK_ReinterpretMemberPointer: { + const Expr *UOExpr = CastE->getSubExpr()->IgnoreParenCasts(); + assert(isa<UnaryOperator>(UOExpr) && + "UnaryOperator as Cast's child was expected"); + if (const UnaryOperator *UO = cast<UnaryOperator>(UOExpr)) { + const Expr *DREExpr = UO->getSubExpr()->IgnoreParenCasts(); + assert(isa<DeclRefExpr>(DREExpr) && + "DeclRefExpr as UnaryOperator's child was expected"); + if (const DeclRefExpr *DRE = cast<DeclRefExpr>(DREExpr)) { + assert(isa<DeclaratorDecl>(DRE->getDecl()) && + "DeclaratorDecl as DeclRefExpr's Decl is expected"); + if (const DeclaratorDecl *DD = + cast<DeclaratorDecl>(DRE->getDecl())) { + SVal Result = svalBuilder.getMemberPointer(DD); + state = state->BindExpr(CastE, LCtx, Result); + Bldr.generateNode(CastE, Pred, state); + continue; + } + } + } + //if dyn_cast failed just fall through to default behaviour + } // Various C++ casts that are not handled yet. case CK_ToUnion: - case CK_BaseToDerivedMemberPointer: - case CK_DerivedToBaseMemberPointer: - case CK_ReinterpretMemberPointer: case CK_VectorSplat: { // Recover some path-sensitivty by conjuring a new value. QualType resultType = CastE->getType(); @@ -868,7 +904,22 @@ assert(!U->isGLValue()); // FALL-THROUGH. case UO_Deref: - case UO_AddrOf: + case UO_AddrOf: { + // Process pointer-to-member address operation + const Expr *Ex = U->getSubExpr()->IgnoreParens(); + if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Ex)) { + const ValueDecl *VD = DRE->getDecl(); + + if (isa<CXXMethodDecl>(VD) || isa<FieldDecl>(VD)) { + ProgramStateRef State = (*I)->getState(); + const LocationContext *LCtx = (*I)->getLocationContext(); + SVal SV = svalBuilder.getMemberPointer(cast<DeclaratorDecl>(VD)); + Bldr.generateNode(U, *I, State->BindExpr(U, LCtx, SV)); + break; + } + } + //Fall through in case of not pointer-to-member address operation + } case UO_Extension: { // FIXME: We can probably just have some magic in Environment::getSVal() // that propagates values, instead of creating a new node here. Index: include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h @@ -459,6 +459,35 @@ } }; +class PointerToMember : public NonLoc { +public: + explicit PointerToMember(const DeclaratorDecl* D) + : NonLoc(PointerToMemberKind, D) {} + + const DeclaratorDecl *getDecl() const { + return static_cast<const DeclaratorDecl*>(Data); + } + + template<typename AdjustedDecl> + const AdjustedDecl* getDeclAs() const { + return dyn_cast_or_null<AdjustedDecl>(getDecl()); + } + + bool isNullMemberPointer() {return Data == nullptr;} + +private: + friend class SVal; + PointerToMember() {} + static bool isKind(const SVal& V) { + return V.getBaseKind() == NonLocKind && + V.getSubKind() == PointerToMemberKind; + } + + static bool isKind(const NonLoc& V) { + return V.getSubKind() == PointerToMemberKind; + } +}; + } // end namespace ento::nonloc //==------------------------------------------------------------------------==// Index: include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def +++ include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def @@ -66,6 +66,7 @@ NONLOC_SVAL(LazyCompoundVal, NonLoc) NONLOC_SVAL(LocAsInteger, NonLoc) NONLOC_SVAL(SymbolVal, NonLoc) + NONLOC_SVAL(PointerToMember, NonLoc) #undef NONLOC_SVAL #undef LOC_SVAL Index: include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h @@ -204,6 +204,8 @@ const LocationContext *LCtx, unsigned count); + DefinedSVal getMemberPointer(const DeclaratorDecl *DD); + DefinedSVal getFunctionPointer(const FunctionDecl *func); DefinedSVal getBlockPointer(const BlockDecl *block, CanQualType locTy,
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits