This revision was automatically updated to reflect the committed changes. Closed by commit rG985888411da9: [analyzer] Refactor makeNull to makeNullWithWidth (NFC) (authored by vabridgers, committed by einvbri <vince.a.bridg...@ericsson.com>).
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119601/new/ https://reviews.llvm.org/D119601 Files: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp clang/lib/StaticAnalyzer/Core/RegionStore.cpp clang/lib/StaticAnalyzer/Core/SValBuilder.cpp clang/test/Analysis/cast-value-notes.cpp
Index: clang/test/Analysis/cast-value-notes.cpp =================================================================== --- clang/test/Analysis/cast-value-notes.cpp +++ clang/test/Analysis/cast-value-notes.cpp @@ -1,9 +1,22 @@ // RUN: %clang_analyze_cc1 -std=c++14 \ // RUN: -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\ -// RUN: -analyzer-output=text -verify %s +// RUN: -analyzer-output=text -verify -DDEFAULT_TRIPLE %s 2>&1 | FileCheck %s -check-prefix=DEFAULT-CHECK +// +// RUN: %clang_analyze_cc1 -std=c++14 -triple amdgcn-unknown-unknown \ +// RUN: -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\ +// RUN: -analyzer-output=text -verify -DAMDGCN_TRIPLE %s 2>&1 | FileCheck %s -check-prefix=AMDGCN-CHECK #include "Inputs/llvm.h" +// The amggcn triple case uses an intentionally different address space. +// The core.NullDereference checker intentionally ignores checks +// that use address spaces, so the case is differentiated here. +// +// From https://llvm.org/docs/AMDGPUUsage.html#address-spaces, +// select address space 3 (local), since the pointer size is +// different than Generic. +#define DEVICE __attribute__((address_space(3))) + namespace clang { struct Shape { template <typename T> @@ -21,12 +34,30 @@ using namespace llvm; using namespace clang; +void clang_analyzer_printState(); + +#if defined(DEFAULT_TRIPLE) void evalReferences(const Shape &S) { const auto &C = dyn_cast<Circle>(S); // expected-note@-1 {{Assuming 'S' is not a 'Circle'}} // expected-note@-2 {{Dereference of null pointer}} // expected-warning@-3 {{Dereference of null pointer}} + clang_analyzer_printState(); + // DEFAULT-CHECK: "dynamic_types": [ + // DEFAULT-CHECK-NEXT: { "region": "SymRegion{reg_$0<const struct clang::Shape & S>}", "dyn_type": "const class clang::Circle &", "sub_classable": true } + (void)C; +} +#elif defined(AMDGCN_TRIPLE) +void evalReferences(const Shape &S) { + const auto &C = dyn_cast<DEVICE Circle>(S); + clang_analyzer_printState(); + // AMDGCN-CHECK: "dynamic_types": [ + // AMDGCN-CHECK-NEXT: { "region": "SymRegion{reg_$0<const struct clang::Shape & S>}", "dyn_type": "const __attribute__((address_space(3))) class clang::Circle &", "sub_classable": true } + (void)C; } +#else +#error Target must be specified, and must be pinned +#endif void evalNonNullParamNonNullReturnReference(const Shape &S) { const auto *C = dyn_cast_or_null<Circle>(S); Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -61,7 +61,7 @@ DefinedOrUnknownSVal SValBuilder::makeZeroVal(QualType type) { if (Loc::isLocType(type)) - return makeNull(); + return makeNullWithType(type); if (type->isIntegralOrEnumerationType()) return makeIntVal(0, type); @@ -359,7 +359,7 @@ return makeBoolVal(cast<ObjCBoolLiteralExpr>(E)); case Stmt::CXXNullPtrLiteralExprClass: - return makeNull(); + return makeNullWithType(E->getType()); case Stmt::CStyleCastExprClass: case Stmt::CXXFunctionalCastExprClass: @@ -399,7 +399,7 @@ if (Loc::isLocType(E->getType())) if (E->isNullPointerConstant(Ctx, Expr::NPC_ValueDependentIsNotNull)) - return makeNull(); + return makeNullWithType(E->getType()); return None; } Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -2410,7 +2410,7 @@ SVal V; if (Loc::isLocType(T)) - V = svalBuilder.makeNull(); + V = svalBuilder.makeNullWithType(T); else if (T->isIntegralOrEnumerationType()) V = svalBuilder.makeZeroVal(T); else if (T->isStructureOrClassType() || T->isArrayType()) { Index: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -460,7 +460,8 @@ continue; } else { // If the cast fails on a pointer, bind to 0. - state = state->BindExpr(CastE, LCtx, svalBuilder.makeNull()); + state = state->BindExpr(CastE, LCtx, + svalBuilder.makeNullWithType(resultType)); } } else { // If we don't know if the cast succeeded, conjure a new symbol. @@ -498,7 +499,7 @@ continue; } case CK_NullToPointer: { - SVal V = svalBuilder.makeNull(); + SVal V = svalBuilder.makeNullWithType(CastE->getType()); state = state->BindExpr(CastE, LCtx, V); Bldr.generateNode(CastE, Pred, state); continue; Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -549,8 +549,9 @@ State->BindExpr(CE, C.getLocationContext(), *StreamVal); // Generate state for NULL return value. // Stream switches to OpenFailed state. - ProgramStateRef StateRetNull = State->BindExpr(CE, C.getLocationContext(), - C.getSValBuilder().makeNull()); + ProgramStateRef StateRetNull = + State->BindExpr(CE, C.getLocationContext(), + C.getSValBuilder().makeNullWithType(CE->getType())); StateRetNotNull = StateRetNotNull->set<StreamMap>(StreamSym, StreamState::getOpened(Desc)); Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp +++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp @@ -71,7 +71,8 @@ bool handleMoveCtr(const CallEvent &Call, CheckerContext &C, const MemRegion *ThisRegion) const; bool updateMovedSmartPointers(CheckerContext &C, const MemRegion *ThisRegion, - const MemRegion *OtherSmartPtrRegion) const; + const MemRegion *OtherSmartPtrRegion, + const CallEvent &Call) const; void handleBoolConversion(const CallEvent &Call, CheckerContext &C) const; bool handleComparisionOp(const CallEvent &Call, CheckerContext &C) const; bool handleOstreamOperator(const CallEvent &Call, CheckerContext &C) const; @@ -379,11 +380,13 @@ if (!ThisRegion) return false; + QualType ThisType = cast<CXXMethodDecl>(Call.getDecl())->getThisType(); + if (CC->getDecl()->isMoveConstructor()) return handleMoveCtr(Call, C, ThisRegion); if (Call.getNumArgs() == 0) { - auto NullVal = C.getSValBuilder().makeNull(); + auto NullVal = C.getSValBuilder().makeNullWithType(ThisType); State = State->set<TrackedRegionMap>(ThisRegion, NullVal); C.addTransition( @@ -640,7 +643,8 @@ *InnerPointVal); } - auto ValueToUpdate = C.getSValBuilder().makeNull(); + QualType ThisType = cast<CXXMethodDecl>(Call.getDecl())->getThisType(); + auto ValueToUpdate = C.getSValBuilder().makeNullWithType(ThisType); State = State->set<TrackedRegionMap>(ThisRegion, ValueToUpdate); C.addTransition(State, C.getNoteTag([ThisRegion](PathSensitiveBugReport &BR, @@ -738,13 +742,15 @@ if (!ThisRegion) return false; + QualType ThisType = cast<CXXMethodDecl>(Call.getDecl())->getThisType(); + const MemRegion *OtherSmartPtrRegion = OC->getArgSVal(0).getAsRegion(); // In case of 'nullptr' or '0' assigned if (!OtherSmartPtrRegion) { bool AssignedNull = Call.getArgSVal(0).isZeroConstant(); if (!AssignedNull) return false; - auto NullVal = C.getSValBuilder().makeNull(); + auto NullVal = C.getSValBuilder().makeNullWithType(ThisType); State = State->set<TrackedRegionMap>(ThisRegion, NullVal); C.addTransition(State, C.getNoteTag([ThisRegion](PathSensitiveBugReport &BR, llvm::raw_ostream &OS) { @@ -758,7 +764,7 @@ return true; } - return updateMovedSmartPointers(C, ThisRegion, OtherSmartPtrRegion); + return updateMovedSmartPointers(C, ThisRegion, OtherSmartPtrRegion, Call); } bool SmartPtrModeling::handleMoveCtr(const CallEvent &Call, CheckerContext &C, @@ -767,17 +773,19 @@ if (!OtherSmartPtrRegion) return false; - return updateMovedSmartPointers(C, ThisRegion, OtherSmartPtrRegion); + return updateMovedSmartPointers(C, ThisRegion, OtherSmartPtrRegion, Call); } bool SmartPtrModeling::updateMovedSmartPointers( CheckerContext &C, const MemRegion *ThisRegion, - const MemRegion *OtherSmartPtrRegion) const { + const MemRegion *OtherSmartPtrRegion, const CallEvent &Call) const { ProgramStateRef State = C.getState(); + QualType ThisType = cast<CXXMethodDecl>(Call.getDecl())->getThisType(); const auto *OtherInnerPtr = State->get<TrackedRegionMap>(OtherSmartPtrRegion); if (OtherInnerPtr) { State = State->set<TrackedRegionMap>(ThisRegion, *OtherInnerPtr); - auto NullVal = C.getSValBuilder().makeNull(); + + auto NullVal = C.getSValBuilder().makeNullWithType(ThisType); State = State->set<TrackedRegionMap>(OtherSmartPtrRegion, NullVal); bool IsArgValNull = OtherInnerPtr->isZeroConstant(); @@ -803,7 +811,8 @@ } else { // In case we dont know anything about value we are moving from // remove the entry from map for which smart pointer got moved to. - auto NullVal = C.getSValBuilder().makeNull(); + // For unique_ptr<A>, Ty will be 'A*'. + auto NullVal = C.getSValBuilder().makeNullWithType(ThisType); State = State->remove<TrackedRegionMap>(ThisRegion); State = State->set<TrackedRegionMap>(OtherSmartPtrRegion, NullVal); C.addTransition(State, C.getNoteTag([OtherSmartPtrRegion, @@ -830,6 +839,8 @@ const MemRegion *ThisRegion = cast<CXXInstanceCall>(&Call)->getCXXThisVal().getAsRegion(); + QualType ThisType = cast<CXXMethodDecl>(Call.getDecl())->getThisType(); + SVal InnerPointerVal; if (const auto *InnerValPtr = State->get<TrackedRegionMap>(ThisRegion)) { InnerPointerVal = *InnerValPtr; @@ -868,7 +879,7 @@ std::tie(NotNullState, NullState) = State->assume(InnerPointerVal.castAs<DefinedOrUnknownSVal>()); - auto NullVal = C.getSValBuilder().makeNull(); + auto NullVal = C.getSValBuilder().makeNullWithType(ThisType); // Explicitly tracking the region as null. NullState = NullState->set<TrackedRegionMap>(ThisRegion, NullVal); Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -945,7 +945,8 @@ // Assume that output is zero on the other branch. NullOutputState = NullOutputState->BindExpr( - CE, LCtx, C.getSValBuilder().makeNull(), /*Invalidate=*/false); + CE, LCtx, C.getSValBuilder().makeNullWithType(ResultTy), + /*Invalidate=*/false); C.addTransition(NullOutputState, &getCastFailTag()); // And on the original branch assume that both input and Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -2593,8 +2593,8 @@ SValBuilder &svalBuilder = C.getSValBuilder(); - DefinedOrUnknownSVal PtrEQ = - svalBuilder.evalEQ(State, arg0Val, svalBuilder.makeNull()); + DefinedOrUnknownSVal PtrEQ = svalBuilder.evalEQ( + State, arg0Val, svalBuilder.makeNullWithType(arg0Expr->getType())); // Get the size argument. const Expr *Arg1 = CE->getArg(1); Index: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp @@ -250,7 +250,7 @@ CastSucceeds); SVal V = CastSucceeds ? C.getSValBuilder().evalCast(DV, CastToTy, CastFromTy) - : C.getSValBuilder().makeNull(); + : C.getSValBuilder().makeNullWithType(CastToTy); C.addTransition( State->BindExpr(Call.getOriginExpr(), C.getLocationContext(), V, false), getNoteTag(C, CastInfo, CastToTy, Object, CastSucceeds, IsKnownCast)); @@ -359,7 +359,9 @@ if (ProgramStateRef State = C.getState()->assume(DV, false)) C.addTransition(State->BindExpr(Call.getOriginExpr(), C.getLocationContext(), - C.getSValBuilder().makeNull(), false), + C.getSValBuilder().makeNullWithType( + Call.getOriginExpr()->getType()), + false), C.getNoteTag("Assuming null pointer is passed into cast", /*IsPrunable=*/true)); } Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h @@ -365,13 +365,21 @@ /// space. /// \param type pointer type. Loc makeNullWithType(QualType type) { + // We cannot use the `isAnyPointerType()`. + assert((type->isPointerType() || type->isObjCObjectPointerType() || + type->isBlockPointerType() || type->isNullPtrType() || + type->isReferenceType()) && + "makeNullWithType must use pointer type"); + + // The `sizeof(T&)` is `sizeof(T)`, thus we replace the reference with a + // pointer. Here we assume that references are actually implemented by + // pointers under-the-hood. + type = type->isReferenceType() + ? Context.getPointerType(type->getPointeeType()) + : type; return loc::ConcreteInt(BasicVals.getZeroWithTypeSize(type)); } - Loc makeNull() { - return loc::ConcreteInt(BasicVals.getZeroWithPtrWidth()); - } - Loc makeLoc(SymbolRef sym) { return loc::MemRegionVal(MemMgr.getSymbolicRegion(sym)); }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
