[Analyzer] Store the pointed/referenced type for dynamic casts

[baloghadamsoftware]

Summary

The successfulness of a dynamic cast depends only on the C++ class, not the pointer or reference. Thus if *A is a *B, then &A is a &B, const *A is a const *B etc. This patch changes DynamicCastInfo to store and check the cast between the unqualified pointed/referenced types. It also removes e.g. SubstTemplateTypeParmType from both the pointer and the pointed type.

[NoQ]

Yes, i completely agree that this is the right decision, thanks!

clang/lib/StaticAnalyzer/Core/DynamicType.cpp

71–72

Just canonicalize instead. The result doesn't depend on sugar either.

[xazax.hun]

clang/lib/StaticAnalyzer/Core/DynamicType.cpp

73

Is this doing what you intended? What about a reference to a pointer? Wouldn't you do too much unboxing?

Also, I think a function returning a value would be more conventional.

Other sugars like typedefs cannot interfere? I think this patch might benefit from additional test coverage. I also see no tests for template substitutions.

[whisperity]

@baloghadamsoftware Maybe there is a typo in the summary of the patch

then &B is a &B

Shouldn't this be "&A is a &B"?

[baloghadamsoftware]

Updated according to the comments.

[baloghadamsoftware]

clang/lib/StaticAnalyzer/Core/DynamicType.cpp

73

Reference to pointer cast using LLVM's cast functions are syntactically invalid, they do not compile.

For QualType in-place modification is usual, since we use it by value.

I see no test coverage for this particular part of the analyzer specifically, it seems that its is only tested indirectly in the tests for CastValueChecker.

[NoQ]

clang/lib/StaticAnalyzer/Core/DynamicType.cpp

73

The usual idiom is

if (Ty->isPointerType() || Ty->isReferenceType())
  Ty = Ty->getPointeeType();

It makes sure that you don't unwrap things twice. Also there's no need to canonicalize before unwrapping the pointee type; all these methods automatically operate over the canonical type.

You might want to add tests for typedefs.

[NoQ]

clang/lib/StaticAnalyzer/Core/DynamicType.cpp

73

Oof, this function is actually being fed references-to-pointers. I'll add a fixme.

clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp

static bool isInfeasibleCast(const DynamicCastInfo *CastInfo,
   return CastSucceeds ? CastInfo->fails() : CastInfo->succeeds();
 }
 
 static const NoteTag *getNoteTag(CheckerContext &C,
                                  const DynamicCastInfo *CastInfo,
                                  QualType CastToTy, const Expr *Object,
                                  bool CastSucceeds, bool IsKnownCast) {
   std::string CastToName =
-      CastInfo ? CastInfo->to()->getPointeeCXXRecordDecl()->getNameAsString()
+      CastInfo ? CastInfo->to()->getAsCXXRecordDecl()->getNameAsString()
                : CastToTy->getPointeeCXXRecordDecl()->getNameAsString();
   Object = Object->IgnoreParenImpCasts();
 
   return C.getNoteTag(
       [=]() -> std::string {
         SmallString<128> Msg;
         llvm::raw_svector_ostream Out(Msg);
 

clang/lib/StaticAnalyzer/Core/DynamicType.cpp

DynamicTypeInfo getDynamicTypeInfo(ProgramStateRef State, const MemRegion *MR) {
   return {};
 }
 
 const DynamicTypeInfo *getRawDynamicTypeInfo(ProgramStateRef State,
                                              const MemRegion *MR) {
   return State->get<DynamicTypeMap>(MR);
 }
 
+static void unbox(QualType &Ty) {
+  if (Ty->isReferenceType())
+    Ty = Ty.getNonReferenceType();
+  if (const auto *STTPTy = dyn_cast<SubstTemplateTypeParmType>(Ty.getTypePtr()))
+    Ty = STTPTy->getReplacementType();
+  if (Ty->isPointerType())
+    Ty = Ty->getPointeeType();
+  if (const auto *STTPTy =
+      dyn_cast<SubstTemplateTypeParmType>(Ty.getTypePtr()))
+    Ty = STTPTy->getReplacementType();
+  Ty = Ty.getUnqualifiedType();
+}
+
 const DynamicCastInfo *getDynamicCastInfo(ProgramStateRef State,
                                           const MemRegion *MR,
                                           QualType CastFromTy,
                                           QualType CastToTy) {
   const auto *Lookup = State->get<DynamicCastMap>().lookup(MR);
   if (!Lookup)
     return nullptr;
 
+  unbox(CastFromTy);
+  unbox(CastToTy);
+
   for (const DynamicCastInfo &Cast : *Lookup)
     if (Cast.equals(CastFromTy, CastToTy))
       return &Cast;
 
   return nullptr;
 }
 
 DynamicTypeInfo getClassObjectDynamicTypeInfo(ProgramStateRef State,
if (!MR)
     return State;
 
   if (CastSucceeds) {
     assert((CastToTy->isAnyPointerType() || CastToTy->isReferenceType()) &&
            "DynamicTypeInfo should always be a pointer.");
     State = State->set<DynamicTypeMap>(MR, CastToTy);
   }
 
+  unbox(CastFromTy);
+  unbox(CastToTy);
+
   DynamicCastInfo::CastResult ResultKind =
       CastSucceeds ? DynamicCastInfo::CastResult::Success
                    : DynamicCastInfo::CastResult::Failure;
 
   CastSet::Factory &F = State->get_context<CastSet>();
 
   const CastSet *TempSet = State->get<DynamicCastMap>(MR);
   CastSet Set = TempSet ? *TempSet : F.getEmptySet();

clang/test/Analysis/cast-value-state-dump.cpp

void evalNonNullParamNonNullReturn(const Shape *S) {
 
   clang_analyzer_printState();
 
   // CHECK:      "dynamic_types": [
   // CHECK-NEXT:   { "region": "SymRegion{reg_$0<const struct clang::Shape * S>}", "dyn_type": "const class clang::Circle", "sub_classable": true }
   // CHECK-NEXT: ],
   // CHECK-NEXT: "dynamic_casts": [
   // CHECK:        { "region": "SymRegion{reg_$0<const struct clang::Shape * S>}", "casts": [
-  // CHECK-NEXT:     { "from": "const struct clang::Shape *", "to": "const class clang::Circle *", "kind": "success" },
-  // CHECK-NEXT:     { "from": "const struct clang::Shape *", "to": "const class clang::Square *", "kind": "fail" }
+  // CHECK-NEXT:     { "from": "struct clang::Shape", "to": "class clang::Circle", "kind": "success" },
+  // CHECK-NEXT:     { "from": "struct clang::Shape", "to": "class clang::Square", "kind": "fail" }
   // CHECK-NEXT:   ] }
 
   (void)(1 / !C);
   // expected-note@-1 {{'C' is non-null}}
   // expected-note@-2 {{Division by zero}}
   // expected-warning@-3 {{Division by zero}}
 }