[PATCH] D66593: [analyzer] CastValueChecker: Fix some assertions

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:743-750
+  QualType RegionType = DynType.getType();
+  if (RegionType->isPointerType())
+    RegionType = RegionType->getPointeeType();
+  else
+    RegionType = RegionType.getNonReferenceType();
+
+  assert(!RegionType.isNull() &&
----------------
NoQ wrote:
> I don't think this does anything:
> ```lang=c++
>    505 QualType Type::getPointeeType() const {
>    506   if (const auto *PT = getAs<PointerType>())
>    507     return PT->getPointeeType();
>    508   if (const auto *OPT = getAs<ObjCObjectPointerType>())
>    509     return OPT->getPointeeType();
>    510   if (const auto *BPT = getAs<BlockPointerType>())
>    511     return BPT->getPointeeType();
>    512   if (const auto *RT = getAs<ReferenceType>())
>    513     return RT->getPointeeType();
>    514   if (const auto *MPT = getAs<MemberPointerType>())
>    515     return MPT->getPointeeType();
>    516   if (const auto *DT = getAs<DecayedType>())
>    517     return DT->getPointeeType();
>    518   return {};
>    519 }
> ```
> This getter usually works very reliably for both pointers and references.
I have measured each assertion failure one-by-one, so all of the hotfixes are necessary in order to reduce the crash-counter to zero. I have not got time to go in-depth, and have a great talk about them, sorry.


================
Comment at: clang/lib/StaticAnalyzer/Core/DynamicType.cpp:118-122
+  for (const auto &Elem : Map) {
+    const MemRegion *MR = Elem.first;
+    if (MR && !SR.isLiveRegion(MR))
+      State = State->remove<DynamicCastMap>(MR);
+  }
----------------
NoQ wrote:
> We shouldn't put null regions into our maps.
Yes, but that is problematic. I have updated the diff.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66593/new/

https://reviews.llvm.org/D66593