[analyzer] refactor makeIntValWithPtrWidth, remove getZeroWithPtrWidth (NFC)

[vabridgers]

Summary

This is a NFC refactoring to change makeIntValWithPtrWidth
and remove getZeroWithPtrWidth to use types when forming values to match
pointer widths. Some targets may have different pointer widths depending
upon address space, so this needs to be comprehended.

[steakhal]

clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h

335–336

This function should have a different name.
Probbaly the makeIntValWithWidth() is a better alternative.
Ah but even this is far less than ideal.

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

2613

We peobably wanted to use a different tspe for this expressuon. 'getArrayIndexType()' is a better fit IMO.
We shall see.

[vabridgers]

Update per @steakhal comments

[vabridgers]

I believe the comments have been addressed. Thank you - Vince

[steakhal]

Thanks for the patch. LGTM on my part.
@NoQ WDYT?

[vabridgers]

@NoQ - ping!

[vabridgers]

Is it ok to land this change? Or shall we continue to wait on @NoQ ?

[vabridgers]

rebase

[vabridgers]

ping! @steakhal accepted, should we wait for @NoQ before landing? Thanks!

[NoQ]

Looks like the API was used incorrectly 50% of the time anyway :)

I have one nit but other than that looks good.

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

2613

TotalSize is literally size_t so ASTContext.getSizeType() is the right type. getArrayIndexType() is signed so it's different.

[vabridgers]

update per @NoQ's latest comments

[vabridgers]

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

2613

Thanks @NoQ , the review is update. Best

clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h

const llvm::APSInt &Sub1(const llvm::APSInt &V) {
     return getValue(X);
   }
 
   const llvm::APSInt &getZeroWithTypeSize(QualType T) {
     assert(T->isScalarType());
     return getValue(0, Ctx.getTypeSize(T), true);
   }
 
-  const llvm::APSInt &getZeroWithPtrWidth(bool isUnsigned = true) {
-    return getValue(0, Ctx.getTypeSize(Ctx.VoidPtrTy), isUnsigned);
-  }
-
-  const llvm::APSInt &getIntWithPtrWidth(uint64_t X, bool isUnsigned) {
-    return getValue(X, Ctx.getTypeSize(Ctx.VoidPtrTy), isUnsigned);
-  }
-
   const llvm::APSInt &getTruthValue(bool b, QualType T) {
     return getValue(b ? 1 : 0, Ctx.getIntWidth(T),
                     T->isUnsignedIntegerOrEnumerationType());
   }
 
   const llvm::APSInt &getTruthValue(bool b) {
     return getTruthValue(b, Ctx.getLogicalOperationType());
   }

clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h

DefinedSVal makeIntVal(uint64_t integer, QualType type) {
 
     return nonloc::ConcreteInt(BasicVals.getValue(integer, type));
   }
 
   NonLoc makeIntVal(uint64_t integer, bool isUnsigned) {
     return nonloc::ConcreteInt(BasicVals.getIntValue(integer, isUnsigned));
   }
 
-  NonLoc makeIntValWithPtrWidth(uint64_t integer, bool isUnsigned) {
-    return nonloc::ConcreteInt(
-        BasicVals.getIntWithPtrWidth(integer, isUnsigned));
+  NonLoc makeIntValWithWidth(QualType ptrType, uint64_t integer) {
+    return nonloc::ConcreteInt(BasicVals.getValue(integer, ptrType));
   }
 
   NonLoc makeLocAsInteger(Loc loc, unsigned bits) {
     return nonloc::LocAsInteger(BasicVals.getPersistentSValWithData(loc, bits));
   }
 
   NonLoc makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode op,
                     const llvm::APSInt& rhs, QualType type);

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

MallocChecker::ReallocMemAux(CheckerContext &C, const CallEvent &Call,
   SVal TotalSize = C.getSVal(Arg1);
   if (SuffixWithN)
     TotalSize = evalMulForBufferSize(C, Arg1, CE->getArg(2));
   if (!TotalSize.getAs<DefinedOrUnknownSVal>())
     return nullptr;
 
   // Compare the size argument to 0.
   DefinedOrUnknownSVal SizeZero =
       svalBuilder.evalEQ(State, TotalSize.castAs<DefinedOrUnknownSVal>(),
-                       svalBuilder.makeIntValWithPtrWidth(0, false));
+                         svalBuilder.makeIntValWithWidth(
+                             svalBuilder.getContext().getSizeType(), 0));
 
   ProgramStateRef StatePtrIsNull, StatePtrNotNull;
   std::tie(StatePtrIsNull, StatePtrNotNull) = State->assume(PtrEQ);
   ProgramStateRef StateSizeIsZero, StateSizeNotZero;
   std::tie(StateSizeIsZero, StateSizeNotZero) = State->assume(SizeZero);
   // We only assume exceptional states if they are definitely true; if the
   // state is under-constrained, assume regular realloc behavior.
   bool PrtIsNull = StatePtrIsNull && !StatePtrNotNull;

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

switch (S->getStmtClass()) {
 
     case Stmt::ObjCSubscriptRefExprClass:
     case Stmt::ObjCPropertyRefExprClass:
       llvm_unreachable("These are handled by PseudoObjectExpr");
 
     case Stmt::GNUNullExprClass: {
       // GNU __null is a pointer-width integer, not an actual pointer.
       ProgramStateRef state = Pred->getState();
-      state = state->BindExpr(S, Pred->getLocationContext(),
-                              svalBuilder.makeIntValWithPtrWidth(0, false));
+      state = state->BindExpr(
+          S, Pred->getLocationContext(),
+          svalBuilder.makeIntValWithWidth(getContext().VoidPtrTy, 0));
       Bldr.generateNode(S, Pred, state);
       break;
     }
 
     case Stmt::ObjCAtSynchronizedStmtClass:
       Bldr.takeNodes(Pred);
       VisitObjCAtSynchronizedStmt(cast<ObjCAtSynchronizedStmt>(S), Pred, Dst);
       Bldr.addNodes(Dst);

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

if (const FunctionCodeRegion *FTR = dyn_cast<FunctionCodeRegion>(R))
           return nonloc::SymbolVal(SymMgr.getExtentSymbol(FTR));
 
     if (const SymbolicRegion *SymR = R->getSymbolicBase()) {
       SymbolRef Sym = SymR->getSymbol();
       QualType Ty = Sym->getType();
       // This change is needed for architectures with varying
       // pointer widths. See the amdgcn opencl reproducer with
       // this change as an example: solver-sym-simplification-ptr-bool.cl
-      // FIXME: Cleanup remainder of `getZeroWithPtrWidth ()`
-      //        and `getIntWithPtrWidth()` functions to prevent future
-      //        confusion
       if (!Ty->isReferenceType())
         return makeNonLoc(Sym, BO_NE, BasicVals.getZeroWithTypeSize(Ty),
                           CastTy);
     }
     // Non-symbolic memory regions are always true.
     return makeTruthVal(true, CastTy);
   }