[analyzer] Refactor makeNull to makeNullWithWidth (NFC)

[vabridgers]

Summary

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

[NoQ]

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

646–648

I don't think this is the right type. This should be the raw pointer type for the corresponding smart pointer, not a pointer to the smart pointer.

Call.getResultType() should be good.

Also general advice, do not use SVal::getType() when you can obtain the type from the AST instead. SVal::getType() is speculative and it inevitably loses some information (in particular, it doesn't discriminate between lvalues and pointers).

788

Same here.

815

You're using dyn_cast but you're unconditionally dereferencing the result later. Looking at other copies of similar code, you probably need a null check.

(Does running the static analyzer produce a warning here? We have a checker for this!)

882

I suspect that this type is going to be bool which is probably not what you want.

[NoQ]

steakhal performed systems testing across a large set of open source projects.

I'm curious if any findings there caused you to make changes in the patch. If so it'd be great to reduce them into test cases so that other people didn't make the same mistake when they edit your new code.

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

882

I think it makes sense to add an assertion in makeNullWithType() to protect against such cases. This will probably also allow you to cover this with a test case.

[vabridgers]

Thanks for the comments. I'll address and revert back soon.

[vabridgers]

In D119601#3317317, @NoQ wrote:

steakhal performed systems testing across a large set of open source projects.

I'm curious if any findings there caused you to make changes in the patch. If so it'd be great to reduce them into test cases so that other people didn't make the same mistake when they edit your new code.

This patch was driven mostly by the negative cases we uncovered as represented in patch https://reviews.llvm.org/D118050. Basically, we have an internal proprietary multicore CPU with a CPU-local address space and a "global" common address space. We support memcpy intrinsics to copy to/from these memories with the different address space pointers - which also happen to be different pointer widths. The CString checker does an overlap check, and an assert caught a pointer comparison using different pointer widths. So the basic fix is to avoid overlap checks when the pointers are of different address spaces, and then that change ripples into these changes and the required supporting test cases. We also see similar problems in the SMTConv layer that we're just patching up internally for now in a non-principled way that are not suitable for upstreaming (yet).

Since our target is downstream, it's challenging to find equivalent and relevant test cases, but we've made some headway since we found some GPUs also have similar properties. https://reviews.llvm.org/D118050 has one such test case. Perhaps it makes sense to squash that change with this one? @NoQ, do you have thoughts or a recommendation for squashing these two changes?

Looks like our internal test cases can be refactored to use GPU address spaces for supporting test cases, I'll add those.

As always, thanks for the guidance. Best

[vabridgers]

I explored some test cases, and the rabbit hole gets deeper :) Continuing to explore. Best

[vabridgers]

Address NoQ comments

[vabridgers]

I believe I've addressed all of Artem's comments thus far. This is a NFC patch, so will include no test cases. We detected these inconsistencies in getting NULL pointers in our downstream target, that supports 2 different pointer sizes depending on address space. That review is here, https://reviews.llvm.org/D118050, and is dependent upon these fixes (makeNull -> makeNullWithWidth refactoring). These two patches will be kept separate, with this patch reviewed and submitted first followed by https://reviews.llvm.org/D118050.

Please let me know what else needs to be done to move this patch forward. Thanks - Vince

[steakhal]

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

372

You should also remove the BasicValueFactor::getZeroWithPtrWidth() along with BasicValueFactor::getIntWithPtrWidth() since those suffer from the same issue.
Feel free to do that in a separate patch.

382–383

Add a comment that isAnyPointerType() won't work here.

385

But you also let reference types.

386–390

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

123

You don't need the whole CheckerContext. Pass the ASTContext instead.
I would also recommend using Idx instead of ndx

133

You should just return it. Oh wait, you also wrap it inside a pointer. The name of the function does not reflect this. Either rename it or hoist this to the callsite.

390

You should use the CC->getDecl()->getThisType() instead.

647

Similarly to the previous one, use getThisType().

753

Same here.

789

At first glance, all callsites have access to the getThisType(), so we shouldn't dig this up from the specialization decl.

882

You probably never want to use Call.getResultType() in this file for this context.
Something like this would be more appropriate: cast<CXXMethodDecl>(Call.getDecl())->getThisType()

[vabridgers]

refactor based on @steakhal comments

[vabridgers]

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

372

Will do. I had also left "TODO" comments in a different source file. I'll find those, clean those up also.

[steakhal]

I'm satisfied. I cannot see any issues.
AFAI remember the tests runs uncovered no report changes - as expected from an NFC change.
So it's good to go on my part.
Wait for a review from @NoQ, just to be sure. Unless Artem has objections, just commit this and the whole patch stack next week.

[vabridgers]

@NoQ - ping!

[vabridgers]

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

[steakhal]

Land it.

[NoQ]

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

378

How do you discover the pointer width by looking only at the pointee type? Are objects of specific type always restricted to a specific address space? I.e., does every int * have the same width everywhere in the program? If not, how is this code supposed to work?

[vabridgers]

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

378

The case this code addresses is represented in the test case clang/test/Analysis/cast-value-notes.cpp, this function ...

24 void evalReferences(const Shape &S) {
25   const auto &C = dyn_cast<Circle>(S);
26   // expected-note@-1 {{Assuming 'S' is not a 'Circle'}}
27   // expected-note@-2 {{Dereference of null pointer}}
28   // expected-warning@-3 {{Dereference of null pointer}}
29 }

The crash occurs from line 25 above.

Debug printing what I see at this point in the code for this case ...

QualType type : LValueReferenceType 0xffea820 'const class clang::Circle &'
`-QualType 0xffea7c1 'const class clang::Circle' const
  `-SubstTemplateTypeParmType 0xffea7c0 'class clang::Circle' sugar
    |-TemplateTypeParmType 0xffe4f90 'X' dependent depth 0 index 0
    | `-TemplateTypeParm 0xffe4f40 'X'
    `-RecordType 0xffea090 'class clang::Circle'
      `-CXXRecord 0xffea000 'Circle'

Context.getPointerType(type) : PointerType 0x10006ab0 'const class clang::Circle &*'
`-LValueReferenceType 0xffea820 'const class clang::Circle &'
  `-QualType 0xffea7c1 'const class clang::Circle' const
    `-SubstTemplateTypeParmType 0xffea7c0 'class clang::Circle' sugar
      |-TemplateTypeParmType 0xffe4f90 'X' dependent depth 0 index 0
      | `-TemplateTypeParm 0xffe4f40 'X'
      `-RecordType 0xffea090 'class clang::Circle'
        `-CXXRecord 0xffea000 'Circle'

Context.getPointerType(type->getPointeeType()) : PointerType 0x10006ae0 'const class clang::Circle *'
`-QualType 0xffea7c1 'const class clang::Circle' const
  `-SubstTemplateTypeParmType 0xffea7c0 'class clang::Circle' sugar
    |-TemplateTypeParmType 0xffe4f90 'X' dependent depth 0 index 0
    | `-TemplateTypeParm 0xffe4f40 'X'
    `-RecordType 0xffea090 'class clang::Circle'
      `-CXXRecord 0xffea000 'Circle'

The LValueReferenceType causes a crash if I do not get a pointer type, looks like ...

clang:  <root>/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h:219: const llvm::APSInt& clang::ento::BasicValueFactory::getZeroWithTypeSize(clang::QualType): Assertion `T->isScalarType()' failed.

I'm assuming the pointer type retains the address space attribute of the LValueReferenceType.

Context.getPointerType(type->getPointeeType()) produces a QualType : PointerType 'const class clang::Circle *' from an original QualType : LValueReferenceType 'const class clang::Circle &'

Is this not what's wanted - a pointer type instead of a reference, with the same address space qualifiers, or am I missing something in the query?

Thanks

[NoQ]

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

378

Do I understand correctly that __attribute__((address_space)) is a type attribute that gets applied to values rather than to pointers, so __attribute__((address_space(3))) int *x defines x as "pointer to (int that lives in address_space(3))", so when you unwrap the pointer type you get an "int that lives in address_space(3)"?

Can you share some dumps to demonstrate that the attribute is correctly preserved by this procedure? Damn, a test case could be great if we could have them.

[vabridgers]

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

378

I'll see what can be done about a test case, and provide the dumps for evaluation. Best

[vabridgers]

add updated test cases per comments from NoQ

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

378

I developed "a" test methodology to demonstrate this at work. I modified the LIT case to use "dyn_cast<Circle>(S)" and "dyn_cast<attribute((address_space(3))) Circle>(S)", checking the program state using clang_analyzer_printState(). Here's the info I see from the program state and from debug prints in each case.

First, the "dyn_cast<Circle>(S)" case.

void evalReferences(const Shape &S) {
  const auto &C = dyn_cast<Circle>(S);
  clang_analyzer_printState();
  ...
}

  "dynamic_types": [
    { "region": "SymRegion{reg_$0<const struct clang::Shape & S>}", "dyn_type": "const class clang::Circle &", "sub_classable": true }


QualType type : LValueReferenceType 0x1118ef60 'const class clang::Circle &'
`-QualType 0x1118ef01 'const class clang::Circle' const
  `-SubstTemplateTypeParmType 0x1118ef00 'class clang::Circle' sugar
    |-TemplateTypeParmType 0x111887d0 'X' dependent depth 0 index 0
    | `-TemplateTypeParm 0x11188780 'X'
    `-RecordType 0x1118e710 'class clang::Circle'
      `-CXXRecord 0x1118e680 'Circle'

Context.getPointerType(type) : PointerType 0x111a94d0 'const class clang::Circle &*'
`-LValueReferenceType 0x1118ef60 'const class clang::Circle &'
  `-QualType 0x1118ef01 'const class clang::Circle' const
    `-SubstTemplateTypeParmType 0x1118ef00 'class clang::Circle' sugar
      |-TemplateTypeParmType 0x111887d0 'X' dependent depth 0 index 0
      | `-TemplateTypeParm 0x11188780 'X'
      `-RecordType 0x1118e710 'class clang::Circle'
        `-CXXRecord 0x1118e680 'Circle'

Context.getPointerType(type->getPointeeType()) : PointerType 0x111a9500 'const class clang::Circle *'
`-QualType 0x1118ef01 'const class clang::Circle' const
  `-SubstTemplateTypeParmType 0x1118ef00 'class clang::Circle' sugar
    |-TemplateTypeParmType 0x111887d0 'X' dependent depth 0 index 0
    | `-TemplateTypeParm 0x11188780 'X'
    `-RecordType 0x1118e710 'class clang::Circle'
      `-CXXRecord 0x1118e680 'Circle'

then the "dyn_cast<attribute((address_space(3))) Circle>(S)" case

void evalReferences(const Shape &S) {
  const auto &C = dyn_cast<__attribute__((address_space(3))) Circle>(S);
  clang_analyzer_printState();
  ...
}

 "dynamic_types": [
    { "region": "SymRegion{reg_$0<const struct clang::Shape & S>}", "dyn_type": "const __attribute__((address_space(3))) class clang::Circle &", "sub_classable": true }


QualType type : LValueReferenceType 0x11527030 'const __attribute__((address_space(3))) class clang::Circle &'
`-QualType 0x11526fd1 'const __attribute__((address_space(3))) class clang::Circle' const
  `-SubstTemplateTypeParmType 0x11526fd0 '__attribute__((address_space(3))) class clang::Circle' sugar
    |-TemplateTypeParmType 0x115207d0 'X' dependent depth 0 index 0
    | `-TemplateTypeParm 0x11520780 'X'
    `-QualType 0x11526ce8 '__attribute__((address_space(3))) class clang::Circle' __attribute__((address_space(3)))
      `-RecordType 0x11526710 'class clang::Circle'
        `-CXXRecord 0x11526680 'Circle'

Context.getPointerType(type) : PointerType 0x115424e0 'const __attribute__((address_space(3))) class clang::Circle &*'
`-LValueReferenceType 0x11527030 'const __attribute__((address_space(3))) class clang::Circle &'
  `-QualType 0x11526fd1 'const __attribute__((address_space(3))) class clang::Circle' const
    `-SubstTemplateTypeParmType 0x11526fd0 '__attribute__((address_space(3))) class clang::Circle' sugar
      |-TemplateTypeParmType 0x115207d0 'X' dependent depth 0 index 0
      | `-TemplateTypeParm 0x11520780 'X'
      `-QualType 0x11526ce8 '__attribute__((address_space(3))) class clang::Circle' __attribute__((address_space(3)))
        `-RecordType 0x11526710 'class clang::Circle'
          `-CXXRecord 0x11526680 'Circle'

Context.getPointerType(type->getPointeeType()) : PointerType 0x11542510 'const __attribute__((address_space(3))) class clang::Circle *'
`-QualType 0x11526fd1 'const __attribute__((address_space(3))) class clang::Circle' const
  `-SubstTemplateTypeParmType 0x11526fd0 '__attribute__((address_space(3))) class clang::Circle' sugar
    |-TemplateTypeParmType 0x115207d0 'X' dependent depth 0 index 0
    | `-TemplateTypeParm 0x11520780 'X'
    `-QualType 0x11526ce8 '__attribute__((address_space(3))) class clang::Circle' __attribute__((address_space(3)))
      `-RecordType 0x11526710 'class clang::Circle'
        `-CXXRecord 0x11526680 'Circle'

So it looks to me like the address space attribute is retained through both the "Context.getPointerType(type)" and "Context.getPointerType(type->getPointeeType())" invocations.

This also revealed the core.NullDereference checker is ignoring pointers with address space attributes - so the test cases need to be different for this change (at least for now).

I think this is enough to prove this is a NFC. Please let me know if there's anything else I need to do here.

Best

[NoQ]

All clear now. Amazing, thank you for your detailed response, I've corrected my understanding of address spaces so I'll be able to understand future patches better.

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

public:
   nonloc::ConcreteInt makeTruthVal(bool b) {
     return nonloc::ConcreteInt(BasicVals.getTruthValue(b));
   }
 
   /// Create NULL pointer, with proper pointer bit-width for given address
   /// 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));
   }
 
   Loc makeLoc(const MemRegion* region) {
     return loc::MemRegionVal(region);
   }
 
   Loc makeLoc(const AddrLabelExpr *expr) {
     return loc::GotoLabel(expr->getLabel());
   }
 
   Loc makeLoc(const llvm::APSInt& integer) {
     return loc::ConcreteInt(BasicVals.getValue(integer));
   }
 

clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp

static void addCastTransition(const CallEvent &Call, DefinedOrUnknownSVal DV,
 
   // Store the type and the cast information.
   bool IsKnownCast = CastInfo || IsCheckedCast || CastFromTy == CastToTy;
   if (!IsKnownCast || IsCheckedCast)
     State = setDynamicTypeAndCastInfo(State, MR, CastFromTy, CastToTy,
                                       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));
 }
 
 static void addInstanceOfTransition(const CallEvent &Call,
                                     DefinedOrUnknownSVal DV,
                                     ProgramStateRef State, CheckerContext &C,

 }
 
 static void evalNullParamNullReturn(const CallEvent &Call,
                                     DefinedOrUnknownSVal DV,
                                     CheckerContext &C) {
   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));
 }
 
 void CastValueChecker::evalCast(const CallEvent &Call, DefinedOrUnknownSVal DV,
                                 CheckerContext &C) const {
   evalNonNullParamNonNullReturn(Call, DV, C, /*IsCheckedCast=*/true);
 }

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

MallocChecker::ReallocMemAux(CheckerContext &C, const CallEvent &Call,
   const Expr *arg0Expr = CE->getArg(0);
   SVal Arg0Val = C.getSVal(arg0Expr);
   if (!Arg0Val.getAs<DefinedOrUnknownSVal>())
     return nullptr;
   DefinedOrUnknownSVal arg0Val = Arg0Val.castAs<DefinedOrUnknownSVal>();
 
   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);
 
   // Get the value of the size argument.
   SVal TotalSize = C.getSVal(Arg1);
   if (SuffixWithN)
     TotalSize = evalMulForBufferSize(C, Arg1, CE->getArg(2));

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp

if (BSmr == BehaviorSummary::Identity ||
     state = state->BindExpr(CE, LCtx, RetVal, /*Invalidate=*/false);
 
     if (BSmr == BehaviorSummary::IdentityOrZero) {
       // Add a branch where the output is zero.
       ProgramStateRef NullOutputState = C.getState();
 
       // 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
       // output are non-zero.
       if (auto L = RetVal.getAs<DefinedOrUnknownSVal>())
         state = state->assume(*L, /*assumption=*/true);
 
     }

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

private:
   void handleReset(const CallEvent &Call, CheckerContext &C) const;
   void handleRelease(const CallEvent &Call, CheckerContext &C) const;
   void handleSwapMethod(const CallEvent &Call, CheckerContext &C) const;
   void handleGet(const CallEvent &Call, CheckerContext &C) const;
   bool handleAssignOp(const CallEvent &Call, CheckerContext &C) const;
   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;
   bool handleSwap(ProgramStateRef State, SVal First, SVal Second,
                   CheckerContext &C) const;
   std::pair<SVal, ProgramStateRef>
   retrieveOrConjureInnerPtrVal(ProgramStateRef State,
                                const MemRegion *ThisRegion, const Expr *E,

 static bool isStdSmartPtr(const CXXRecordDecl *RD) {
   return hasStdClassWithName(RD, STD_PTR_NAMES);
 }
 
 static bool isStdSmartPtr(const Expr *E) {
   return isStdSmartPtr(E->getType()->getAsCXXRecordDecl());
 }
 
 // Define the inter-checker API.
 namespace clang {
 namespace ento {
 namespace smartptr {
 bool isStdSmartPtrCall(const CallEvent &Call) {
   const auto *MethodDecl = dyn_cast_or_null<CXXMethodDecl>(Call.getDecl());
   if (!MethodDecl || !MethodDecl->getParent())
     return false;
   return isStdSmartPtr(MethodDecl->getParent());
 }
 
 bool isStdSmartPtr(const CXXRecordDecl *RD) {
   if (!RD || !RD->getDeclContext()->isStdNamespace())
     return false;
 
   if (RD->getDeclName().isIdentifier()) {
     StringRef Name = RD->getName();
     return Name == "shared_ptr" || Name == "unique_ptr" || Name == "weak_ptr";
   }
bool SmartPtrModeling::evalCall(const CallEvent &Call,
   if (const auto *CC = dyn_cast<CXXConstructorCall>(&Call)) {
     if (CC->getDecl()->isCopyConstructor())
       return false;
 
     const MemRegion *ThisRegion = CC->getCXXThisVal().getAsRegion();
     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(
           State, C.getNoteTag([ThisRegion](PathSensitiveBugReport &BR,
                                            llvm::raw_ostream &OS) {
             if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
                 !BR.isInteresting(ThisRegion))
               return;
             OS << "Default constructed smart pointer";
void SmartPtrModeling::handleRelease(const CallEvent &Call,
 
   const auto *InnerPointVal = State->get<TrackedRegionMap>(ThisRegion);
 
   if (InnerPointVal) {
     State = State->BindExpr(Call.getOriginExpr(), C.getLocationContext(),
                             *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,
                                                    llvm::raw_ostream &OS) {
     if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
         !BR.isInteresting(ThisRegion))
       return;
 
     OS << "Smart pointer";
if (!OC)
     return false;
   OverloadedOperatorKind OOK = OC->getOverloadedOperator();
   if (OOK != OO_Equal)
     return false;
   const MemRegion *ThisRegion = OC->getCXXThisVal().getAsRegion();
   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) {
       if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
           !BR.isInteresting(ThisRegion))
         return;
       OS << "Smart pointer";
       checkAndPrettyPrintRegion(OS, ThisRegion);
       OS << " is assigned to null";
     }));
     return true;
   }
 
-  return updateMovedSmartPointers(C, ThisRegion, OtherSmartPtrRegion);
+  return updateMovedSmartPointers(C, ThisRegion, OtherSmartPtrRegion, Call);
 }
 
 bool SmartPtrModeling::handleMoveCtr(const CallEvent &Call, CheckerContext &C,
                                      const MemRegion *ThisRegion) const {
   const auto *OtherSmartPtrRegion = Call.getArgSVal(0).getAsRegion();
   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();
 
     C.addTransition(
         State,
         C.getNoteTag([ThisRegion, OtherSmartPtrRegion, IsArgValNull](
                          PathSensitiveBugReport &BR, llvm::raw_ostream &OS) {
           if (&BR.getBugType() != smartptr::getNullDereferenceBugType())
             return;
           if (BR.isInteresting(OtherSmartPtrRegion)) {
             OS << "Smart pointer";
             checkAndPrettyPrintRegion(OS, OtherSmartPtrRegion);
             OS << " is null after being moved to";
             checkAndPrettyPrintRegion(OS, ThisRegion);
           }
           if (BR.isInteresting(ThisRegion) && IsArgValNull) {
             OS << "A null pointer value is moved to";
             checkAndPrettyPrintRegion(OS, ThisRegion);
             BR.markInteresting(OtherSmartPtrRegion);
           }
         }));
     return true;
   } 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,
                                          ThisRegion](PathSensitiveBugReport &BR,
                                                      llvm::raw_ostream &OS) {
       if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
           !BR.isInteresting(OtherSmartPtrRegion))
         return;

 void SmartPtrModeling::handleBoolConversion(const CallEvent &Call,
                                             CheckerContext &C) const {
   // To model unique_ptr::operator bool
   ProgramStateRef State = C.getState();
   const Expr *CallExpr = Call.getOriginExpr();
   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;
   } else {
     // In case of inner pointer SVal is not available we create
     // conjureSymbolVal for inner pointer value.
     auto InnerPointerType = getInnerPointerType(Call, C);
     if (InnerPointerType.isNull())
C.addTransition(
         State->BindExpr(CallExpr, C.getLocationContext(),
                         C.getSValBuilder().makeZeroVal(Call.getResultType())));
     return;
   } else {
     ProgramStateRef NotNullState, NullState;
     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);
 
     NullState = NullState->BindExpr(CallExpr, C.getLocationContext(),
                                     C.getSValBuilder().makeTruthVal(false));
     C.addTransition(NullState, C.getNoteTag(
                                    [ThisRegion](PathSensitiveBugReport &BR,
                                                 llvm::raw_ostream &OS) {

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

void StreamChecker::evalFreopen(const FnDescription *Desc,
   // Generate state for non-failed case.
   // Return value is the passed stream pointer.
   // According to the documentations, the stream is closed first
   // but any close error is ignored. The state changes to (or remains) opened.
   ProgramStateRef StateRetNotNull =
       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));
   StateRetNull =
       StateRetNull->set<StreamMap>(StreamSym, StreamState::getOpenFailed(Desc));
 
   C.addTransition(StateRetNotNull,
                   constructNoteTag(C, StreamSym, "Stream reopened here"));

clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp

switch (CastE->getCastKind()) {
         if (Failed) {
           if (T->isReferenceType()) {
             // A bad_cast exception is thrown if input value is a reference.
             // Currently, we model this, by generating a sink.
             Bldr.generateSink(CastE, Pred, state);
             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.
           if (val.isUnknown()) {
             DefinedOrUnknownSVal NewSym =
               svalBuilder.conjureSymbolVal(nullptr, CastE, LCtx, resultType,
                                            currBldrCtx->blockCount());
             state = state->BindExpr(CastE, LCtx, NewSym);
switch (CastE->getCastKind()) {
             svalBuilder.conjureSymbolVal(nullptr, CastE, LCtx, resultType,
                                          currBldrCtx->blockCount());
         }
         state = state->BindExpr(CastE, LCtx, val);
         Bldr.generateNode(CastE, Pred, state);
         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;
       }
       case CK_NullToMemberPointer: {
         SVal V = svalBuilder.getMemberPointer(nullptr);
         state = state->BindExpr(CastE, LCtx, V);
         Bldr.generateNode(CastE, Pred, state);

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

 
 RegionBindingsRef
 RegionStoreManager::setImplicitDefaultValue(RegionBindingsConstRef B,
                                             const MemRegion *R,
                                             QualType T) {
   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()) {
     // Set the default value to a zero constant when it is a structure
     // or array.  The type doesn't really matter.
     V = svalBuilder.makeZeroVal(Ctx.IntTy);
   }
   else {

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

: Context(context), BasicVals(context, alloc),
       StateMgr(stateMgr),
       AnOpts(
           stateMgr.getOwningEngine().getAnalysisManager().getAnalyzerOptions()),
       ArrayIndexTy(context.LongLongTy),
       ArrayIndexWidth(context.getTypeSize(ArrayIndexTy)) {}
 
 DefinedOrUnknownSVal SValBuilder::makeZeroVal(QualType type) {
   if (Loc::isLocType(type))
-    return makeNull();
+    return makeNullWithType(type);
 
   if (type->isIntegralOrEnumerationType())
     return makeIntVal(0, type);
 
   if (type->isArrayType() || type->isRecordType() || type->isVectorType() ||
       type->isAnyComplexType())
     return makeCompoundVal(type, BasicVals.getEmptySValList());
 
Optional<SVal> SValBuilder::getConstantVal(const Expr *E) {
 
   case Stmt::IntegerLiteralClass:
     return makeIntVal(cast<IntegerLiteral>(E));
 
   case Stmt::ObjCBoolLiteralExprClass:
     return makeBoolVal(cast<ObjCBoolLiteralExpr>(E));
 
   case Stmt::CXXNullPtrLiteralExprClass:
-    return makeNull();
+    return makeNullWithType(E->getType());
 
   case Stmt::CStyleCastExprClass:
   case Stmt::CXXFunctionalCastExprClass:
   case Stmt::CXXConstCastExprClass:
   case Stmt::CXXReinterpretCastExprClass:
   case Stmt::CXXStaticCastExprClass:
   case Stmt::ImplicitCastExprClass: {
     const auto *CE = cast<CastExpr>(E);
default: {
 
     ASTContext &Ctx = getContext();
     Expr::EvalResult Result;
     if (E->EvaluateAsInt(Result, Ctx))
       return makeIntVal(Result.Val.getInt());
 
     if (Loc::isLocType(E->getType()))
       if (E->isNullPointerConstant(Ctx, Expr::NPC_ValueDependentIsNotNull))
-        return makeNull();
+        return makeNullWithType(E->getType());
 
     return None;
   }
   }
 }
 
 SVal SValBuilder::makeSymExprValNN(BinaryOperator::Opcode Op,
                                    NonLoc LHS, NonLoc RHS,

clang/test/Analysis/cast-value-notes.cpp

 // 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>
   const T *castAs() const;
 
   template <typename T>
   const T *getAs() const;
 };
 class Triangle : public Shape {};
 class Rectangle : public Shape {};
 class Hexagon : public Shape {};
 class Circle : public Shape {};
 } // namespace clang
 
 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);
   // expected-note@-1 {{'C' initialized here}}
 
   if (!dyn_cast_or_null<Circle>(C)) {
     // expected-note@-1 {{'C' is a 'Circle'}}
     // expected-note@-2 {{Taking false branch}}