vabridgers created this revision.
vabridgers added reviewers: NoQ, martong.
Herald added subscribers: manas, steakhal, ASDenysPetrov, dkrupp, donat.nagy,
Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware,
xazax.hun.
vabridgers requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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
Repository:
rG LLVM Github Monorepo
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
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;
@@ -383,7 +384,8 @@
return handleMoveCtr(Call, C, ThisRegion);
if (Call.getNumArgs() == 0) {
- auto NullVal = C.getSValBuilder().makeNull();
+ auto NullVal = C.getSValBuilder().makeNullWithType(
+ CC->getCXXThisVal().getType(C.getASTContext()));
State = State->set<TrackedRegionMap>(ThisRegion, NullVal);
C.addTransition(
@@ -640,7 +642,8 @@
*InnerPointVal);
}
- auto ValueToUpdate = C.getSValBuilder().makeNull();
+ auto ValueToUpdate = C.getSValBuilder().makeNullWithType(
+ IC->getCXXThisVal().getType(C.getASTContext()));
State = State->set<TrackedRegionMap>(ThisRegion, ValueToUpdate);
C.addTransition(State, C.getNoteTag([ThisRegion](PathSensitiveBugReport &BR,
@@ -744,7 +747,8 @@
bool AssignedNull = Call.getArgSVal(0).isZeroConstant();
if (!AssignedNull)
return false;
- auto NullVal = C.getSValBuilder().makeNull();
+ auto NullVal = C.getSValBuilder().makeNullWithType(
+ Call.getArgSVal(0).getType(C.getASTContext()));
State = State->set<TrackedRegionMap>(ThisRegion, NullVal);
C.addTransition(State, C.getNoteTag([ThisRegion](PathSensitiveBugReport &BR,
llvm::raw_ostream &OS) {
@@ -758,7 +762,7 @@
return true;
}
- return updateMovedSmartPointers(C, ThisRegion, OtherSmartPtrRegion);
+ return updateMovedSmartPointers(C, ThisRegion, OtherSmartPtrRegion, Call);
}
bool SmartPtrModeling::handleMoveCtr(const CallEvent &Call, CheckerContext &C,
@@ -767,17 +771,18 @@
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();
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(
+ OtherInnerPtr->getType(C.getASTContext()));
State = State->set<TrackedRegionMap>(OtherSmartPtrRegion, NullVal);
bool IsArgValNull = OtherInnerPtr->isZeroConstant();
@@ -803,7 +808,16 @@
} 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();
+ const ClassTemplateSpecializationDecl *SpecDecl =
+ dyn_cast<ClassTemplateSpecializationDecl>(
+ cast<CXXMethodDecl>(Call.getDecl())->getParent());
+ ArrayRef<TemplateArgument> TemplateArgs =
+ SpecDecl->getTemplateArgs().asArray();
+ const TemplateArgument &T = TemplateArgs[0];
+ assert(T.getKind() == TemplateArgument::Type);
+ QualType Ty = C.getASTContext().getPointerType(T.getAsType());
+ // For unique_ptr<A>, Ty will be 'A*'.
+ auto NullVal = C.getSValBuilder().makeNullWithType(Ty);
State = State->remove<TrackedRegionMap>(ThisRegion);
State = State->set<TrackedRegionMap>(OtherSmartPtrRegion, NullVal);
C.addTransition(State, C.getNoteTag([OtherSmartPtrRegion,
@@ -868,7 +882,7 @@
std::tie(NotNullState, NullState) =
State->assume(InnerPointerVal.castAs<DefinedOrUnknownSVal>());
- auto NullVal = C.getSValBuilder().makeNull();
+ auto NullVal = C.getSValBuilder().makeNullWithType(CallExpr->getType());
// 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
@@ -2543,8 +2543,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
@@ -249,8 +249,11 @@
State = setDynamicTypeAndCastInfo(State, MR, CastFromTy, CastToTy,
CastSucceeds);
+ // Vince -- TODO: publish to github branch first for Balazs to check. and
+ // check against our code base.
+
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 +362,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,11 +365,14 @@
/// space.
/// \param type pointer type.
Loc makeNullWithType(QualType type) {
- return loc::ConcreteInt(BasicVals.getZeroWithTypeSize(type));
- }
-
- Loc makeNull() {
- return loc::ConcreteInt(BasicVals.getZeroWithPtrWidth());
+ // step1 - this assert
+ // step2 - remove this assert, add the steps from CastValueChecker.cpp
+ // assert(!type->isReferenceType());
+ QualType nullType = type;
+ if (type->isReferenceType()) {
+ nullType = Context.getPointerType(type->getPointeeType());
+ }
+ return loc::ConcreteInt(BasicVals.getZeroWithTypeSize(nullType));
}
Loc makeLoc(SymbolRef sym) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
