From fae3534b3056bb96d26a6d1b6e7d6a2ccaf4fab1 Mon Sep 17 00:00:00 2001 From: Denys Petrov Date: Fri, 28 May 2021 16:52:44 +0300 Subject: [PATCH] [analyzer] Use Optional as a return type of StoreManager::castRegion Summary: Make StoreManager::castRegion function usage safier. Replace `const MemRegion *` with `Optional`. Simplified one of related test cases due to suggestions in D101635. Differential Revision: https://reviews.llvm.org/D103319 --- .../clang/StaticAnalyzer/Core/PathSensitive/Store.h | 3 ++- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp | 20 ++++++++++---------- clang/lib/StaticAnalyzer/Core/Store.cpp | 7 ++++--- clang/test/Analysis/casts.c | 19 +++++-------------- 4 files changed, 21 insertions(+), 28 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h index 947913a..d246170 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h @@ -181,7 +181,8 @@ public: /// castRegion - Used by ExprEngine::VisitCast to handle casts from /// a MemRegion* to a specific location type. 'R' is the region being /// casted and 'CastToTy' the result type of the cast. - const MemRegion *castRegion(const MemRegion *region, QualType CastToTy); + Optional castRegion(const MemRegion *region, + QualType CastToTy); virtual StoreRef removeDeadBindings(Store store, const StackFrameContext *LCtx, SymbolReaper &SymReaper) = 0; diff --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp index 0003c275..3978788 100644 --- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -753,16 +753,16 @@ SVal SValBuilder::evalCastSubKind(loc::MemRegionVal V, QualType CastTy, if (const auto *SR = dyn_cast(R)) { QualType SRTy = SR->getSymbol()->getType(); if (!hasSameUnqualifiedPointeeType(SRTy, CastTy)) { - R = StateMgr.getStoreManager().castRegion(SR, CastTy); - return loc::MemRegionVal(R); + if (auto OptR = StateMgr.getStoreManager().castRegion(SR, CastTy)) + return loc::MemRegionVal(*OptR); } } } // Next fixes pointer dereference using type different from its initial // one. See PR37503 and PR49007 for details. if (const auto *ER = dyn_cast(R)) { - if ((R = StateMgr.getStoreManager().castRegion(ER, CastTy))) - return loc::MemRegionVal(R); + if (auto OptR = StateMgr.getStoreManager().castRegion(ER, CastTy)) + return loc::MemRegionVal(*OptR); } return V; @@ -807,8 +807,8 @@ SVal SValBuilder::evalCastSubKind(loc::MemRegionVal V, QualType CastTy, // Get the result of casting a region to a different type. const MemRegion *R = V.getRegion(); - if ((R = StateMgr.getStoreManager().castRegion(R, CastTy))) - return loc::MemRegionVal(R); + if (auto OptR = StateMgr.getStoreManager().castRegion(R, CastTy)) + return loc::MemRegionVal(*OptR); } // Pointer to whatever else. @@ -873,8 +873,8 @@ SVal SValBuilder::evalCastSubKind(nonloc::LocAsInteger V, QualType CastTy, if (!IsUnknownOriginalType && Loc::isLocType(CastTy) && OriginalTy->isIntegralOrEnumerationType()) { if (const MemRegion *R = L.getAsRegion()) - if ((R = StateMgr.getStoreManager().castRegion(R, CastTy))) - return loc::MemRegionVal(R); + if (auto OptR = StateMgr.getStoreManager().castRegion(R, CastTy)) + return loc::MemRegionVal(*OptR); return L; } @@ -890,8 +890,8 @@ SVal SValBuilder::evalCastSubKind(nonloc::LocAsInteger V, QualType CastTy, // Delegate to store manager to get the result of casting a region to a // different type. If the MemRegion* returned is NULL, this expression // Evaluates to UnknownVal. - if ((R = StateMgr.getStoreManager().castRegion(R, CastTy))) - return loc::MemRegionVal(R); + if (auto OptR = StateMgr.getStoreManager().castRegion(R, CastTy)) + return loc::MemRegionVal(*OptR); } } else { if (Loc::isLocType(CastTy)) { diff --git a/clang/lib/StaticAnalyzer/Core/Store.cpp b/clang/lib/StaticAnalyzer/Core/Store.cpp index c563b44..b867b07 100644 --- a/clang/lib/StaticAnalyzer/Core/Store.cpp +++ b/clang/lib/StaticAnalyzer/Core/Store.cpp @@ -71,7 +71,8 @@ const ElementRegion *StoreManager::GetElementZeroRegion(const SubRegion *R, return MRMgr.getElementRegion(T, idx, R, Ctx); } -const MemRegion *StoreManager::castRegion(const MemRegion *R, QualType CastToTy) { +Optional StoreManager::castRegion(const MemRegion *R, + QualType CastToTy) { ASTContext &Ctx = StateMgr.getContext(); // Handle casts to Objective-C objects. @@ -88,7 +89,7 @@ const MemRegion *StoreManager::castRegion(const MemRegion *R, QualType CastToTy) // We don't know what to make of it. Return a NULL region, which // will be interpreted as UnknownVal. - return nullptr; + return None; } // Now assume we are casting from pointer to pointer. Other cases should @@ -168,7 +169,7 @@ const MemRegion *StoreManager::castRegion(const MemRegion *R, QualType CastToTy) // If we cannot compute a raw offset, throw up our hands and return // a NULL MemRegion*. if (!baseR) - return nullptr; + return None; CharUnits off = rawOff.getOffset(); diff --git a/clang/test/Analysis/casts.c b/clang/test/Analysis/casts.c index 1de7ef5..6b9108a 100644 --- a/clang/test/Analysis/casts.c +++ b/clang/test/Analysis/casts.c @@ -251,18 +251,9 @@ void no_crash_reinterpret_char_as_uchar(char ***a, int *b) { ; } -// See PR50179. -// Just don't crash. -typedef struct taskS { - void *pJob; -} taskS; - -typedef struct workS { - taskS *pTaskList; -} workS; - -void *getTaskJob(unsigned jobId, workS *pWork, unsigned taskId) { - const taskS *pTask = pWork->pTaskList + taskId; - taskS task = *pTask; - return task.pJob; +// PR50179. +struct S {}; +void symbolic_offset(struct S *ptr, int i) { + const struct S *pS = ptr + i; + struct S s = *pS; // no-crash } -- 2.7.4