[clang] 9858884 - [analyzer] Refactor makeNull to makeNullWithWidth (NFC)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 22 05:35:28 PDT 2022
Author: Vince Bridgers
Date: 2022-03-22T07:35:13-05:00
New Revision: 985888411da9c62aea7b0b41f93eb75ad31587a6
URL: https://github.com/llvm/llvm-project/commit/985888411da9c62aea7b0b41f93eb75ad31587a6
DIFF: https://github.com/llvm/llvm-project/commit/985888411da9c62aea7b0b41f93eb75ad31587a6.diff
LOG: [analyzer] Refactor makeNull to makeNullWithWidth (NFC)
Usages of makeNull need to be deprecated in favor of makeNullWithWidth
for architectures where the pointer size should not be assumed. This can
occur when pointer sizes can be of different sizes, depending on address
space for example. See https://reviews.llvm.org/D118050 as an example.
This was uncovered initially in a downstream compiler project, and
tested through those systems tests.
steakhal performed systems testing across a large set of open source
projects.
Co-authored-by: steakhal
Resolves: https://github.com/llvm/llvm-project/issues/53664
Reviewed By: NoQ, steakhal
Differential Revision: https://reviews.llvm.org/D119601
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
index 1df47dae25bf5..1f02ee84c898a 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -365,13 +365,21 @@ class SValBuilder {
/// 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));
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
index 4235c0c138210..e2b99fca7d148 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
@@ -250,7 +250,7 @@ static void addCastTransition(const CallEvent &Call, DefinedOrUnknownSVal DV,
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 @@ static void evalNullParamNullReturn(const CallEvent &Call,
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));
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 238022d1253b9..63b1065ae4de0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2593,8 +2593,8 @@ MallocChecker::ReallocMemAux(CheckerContext &C, const CallEvent &Call,
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);
diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
index 0bde088d0e856..f43b8d95f0d1f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -945,7 +945,8 @@ bool RetainCountChecker::evalCall(const CallEvent &Call,
// 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
diff --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
index c789a8dbcca16..d1b8db38a6b60 100644
--- a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -71,7 +71,8 @@ class SmartPtrModeling
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 @@ bool SmartPtrModeling::evalCall(const CallEvent &Call,
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 @@ void SmartPtrModeling::handleRelease(const CallEvent &Call,
*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 @@ bool SmartPtrModeling::handleAssignOp(const CallEvent &Call,
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 @@ bool SmartPtrModeling::handleAssignOp(const CallEvent &Call,
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 @@ bool SmartPtrModeling::handleMoveCtr(const CallEvent &Call, CheckerContext &C,
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 @@ bool SmartPtrModeling::updateMovedSmartPointers(
} 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 @@ void SmartPtrModeling::handleBoolConversion(const CallEvent &Call,
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 @@ void SmartPtrModeling::handleBoolConversion(const CallEvent &Call,
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);
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 26218b8e0454a..099e70aaf7eca 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -549,8 +549,9 @@ void StreamChecker::evalFreopen(const FnDescription *Desc,
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));
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index 302a971a15f21..21fd46e308f3f 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -460,7 +460,8 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex,
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 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex,
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;
diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index cad5f66a61aa8..9d062ddb3e8c1 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2410,7 +2410,7 @@ RegionStoreManager::setImplicitDefaultValue(RegionBindingsConstRef B,
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()) {
diff --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
index bb3261bae3bf9..a4a65ad20b577 100644
--- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -61,7 +61,7 @@ SValBuilder::SValBuilder(llvm::BumpPtrAllocator &alloc, ASTContext &context,
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 @@ Optional<SVal> SValBuilder::getConstantVal(const Expr *E) {
return makeBoolVal(cast<ObjCBoolLiteralExpr>(E));
case Stmt::CXXNullPtrLiteralExprClass:
- return makeNull();
+ return makeNullWithType(E->getType());
case Stmt::CStyleCastExprClass:
case Stmt::CXXFunctionalCastExprClass:
@@ -399,7 +399,7 @@ Optional<SVal> SValBuilder::getConstantVal(const Expr *E) {
if (Loc::isLocType(E->getType()))
if (E->isNullPointerConstant(Ctx, Expr::NPC_ValueDependentIsNotNull))
- return makeNull();
+ return makeNullWithType(E->getType());
return None;
}
diff --git a/clang/test/Analysis/cast-value-notes.cpp b/clang/test/Analysis/cast-value-notes.cpp
index a09586309fb41..19a9f660f51c6 100644
--- a/clang/test/Analysis/cast-value-notes.cpp
+++ b/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
diff erent address space.
+// The core.NullDereference checker intentionally ignores checks
+// that use address spaces, so the case is
diff erentiated here.
+//
+// From https://llvm.org/docs/AMDGPUUsage.html#address-spaces,
+// select address space 3 (local), since the pointer size is
+//
diff erent than Generic.
+#define DEVICE __attribute__((address_space(3)))
+
namespace clang {
struct Shape {
template <typename T>
@@ -21,12 +34,30 @@ class Circle : public Shape {};
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 at -1 {{Assuming 'S' is not a 'Circle'}}
// expected-note at -2 {{Dereference of null pointer}}
// expected-warning at -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);
More information about the cfe-commits
mailing list