[analyzer] Wrong type cast occures during pointer dereferencing after type punning

[ASDenysPetrov]

Summary

During pointer dereferencing CastRetrievedVal uses wrong type from the Store after type punning. Namely, the pointer casts to another type and then assigns with a value of one more another type. It produces NonLoc value when Loc is expected.

Example for visibility:

void foo(char ***c, int *i) {
  *(unsigned**)c = (unsigned*)i; // type punning
  ***c; // uses 'unsigned**' from the Store instead of 'char***'
}

Fixes:
https://bugs.llvm.org/show_bug.cgi?id=37503
https://bugs.llvm.org/show_bug.cgi?id=49007

[NoQ]

This actually looks correct to me but i suggest a bit more investigation. Architecturally, it doesn't make sense to me that CastRetrievedVal() does anything beyond svalBuilder.dispatchCast(V, castTy). After all, there's only one way to cast a value from one type to another, and dispatchCast() was supposed to be the ultimate method to do so.

Can we collapse this function further towards this goal? Say, why not pump every region unconditionally through castRegion()? Does dispatchCast() use castRegion() internally - and if it does, why are there so many branches in this function?

[ASDenysPetrov]

@NoQ

Can we collapse this function further towards this goal? Say, why not pump every region unconditionally through castRegion()? Does dispatchCast() use castRegion() internally - and if it does, why are there so many branches in this function?

Thank you for raising such a good questions :)
Honestly, I spent a week debugging to undersand what's going on in the Store, I never had a chance to dig into it before. It was tough. I'll proceed this to try to make what you mentioned.

The thing I'm sure that the type of retrieved value should be the same as the type of which this value was declared? In other words we can unconditionally cast all the values, can't we? Is there any case that contradicts me?

[ASDenysPetrov]

@NoQ
BTW, what do you think we should do with D77062 and D88477 then?

[steakhal]

If you don't mind I would also request some credit in the summary since I've spent some time on this as well.
Besides that I don't have much objection about this patch. It's defenately not my expertiese.
Good job coming up with the fix though, I had to focus on other things.

clang/test/Analysis/string.c

367–373

I would prefer slightly more readbale names.
func_strcpy_no_assertion() -> get_void_ptr()
ptr_strcpy_no_assertion -> type_punned_ptr
no-assertion -> no-crash
You could also hoist the char a as a function parameter to spare a line :)

[ASDenysPetrov]

Updat patch due to suggestions and fixed formating.

Proceeding work on improving solution by moving the logic inside SimpleSValBuilder::dispatchCast.

[ASDenysPetrov]

Updated. Removed a new test file, moved the test to an existing file instead.

[ASDenysPetrov]

@NoQ

Can we collapse this function further towards this goal? Say, why not pump every region unconditionally through castRegion()? Does dispatchCast() use castRegion() internally - and if it does, why are there so many branches in this function?

I've made some investigations. castRegion() does not use inside dispatchCast(), it belongs to StoreManager. Moving it to SimpleSValBuilder will be hard at least for now, since it is quite complex, but we can try. What do you think. Should I go further or we can apply this patch as is?

[ASDenysPetrov]

Updated. Moved castRegion() from StoreManager to SValBuilder.

Actually it wasn't so hard as I thought :)
Would it be better to make this moving in a separate patch as [NFC]?

[NoQ]

Aha, great, sounds like all casting can be made more correct, not just casting on store retrieve! Maybe it's worth celebrating through extra unittests on evalCast(). Thank you for looking into this.

The new code should obviously be restricted into evalCastFromLoc() because if it's a region it's a Loc.

You didn't need to move castRegion(): StoreManager is available in SValBuilder through this->StateMgr. I don't have a strong preference on where castRegion() should live; the original idea was that StoreManager should be allowed to have an opinion on this matter because different store managers would handle that differently; however, as of now there's only one store manager and there doesn't seem to be an interest in introducing more of them whereas the abstraction has already leaked all over the place anyway.

I still have questions about the extra if-statements that you've added. Shouldn't we do castRegion() unconditionally, given that we're trying to cast a region and that function presumably does exactly that? I just want to avoid these situations:

In D89055#2320363, @NoQ wrote:

dispatchCast() was supposed to be the ultimate method to do so

Ugh, sorry, no, that's evalCast(). Like evalBinOp() etc. My bad. Can we also use evalCast()?

[ASDenysPetrov]

@NoQ

The new code should obviously be restricted into evalCastFromLoc() because if it's a region it's a Loc.

The first I tryed was evalCastFromLoc(), but it turned out that SVal which binds to a pointer can be NonLoc as well through violation of pointing levels.
Look here:

void foo(int** p) { // here is a two-level pointer
   *(int*)p = 42; // pretend as a one-level pointer, dereference it and assign a number
   *p; // dereferencing once gives a `nonloc::ConcreteInt`, though it actually should be a one-level pointer aka Loc.
}

P.S. I can have miscomprehension of this, due to a lack of experience, but this is what I observed.

Shouldn't we do castRegion() unconditionally,

I've also tryed and got 10+ tests unpassed. Didn't dig deeper, just refused this idea.

Can we also use evalCast() ?

Do you mean to move castRegion() to evalCast() instead of dispatchCast(). I can investigate this.
Could you explain please what is a major difference between evalCast() and dispatchCast()?

[ASDenysPetrov]

In D89055#2336709, @NoQ wrote:

Ugh, sorry, no, that's evalCast(). Like evalBinOp() etc. My bad. Can we also use evalCast()?

I dived into evalCast(). Initially I had to figure it out and rework it to find the right place to put the fix in. You are welcome to review D90157 as a parent revision.

[ASDenysPetrov]

@NoQ

Ugh, sorry, no, that's evalCast(). Like evalBinOp() etc. My bad. Can we also use evalCast()?

Finally :-) Now we can use evalCast() instead of CastRetrievedVal(). It would be nice if you could look at D96090.

I made a stack of four patches to resolve bugs of this revision.

[ASDenysPetrov]

Updated. Rolled the fix over the evalCast refactoring.

[steakhal]

I'm looking forward to this patch.

clang/test/Analysis/string.c

367–373

I think this is 'done'. :)

[ASDenysPetrov]

@NoQ, @steakhal, @vsavchenko
I think we have met all the conditions with previous patchs to make this patch acceptable.
If you think it's OK, could you, please, approve it?

[NoQ]

This looks like the fix is in the right place and it looks more or less how I expected it to look. Our casting procedure hopefully became more correct and now we have more correct analysis everywhere. It's still hard to tell whether this ElementRegion should be there or not, given how nobody still knows why they're there and when exactly should they be there; the very representation of casted pointers still needs a major rework. But we get there when we get there. Thanks a lot for debugging and digging and refactoring along the way.

[bjope]

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

747

This caused some problems with assertion failures, see https://bugs.llvm.org/show_bug.cgi?id=50179

[ASDenysPetrov]

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

747

Yes, you are right. Thanks for the catch. The fix has already been loaded. D101635

[chrish_ericsson_atx]

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

745–747

This causes new false-positive static analysis warnings from core.CallAndMessage and core.UndefinedBinaryOperatorResult, (and possibly others?), possibly because of loss of array extent information. See https://bugs.llvm.org/show_bug.cgi?id=50604.

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

protected:
 
   virtual SVal evalCastFromNonLoc(NonLoc val, QualType castTy) = 0;
   virtual SVal evalCastFromLoc(Loc val, QualType castTy) = 0;
 
 public:
   // FIXME: Make these protected again once RegionStoreManager correctly
   // handles loads from different bound value types.
   virtual SVal dispatchCast(SVal val, QualType castTy) = 0;
+  const ElementRegion *MakeElementRegion(const SubRegion *Base, QualType EleTy,
+                                         uint64_t index = 0);
+  /// 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 *R, QualType CastToTy);
 
 public:
   SValBuilder(llvm::BumpPtrAllocator &alloc, ASTContext &context,
               ProgramStateManager &stateMgr)
       : Context(context), BasicVals(context, alloc),
         SymMgr(context, BasicVals, alloc), MemMgr(context, alloc),
         StateMgr(stateMgr), ArrayIndexTy(context.LongLongTy),
         ArrayIndexWidth(context.getTypeSize(ArrayIndexTy)) {}

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

public:
   ///  - We don't know (base is a symbolic region and we don't have
   ///    enough info to determine if the cast will succeed at run time).
   /// The function returns an SVal representing the derived class; it's
   /// valid only if Failed flag is set to false.
   SVal attemptDownCast(SVal Base, QualType DerivedPtrType, bool &Failed);
 
   const ElementRegion *GetElementZeroRegion(const SubRegion *R, QualType T);
 
-  /// 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);
-
   virtual StoreRef removeDeadBindings(Store store, const StackFrameContext *LCtx,
                                       SymbolReaper &SymReaper) = 0;
 
   virtual bool includedInBindings(Store store,
                                   const MemRegion *region) const = 0;
 
   /// If the StoreManager supports it, increment the reference count of
   /// the specified Store object.
bool HandleBinding(StoreManager& SMgr, Store store, const MemRegion* R,
                        SVal val) override;
     const MemRegion *getRegion() { return Binding; }
   };
 
   /// iterBindings - Iterate over the bindings in the Store.
   virtual void iterBindings(Store store, BindingsHandler& f) = 0;
 
 protected:
-  const ElementRegion *MakeElementRegion(const SubRegion *baseRegion,
-                                         QualType pointeeTy,
-                                         uint64_t index = 0);
 
   /// CastRetrievedVal - Used by subclasses of StoreManager to implement
   ///  implicit casts that arise from loads from regions that are reinterpreted
   ///  as another region.
   SVal CastRetrievedVal(SVal val, const TypedValueRegion *region,
                         QualType castTy);
 
 private:

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

SVal SValBuilder::evalIntegralCast(ProgramStateRef state, SVal val,
   if (!IsNotTruncated && IsTruncated) {
     // Symbol is truncated so we evaluate it as a cast.
     NonLoc CastVal = makeNonLoc(se, originalTy, castTy);
     return CastVal;
   }
   return evalCast(val, castTy, originalTy);
 }
 
+const ElementRegion *SValBuilder::MakeElementRegion(const SubRegion *Base,
+                                                    QualType EleTy,
+                                                    uint64_t index /*= 0*/) {
+  NonLoc idx = makeArrayIndex(index);
+  return MemMgr.getElementRegion(EleTy, idx, Base, getContext());
+}
+
+const MemRegion *SValBuilder::castRegion(const MemRegion *R,
+                                         QualType CastToTy) {
+  ASTContext &Ctx = getContext();
+
+  // Handle casts to Objective-C objects.
+  if (CastToTy->isObjCObjectPointerType())
+    return R->StripCasts();
+
+  if (CastToTy->isBlockPointerType()) {
+    // FIXME: We may need different solutions, depending on the symbol
+    // involved.  Blocks can be casted to/from 'id', as they can be treated
+    // as Objective-C objects.  This could possibly be handled by enhancing
+    // our reasoning of downcasts of symbolic objects.
+    if (isa<CodeTextRegion>(R) || isa<SymbolicRegion>(R))
+      return R;
+
+    // We don't know what to make of it.  Return a NULL region, which
+    // will be interpreted as UnknownVal.
+    return nullptr;
+  }
+
+  // Now assume we are casting from pointer to pointer. Other cases should
+  // already be handled.
+  QualType PointeeTy = CastToTy->getPointeeType();
+  QualType CanonPointeeTy = Ctx.getCanonicalType(PointeeTy);
+
+  // Handle casts to void*.  We just pass the region through.
+  if (CanonPointeeTy.getLocalUnqualifiedType() == Ctx.VoidTy)
+    return R;
+
+  // Handle casts from compatible types.
+  if (R->isBoundable())
+    if (const auto *TR = dyn_cast<TypedValueRegion>(R)) {
+      QualType ObjTy = Ctx.getCanonicalType(TR->getValueType());
+      if (CanonPointeeTy == ObjTy)
+        return R;
+    }
+
+  // Process region cast according to the kind of the region being cast.
+  switch (R->getKind()) {
+  case MemRegion::CXXThisRegionKind:
+  case MemRegion::CodeSpaceRegionKind:
+  case MemRegion::StackLocalsSpaceRegionKind:
+  case MemRegion::StackArgumentsSpaceRegionKind:
+  case MemRegion::HeapSpaceRegionKind:
+  case MemRegion::UnknownSpaceRegionKind:
+  case MemRegion::StaticGlobalSpaceRegionKind:
+  case MemRegion::GlobalInternalSpaceRegionKind:
+  case MemRegion::GlobalSystemSpaceRegionKind:
+  case MemRegion::GlobalImmutableSpaceRegionKind: {
+    llvm_unreachable("Invalid region cast");
+  }
+
+  case MemRegion::FunctionCodeRegionKind:
+  case MemRegion::BlockCodeRegionKind:
+  case MemRegion::BlockDataRegionKind:
+  case MemRegion::StringRegionKind:
+    // FIXME: Need to handle arbitrary downcasts.
+  case MemRegion::SymbolicRegionKind:
+  case MemRegion::AllocaRegionKind:
+  case MemRegion::CompoundLiteralRegionKind:
+  case MemRegion::FieldRegionKind:
+  case MemRegion::ObjCIvarRegionKind:
+  case MemRegion::ObjCStringRegionKind:
+  case MemRegion::NonParamVarRegionKind:
+  case MemRegion::ParamVarRegionKind:
+  case MemRegion::CXXTempObjectRegionKind:
+  case MemRegion::CXXBaseObjectRegionKind:
+  case MemRegion::CXXDerivedObjectRegionKind:
+    return MakeElementRegion(cast<SubRegion>(R), PointeeTy);
+
+  case MemRegion::ElementRegionKind: {
+    // If we are casting from an ElementRegion to another type, the
+    // algorithm is as follows:
+    //
+    // (1) Compute the "raw offset" of the ElementRegion from the
+    //     base region.  This is done by calling 'getAsRawOffset()'.
+    //
+    // (2a) If we get a 'RegionRawOffset' after calling
+    //      'getAsRawOffset()', determine if the absolute offset
+    //      can be exactly divided into chunks of the size of the
+    //      casted-pointee type.  If so, create a new ElementRegion with
+    //      the pointee-cast type as the new ElementType and the index
+    //      being the offset divded by the chunk size.  If not, create
+    //      a new ElementRegion at offset 0 off the raw offset region.
+    //
+    // (2b) If we don't a get a 'RegionRawOffset' after calling
+    //      'getAsRawOffset()', it means that we are at offset 0.
+    //
+    // FIXME: Handle symbolic raw offsets.
+
+    const ElementRegion *elementR = cast<ElementRegion>(R);
+    const RegionRawOffset &rawOff = elementR->getAsArrayOffset();
+    const MemRegion *baseR = rawOff.getRegion();
+
+    // If we cannot compute a raw offset, throw up our hands and return
+    // a NULL MemRegion*.
+    if (!baseR)
+      return nullptr;
+
+    CharUnits off = rawOff.getOffset();
+
+    if (off.isZero()) {
+      // Edge case: we are at 0 bytes off the beginning of baseR.  We
+      // check to see if type we are casting to is the same as the base
+      // region.  If so, just return the base region.
+      if (const auto *TR = dyn_cast<TypedValueRegion>(baseR)) {
+        QualType ObjTy = Ctx.getCanonicalType(TR->getValueType());
+        QualType CanonPointeeTy = Ctx.getCanonicalType(PointeeTy);
+        if (CanonPointeeTy == ObjTy)
+          return baseR;
+      }
+
+      // Otherwise, create a new ElementRegion at offset 0.
+      return MakeElementRegion(cast<SubRegion>(baseR), PointeeTy);
+    }
+
+    // We have a non-zero offset from the base region.  We want to determine
+    // if the offset can be evenly divided by sizeof(PointeeTy).  If so,
+    // we create an ElementRegion whose index is that value.  Otherwise, we
+    // create two ElementRegions, one that reflects a raw offset and the other
+    // that reflects the cast.
+
+    // Compute the index for the new ElementRegion.
+    int64_t newIndex = 0;
+    const MemRegion *newSuperR = nullptr;
+
+    // We can only compute sizeof(PointeeTy) if it is a complete type.
+    if (!PointeeTy->isIncompleteType()) {
+      // Compute the size in **bytes**.
+      CharUnits pointeeTySize = Ctx.getTypeSizeInChars(PointeeTy);
+      if (!pointeeTySize.isZero()) {
+        // Is the offset a multiple of the size?  If so, we can layer the
+        // ElementRegion (with elementType == PointeeTy) directly on top of
+        // the base region.
+        if (off % pointeeTySize == 0) {
+          newIndex = off / pointeeTySize;
+          newSuperR = baseR;
+        }
+      }
+    }
+
+    if (!newSuperR) {
+      // Create an intermediate ElementRegion to represent the raw byte.
+      // This will be the super region of the final ElementRegion.
+      newSuperR = MakeElementRegion(cast<SubRegion>(baseR), Ctx.CharTy,
+                                    off.getQuantity());
+    }
+
+    return MakeElementRegion(cast<SubRegion>(newSuperR), PointeeTy, newIndex);
+  }
+  }
+
+  llvm_unreachable("unreachable");
+}
+
 // FIXME: should rewrite according to the cast kind.
 SVal SValBuilder::evalCast(SVal val, QualType castTy, QualType originalTy) {
   castTy = Context.getCanonicalType(castTy);
   originalTy = Context.getCanonicalType(originalTy);
   if (val.isUnknownOrUndef() || castTy == originalTy)
     return val;
 
   if (castTy->isBooleanType()) {
SVal SValBuilder::evalCast(SVal val, QualType castTy, QualType originalTy) {
   // Check for casts from pointers to integers.
   if (castTy->isIntegralOrEnumerationType() && Loc::isLocType(originalTy))
     return evalCastFromLoc(val.castAs<Loc>(), castTy);
 
   // Check for casts from integers to pointers.
   if (Loc::isLocType(castTy) && originalTy->isIntegralOrEnumerationType()) {
     if (Optional<nonloc::LocAsInteger> LV = val.getAs<nonloc::LocAsInteger>()) {
       if (const MemRegion *R = LV->getLoc().getAsRegion()) {
-        StoreManager &storeMgr = StateMgr.getStoreManager();
-        R = storeMgr.castRegion(R, castTy);
+        R = castRegion(R, castTy);
         return R ? SVal(loc::MemRegionVal(R)) : UnknownVal();
       }
       return LV->getLoc();
     }
     return dispatchCast(val, castTy);
   }
 
   // Just pass through function and block pointers.
   if (originalTy->isBlockPointerType() || originalTy->isFunctionPointerType()) {
     assert(Loc::isLocType(castTy));
     return val;
   }
 
   // Check for casts from array type to another type.
   if (const auto *arrayT =
if (const MemRegion *R = val.getAsRegion()) {
     //    int x;
     //    (*foo->my_func)(&x);
     //    return bar(x)+1; // no-warning
     //  }
 
     assert(Loc::isLocType(originalTy) || originalTy->isFunctionType() ||
            originalTy->isBlockPointerType() || castTy->isReferenceType());
 
-    StoreManager &storeMgr = StateMgr.getStoreManager();
-
-    // 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.
-    R = storeMgr.castRegion(R, castTy);
+    // Get the result of casting a region to a different type.
+    // If the MemRegion* returned is NULL, this expression evaluates to
+    // UnknownVal.
+    R = castRegion(R, castTy);
     return R ? SVal(loc::MemRegionVal(R)) : UnknownVal();
   }
 
   return dispatchCast(val, castTy);
 }

clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

SValBuilder *ento::createSimpleSValBuilder(llvm::BumpPtrAllocator &alloc,
                                            ProgramStateManager &stateMgr) {
   return new SimpleSValBuilder(alloc, context, stateMgr);
 }
 
 //===----------------------------------------------------------------------===//
 // Transfer function for Casts.
 //===----------------------------------------------------------------------===//
 
+static bool hasSameUnqualifiedPointeeType(QualType ty1, QualType ty2) {
+  return ty1->getPointeeType().getCanonicalType().getTypePtr() ==
+         ty2->getPointeeType().getCanonicalType().getTypePtr();
+}
+
 SVal SimpleSValBuilder::dispatchCast(SVal Val, QualType CastTy) {
   assert(Val.getAs<Loc>() || Val.getAs<NonLoc>());
+
+  // When retrieving symbolic pointer and expecting a non-void pointer,
+  // wrap them into element regions of the expected type if necessary.
+  // it is necessary to make sure that the retrieved value makes sense,
+  // because there's no other cast in the AST that would tell us to cast
+  // it to the correct pointer type. We might need to do that for non-void
+  // pointers as well.
+  // FIXME: We really need a single good function to perform casts for us
+  // correctly every time we need it.
+  if (CastTy->isPointerType() && !CastTy->isVoidPointerType()) {
+    if (const auto *SR = dyn_cast_or_null<SymbolicRegion>(Val.getAsRegion())) {
+      QualType sr = SR->getSymbol()->getType();
+      if (!hasSameUnqualifiedPointeeType(sr, CastTy))
+        return loc::MemRegionVal(castRegion(SR, CastTy));
+    }
+    // Next fixes pointer dereference using type different from its initial
+    // one. See PR37503 for details.
+    if (const auto *ER = dyn_cast_or_null<ElementRegion>(Val.getAsRegion()))
+      return loc::MemRegionVal(castRegion(ER, CastTy));
+  }
+
   return Val.getAs<Loc>() ? evalCastFromLoc(Val.castAs<Loc>(), CastTy)
                            : evalCastFromNonLoc(Val.castAs<NonLoc>(), CastTy);
 }
 
 SVal SimpleSValBuilder::evalCastFromNonLoc(NonLoc val, QualType castTy) {
   bool isLocType = Loc::isLocType(castTy);
   if (val.getAs<nonloc::PointerToMember>())
     return val;

 SVal SimpleSValBuilder::evalCastFromLoc(Loc val, QualType castTy) {
 
   // Casts from pointers -> pointers, just return the lval.
   //
   // Casts from pointers -> references, just return the lval.  These
   //   can be introduced by the frontend for corner cases, e.g
   //   casting from va_list* to __builtin_va_list&.
   //
-  if (Loc::isLocType(castTy) || castTy->isReferenceType())
+  if (Loc::isLocType(castTy))
     return val;
 
   // FIXME: Handle transparent unions where a value can be "transparently"
   //  lifted into a union type.
   if (castTy->isUnionType())
     return UnknownVal();
 
   // Casting a Loc to a bool will almost always be true,

clang/lib/StaticAnalyzer/Core/Store.cpp

StoreRef StoreManager::enterStackFrame(Store OldStore,
   Call.getInitialStackFrameContents(LCtx, InitialBindings);
 
   for (const auto &I : InitialBindings)
     Store = Bind(Store.getStore(), I.first.castAs<Loc>(), I.second);
 
   return Store;
 }
 
-const ElementRegion *StoreManager::MakeElementRegion(const SubRegion *Base,
-                                                     QualType EleTy,
-                                                     uint64_t index) {
-  NonLoc idx = svalBuilder.makeArrayIndex(index);
-  return MRMgr.getElementRegion(EleTy, idx, Base, svalBuilder.getContext());
-}
-
 const ElementRegion *StoreManager::GetElementZeroRegion(const SubRegion *R,
                                                         QualType T) {
   NonLoc idx = svalBuilder.makeZeroArrayIndex();
   assert(!T.isNull());
   return MRMgr.getElementRegion(T, idx, R, Ctx);
 }
 
-const MemRegion *StoreManager::castRegion(const MemRegion *R, QualType CastToTy) {
-  ASTContext &Ctx = StateMgr.getContext();
-
-  // Handle casts to Objective-C objects.
-  if (CastToTy->isObjCObjectPointerType())
-    return R->StripCasts();
-
-  if (CastToTy->isBlockPointerType()) {
-    // FIXME: We may need different solutions, depending on the symbol
-    // involved.  Blocks can be casted to/from 'id', as they can be treated
-    // as Objective-C objects.  This could possibly be handled by enhancing
-    // our reasoning of downcasts of symbolic objects.
-    if (isa<CodeTextRegion>(R) || isa<SymbolicRegion>(R))
-      return R;
-
-    // We don't know what to make of it.  Return a NULL region, which
-    // will be interpreted as UnknownVal.
-    return nullptr;
-  }
-
-  // Now assume we are casting from pointer to pointer. Other cases should
-  // already be handled.
-  QualType PointeeTy = CastToTy->getPointeeType();
-  QualType CanonPointeeTy = Ctx.getCanonicalType(PointeeTy);
-
-  // Handle casts to void*.  We just pass the region through.
-  if (CanonPointeeTy.getLocalUnqualifiedType() == Ctx.VoidTy)
-    return R;
-
-  // Handle casts from compatible types.
-  if (R->isBoundable())
-    if (const auto *TR = dyn_cast<TypedValueRegion>(R)) {
-      QualType ObjTy = Ctx.getCanonicalType(TR->getValueType());
-      if (CanonPointeeTy == ObjTy)
-        return R;
-    }
-
-  // Process region cast according to the kind of the region being cast.
-  switch (R->getKind()) {
-    case MemRegion::CXXThisRegionKind:
-    case MemRegion::CodeSpaceRegionKind:
-    case MemRegion::StackLocalsSpaceRegionKind:
-    case MemRegion::StackArgumentsSpaceRegionKind:
-    case MemRegion::HeapSpaceRegionKind:
-    case MemRegion::UnknownSpaceRegionKind:
-    case MemRegion::StaticGlobalSpaceRegionKind:
-    case MemRegion::GlobalInternalSpaceRegionKind:
-    case MemRegion::GlobalSystemSpaceRegionKind:
-    case MemRegion::GlobalImmutableSpaceRegionKind: {
-      llvm_unreachable("Invalid region cast");
-    }
-
-    case MemRegion::FunctionCodeRegionKind:
-    case MemRegion::BlockCodeRegionKind:
-    case MemRegion::BlockDataRegionKind:
-    case MemRegion::StringRegionKind:
-      // FIXME: Need to handle arbitrary downcasts.
-    case MemRegion::SymbolicRegionKind:
-    case MemRegion::AllocaRegionKind:
-    case MemRegion::CompoundLiteralRegionKind:
-    case MemRegion::FieldRegionKind:
-    case MemRegion::ObjCIvarRegionKind:
-    case MemRegion::ObjCStringRegionKind:
-    case MemRegion::NonParamVarRegionKind:
-    case MemRegion::ParamVarRegionKind:
-    case MemRegion::CXXTempObjectRegionKind:
-    case MemRegion::CXXBaseObjectRegionKind:
-    case MemRegion::CXXDerivedObjectRegionKind:
-      return MakeElementRegion(cast<SubRegion>(R), PointeeTy);
-
-    case MemRegion::ElementRegionKind: {
-      // If we are casting from an ElementRegion to another type, the
-      // algorithm is as follows:
-      //
-      // (1) Compute the "raw offset" of the ElementRegion from the
-      //     base region.  This is done by calling 'getAsRawOffset()'.
-      //
-      // (2a) If we get a 'RegionRawOffset' after calling
-      //      'getAsRawOffset()', determine if the absolute offset
-      //      can be exactly divided into chunks of the size of the
-      //      casted-pointee type.  If so, create a new ElementRegion with
-      //      the pointee-cast type as the new ElementType and the index
-      //      being the offset divded by the chunk size.  If not, create
-      //      a new ElementRegion at offset 0 off the raw offset region.
-      //
-      // (2b) If we don't a get a 'RegionRawOffset' after calling
-      //      'getAsRawOffset()', it means that we are at offset 0.
-      //
-      // FIXME: Handle symbolic raw offsets.
-
-      const ElementRegion *elementR = cast<ElementRegion>(R);
-      const RegionRawOffset &rawOff = elementR->getAsArrayOffset();
-      const MemRegion *baseR = rawOff.getRegion();
-
-      // If we cannot compute a raw offset, throw up our hands and return
-      // a NULL MemRegion*.
-      if (!baseR)
-        return nullptr;
-
-      CharUnits off = rawOff.getOffset();
-
-      if (off.isZero()) {
-        // Edge case: we are at 0 bytes off the beginning of baseR.  We
-        // check to see if type we are casting to is the same as the base
-        // region.  If so, just return the base region.
-        if (const auto *TR = dyn_cast<TypedValueRegion>(baseR)) {
-          QualType ObjTy = Ctx.getCanonicalType(TR->getValueType());
-          QualType CanonPointeeTy = Ctx.getCanonicalType(PointeeTy);
-          if (CanonPointeeTy == ObjTy)
-            return baseR;
-        }
-
-        // Otherwise, create a new ElementRegion at offset 0.
-        return MakeElementRegion(cast<SubRegion>(baseR), PointeeTy);
-      }
-
-      // We have a non-zero offset from the base region.  We want to determine
-      // if the offset can be evenly divided by sizeof(PointeeTy).  If so,
-      // we create an ElementRegion whose index is that value.  Otherwise, we
-      // create two ElementRegions, one that reflects a raw offset and the other
-      // that reflects the cast.
-
-      // Compute the index for the new ElementRegion.
-      int64_t newIndex = 0;
-      const MemRegion *newSuperR = nullptr;
-
-      // We can only compute sizeof(PointeeTy) if it is a complete type.
-      if (!PointeeTy->isIncompleteType()) {
-        // Compute the size in **bytes**.
-        CharUnits pointeeTySize = Ctx.getTypeSizeInChars(PointeeTy);
-        if (!pointeeTySize.isZero()) {
-          // Is the offset a multiple of the size?  If so, we can layer the
-          // ElementRegion (with elementType == PointeeTy) directly on top of
-          // the base region.
-          if (off % pointeeTySize == 0) {
-            newIndex = off / pointeeTySize;
-            newSuperR = baseR;
-          }
-        }
-      }
-
-      if (!newSuperR) {
-        // Create an intermediate ElementRegion to represent the raw byte.
-        // This will be the super region of the final ElementRegion.
-        newSuperR = MakeElementRegion(cast<SubRegion>(baseR), Ctx.CharTy,
-                                      off.getQuantity());
-      }
-
-      return MakeElementRegion(cast<SubRegion>(newSuperR), PointeeTy, newIndex);
-    }
-  }
-
-  llvm_unreachable("unreachable");
-}
-
 static bool regionMatchesCXXRecordType(SVal V, QualType Ty) {
   const MemRegion *MR = V.getAsRegion();
   if (!MR)
     return true;
 
   const auto *TVR = dyn_cast<TypedValueRegion>(MR);
   if (!TVR)
     return true;
SVal StoreManager::CastRetrievedVal(SVal V, const TypedValueRegion *R,
   // from ints to floats.
   // TODO: What other combinations of types are affected?
   if (castTy->isFloatingType()) {
     SymbolRef Sym = V.getAsSymbol();
     if (Sym && !Sym->getType()->isFloatingType())
       return UnknownVal();
   }
 
-  // When retrieving symbolic pointer and expecting a non-void pointer,
-  // wrap them into element regions of the expected type if necessary.
-  // SValBuilder::dispatchCast() doesn't do that, but it is necessary to
-  // make sure that the retrieved value makes sense, because there's no other
-  // cast in the AST that would tell us to cast it to the correct pointer type.
-  // We might need to do that for non-void pointers as well.
-  // FIXME: We really need a single good function to perform casts for us
-  // correctly every time we need it.
-  if (castTy->isPointerType() && !castTy->isVoidPointerType())
-    if (const auto *SR = dyn_cast_or_null<SymbolicRegion>(V.getAsRegion())) {
-      QualType sr = SR->getSymbol()->getType();
-      if (!hasSameUnqualifiedPointeeType(sr, castTy))
-          return loc::MemRegionVal(castRegion(SR, castTy));
-    }
-
   return svalBuilder.dispatchCast(V, castTy);
 }
 
 SVal StoreManager::getLValueFieldOrIvar(const Decl *D, SVal Base) {
   if (Base.isUnknownOrUndef())
     return Base;
 
   Loc BaseL = Base.castAs<Loc>();

clang/test/Analysis/casts.c

double no_crash_reinterpret_double_as_sym_int(double a, int b) {
   return a * a;
 }
 
 double no_crash_reinterpret_double_as_sym_ptr(double a, void * b) {
   *(void **)&a = b;
   return a * a;
 }
 
+void no_crash_reinterpret_char_as_uchar(char ***a, int *b) {
+  *(unsigned char **)a = (unsigned char *)b;
+  if (**a == 0) // no-crash
+    ;
+}

clang/test/Analysis/string.c

 #endif
 
 void strcpy_no_overflow(char *y) {
   char x[4];
   if (strlen(y) == 3)
     strcpy(x, y); // no-warning
 }
 
+// PR37503
+void *get_void_ptr();
+char ***type_punned_ptr;
+void strcpy_no_assertion(char c) {
+  *(unsigned char **)type_punned_ptr = (unsigned char *)(get_void_ptr());
+  strcpy(**type_punned_ptr, &c); // no-crash
+}
+
 //===----------------------------------------------------------------------===
 // stpcpy()
 //===----------------------------------------------------------------------===
 
 #ifdef VARIANT
 
 #define __stpcpy_chk BUILTIN(__stpcpy_chk)
 char *__stpcpy_chk(char *restrict s1, const char *restrict s2, size_t destlen);