[analyzer] Fix null pointer deref in CastValueChecker

[vabridgers]

Summary

A crash was seen in CastValueChecker due to a null pointer dereference.

The fix uses QualType::getAsString to avoid the null dereference
when a CXXRecordDecl cannot be obtained. A small reproducer is added,
and cast value notes LITs are updated for the new debug messages.

[vabridgers]

I know this will need a reproducer, and I'm working on that. That work is still in progress.

[vabridgers]

add test case

[vabridgers]

I was able to find and add a covering test case. I understand the fix may not be "correct", but it does avoid the crash demonstrated by the test case.

[vabridgers]

Use QualType::getAsString() per suggestion from martong

[steakhal]

Likely related to https://github.com/llvm/llvm-project/issues/55715. Mention this in the summary as Fixes #55715.

clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp

168

So this was null, right? Which caused the crash.

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

306

It's good to know which line exactly caused the crash. Put this note right there.

309–310

311

This gotta be the getAs<T>. Please try to reconstruct the 'feel' of it; like return a T* instead of void etc.

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

26

IDK, I see the motivation, but we don't need the full name of these in most cases.
I find it more disturbing than helpful. I would instead stick to the shorter form.

[martong]

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

26

I see your point, but I this way, the checker provides more precise type info, which is good in my opinion.

[steakhal]

Vince, please update the summary - it looks really weird. Along with that the content of it might not be much useful, as we have a test case to demonstrate the crash; you can probably remove those dumps etc.
Otherwise LGTM.

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

26

Okay. It's not that important. This is a domain-specific check only for llvm codebases, so I'm not too picky about it.

[vabridgers]

clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp

168

Yes, the call to "CastToTy->getPointeeCXXRecordDecl()" returned nullptr, which was then used to dereference getNameAsString(), then boom :)

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

306

Will address, thank you

311

I'll attempt a further simplification. This was the product of a very long and tedious manual and creduce reduction process from a 12M preprocessed file :/

[steakhal]

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

311

What I'm proposing is concretization.

Something like this:

template <typename> struct PointerUnion {
  template <typename T> T* getAs() {
    (void)isa<int>(*this);
    return nullptr;
}

[vabridgers]

address @steakhal comments

[vabridgers]

I think all comments have been addressed, please let me know if I missed some detail :) Best!

[steakhal]

Submitted a backport request to land this in clang-14.0.5, since many llvm devs might be affected by this crash due to clangd and clang-tidy integration.
https://github.com/llvm/llvm-project/issues/55937

clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp

 }
 
 static const NoteTag *getNoteTag(CheckerContext &C,
                                  const DynamicCastInfo *CastInfo,
                                  QualType CastToTy, const Expr *Object,
                                  bool CastSucceeds, bool IsKnownCast) {
   std::string CastToName =
       CastInfo ? CastInfo->to()->getAsCXXRecordDecl()->getNameAsString()
-               : CastToTy->getPointeeCXXRecordDecl()->getNameAsString();
+               : CastToTy.getAsString();
   Object = Object->IgnoreParenImpCasts();
 
   return C.getNoteTag(
       [=]() -> std::string {
         SmallString<128> Msg;
         llvm::raw_svector_ostream Out(Msg);
 
         if (!IsKnownCast)
return C.getNoteTag(
         } else {
           Out << (IsKnownCast ? "The object" : "the object");
         }
         Out << " is";
 
         bool First = true;
         for (QualType CastToTy: CastToTyVec) {
           std::string CastToName =
-            CastToTy->getAsCXXRecordDecl() ?
-            CastToTy->getAsCXXRecordDecl()->getNameAsString() :
-            CastToTy->getPointeeCXXRecordDecl()->getNameAsString();
+              CastToTy->getAsCXXRecordDecl()
+                  ? CastToTy->getAsCXXRecordDecl()->getNameAsString()
+                  : CastToTy.getAsString();
           Out << ' ' << ((CastToTyVec.size() == 1) ? "not" :
                          (First ? "neither" : "nor")) << " a '" << CastToName
               << '\'';
           First = false;
         }
 
         return std::string(Out.str());
       },

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

 using namespace llvm;
 using namespace clang;
 
 void clang_analyzer_printState();
 
 #if defined(X86)
 void evalReferences(const Shape &S) {
   const auto &C = dyn_cast<Circle>(S);
-  // expected-note@-1 {{Assuming 'S' is not a 'Circle'}}
+  // expected-note@-1 {{Assuming 'S' is not a 'const class clang::Circle &'}}
   // expected-note@-2 {{Dereference of null pointer}}
   // expected-warning@-3 {{Dereference of null pointer}}
   clang_analyzer_printState();
   // XX86-CHECK:      "dynamic_types": [
   // XX86-CHECK-NEXT:   { "region": "SymRegion{reg_$0<const struct clang::Shape & S>}", "dyn_type": "const class clang::Circle &", "sub_classable": true }
   (void)C;
 }
 #if defined(SUPPRESSED)
 void evalReferences_addrspace(const Shape &S) {
   const auto &C = dyn_cast<DEVICE Circle>(S);
   clang_analyzer_printState();
   // X86-CHECK-SUPPRESSED: "dynamic_types": [
   // X86-CHECK-SUPPRESSED-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;
 }
 #endif
 #if defined(NOT_SUPPRESSED)
 void evalReferences_addrspace(const Shape &S) {
   const auto &C = dyn_cast<DEVICE Circle>(S);
-  // expected-note@-1 {{Assuming 'S' is not a 'Circle'}}
+  // expected-note@-1 {{Assuming 'S' is not a 'const __attribute__((address_space(3))) class clang::Circle &'}}
   // expected-note@-2 {{Dereference of null pointer}}
   // expected-warning@-3 {{Dereference of null pointer}}
   clang_analyzer_printState();
   // X86-CHECK: "dynamic_types": [
   // X86-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;
 }
 #endif
 #elif defined(MIPS)
 void evalReferences(const Shape &S) {
   const auto &C = dyn_cast<Circle>(S);
-  // expected-note@-1 {{Assuming 'S' is not a 'Circle'}}
+  // expected-note@-1 {{Assuming 'S' is not a 'const class clang::Circle &'}}
   // expected-note@-2 {{Dereference of null pointer}}
   // expected-warning@-3 {{Dereference of null pointer}}
 }
 #if defined(MIPS_SUPPRESSED)
 void evalReferences_addrspace(const Shape &S) {
   const auto &C = dyn_cast<DEVICE Circle>(S);
   (void)C;
 }
 #endif
 #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@-1 {{Assuming 'C' is a 'const class clang::Circle *'}}
     // expected-note@-2 {{Taking false branch}}
     return;
   }
 
   if (dyn_cast_or_null<Triangle>(C)) {
-    // expected-note@-1 {{Assuming 'C' is not a 'Triangle'}}
+    // expected-note@-1 {{Assuming 'C' is not a 'const class clang::Triangle *'}}
     // expected-note@-2 {{Taking false branch}}
     return;
   }
 
   if (dyn_cast_or_null<Rectangle>(C)) {
-    // expected-note@-1 {{Assuming 'C' is not a 'Rectangle'}}
+    // expected-note@-1 {{Assuming 'C' is not a 'const class clang::Rectangle *'}}
     // expected-note@-2 {{Taking false branch}}
     return;
   }
 
   if (dyn_cast_or_null<Hexagon>(C)) {
-    // expected-note@-1 {{Assuming 'C' is not a 'Hexagon'}}
+    // expected-note@-1 {{Assuming 'C' is not a 'const class clang::Hexagon *'}}
     // expected-note@-2 {{Taking false branch}}
     return;
   }
 
   if (isa<Triangle>(C)) {
     // expected-note@-1 {{'C' is not a 'Triangle'}}
     // expected-note@-2 {{Taking false branch}}
     return;
if (isa<Circle, Rectangle, Hexagon>(C)) {
     // expected-note@-1 {{'C' is non-null}}
     // expected-note@-2 {{Division by zero}}
     // expected-warning@-3 {{Division by zero}}
   }
 }
 
 void evalNonNullParamNonNullReturn(const Shape *S) {
   const auto *C = cast<Circle>(S);
-  // expected-note@-1 {{'S' is a 'Circle'}}
+  // expected-note@-1 {{'S' is a 'const class clang::Circle *'}}
   // expected-note@-2 {{'C' initialized here}}
 
   if (!dyn_cast_or_null<Circle>(C)) {
-    // expected-note@-1 {{'C' is a 'Circle'}}
+    // expected-note@-1 {{Assuming 'C' is a 'const class clang::Circle *'}}
     // expected-note@-2 {{Taking false branch}}
     return;
   }
 
   if (dyn_cast_or_null<Triangle>(C)) {
-    // expected-note@-1 {{Assuming 'C' is not a 'Triangle'}}
+    // expected-note@-1 {{Assuming 'C' is not a 'const class clang::Triangle *'}}
     // expected-note@-2 {{Taking false branch}}
     return;
   }
 
   if (dyn_cast_or_null<Rectangle>(C)) {
-    // expected-note@-1 {{Assuming 'C' is not a 'Rectangle'}}
+    // expected-note@-1 {{Assuming 'C' is not a 'const class clang::Rectangle *'}}
     // expected-note@-2 {{Taking false branch}}
     return;
   }
 
   if (dyn_cast_or_null<Hexagon>(C)) {
-    // expected-note@-1 {{Assuming 'C' is not a 'Hexagon'}}
+    // expected-note@-1 {{Assuming 'C' is not a 'const class clang::Hexagon *'}}
     // expected-note@-2 {{Taking false branch}}
     return;
   }
 
   if (isa<Triangle>(C)) {
     // expected-note@-1 {{'C' is not a 'Triangle'}}
     // expected-note@-2 {{Taking false branch}}
     return;
if (isa<Circle, Rectangle, Hexagon>(C)) {
     // expected-note@-1 {{'C' is non-null}}
     // expected-note@-2 {{Division by zero}}
     // expected-warning@-3 {{Division by zero}}
   }
 }
 
 void evalNonNullParamNullReturn(const Shape *S) {
   const auto *C = dyn_cast_or_null<Circle>(S);
-  // expected-note@-1 {{Assuming 'S' is not a 'Circle'}}
+  // expected-note@-1 {{Assuming 'S' is not a 'const class clang::Circle *'}}
 
   if (const auto *T = dyn_cast_or_null<Triangle>(S)) {
-    // expected-note@-1 {{Assuming 'S' is a 'Triangle'}}
+    // expected-note@-1 {{Assuming 'S' is a 'const class clang::Triangle *'}}
     // expected-note@-2 {{'T' initialized here}}
     // expected-note@-3 {{'T' is non-null}}
     // expected-note@-4 {{Taking true branch}}
 
     (void)(1 / !T);
     // expected-note@-1 {{'T' is non-null}}
     // expected-note@-2 {{Division by zero}}
     // expected-warning@-3 {{Division by zero}}
   }
 }
 
 void evalNullParamNullReturn(const Shape *S) {
   const auto *C = dyn_cast_or_null<Circle>(S);
   // expected-note@-1 {{Assuming null pointer is passed into cast}}
   // expected-note@-2 {{'C' initialized to a null pointer value}}
 
   (void)(1 / (bool)C);
   // expected-note@-1 {{Division by zero}}
   // expected-warning@-2 {{Division by zero}}
 }
 
 void evalZeroParamNonNullReturnPointer(const Shape *S) {
   const auto *C = S->castAs<Circle>();
-  // expected-note@-1 {{'S' is a 'Circle'}}
+  // expected-note@-1 {{'S' is a 'const class clang::Circle *'}}
   // expected-note@-2 {{'C' initialized here}}
 
   (void)(1 / !C);
   // expected-note@-1 {{'C' is non-null}}
   // expected-note@-2 {{Division by zero}}
   // expected-warning@-3 {{Division by zero}}
 }
 
 void evalZeroParamNonNullReturn(const Shape &S) {
   const auto *C = S.castAs<Circle>();
   // expected-note@-1 {{'C' initialized here}}
 
   (void)(1 / !C);
   // expected-note@-1 {{'C' is non-null}}
   // expected-note@-2 {{Division by zero}}
   // expected-warning@-3 {{Division by zero}}
 }
 
 void evalZeroParamNullReturn(const Shape *S) {
   const auto &C = S->getAs<Circle>();
-  // expected-note@-1 {{Assuming 'S' is not a 'Circle'}}
+  // expected-note@-1 {{Assuming 'S' is not a 'const class clang::Circle *'}}
   // expected-note@-2 {{Storing null pointer value}}
   // expected-note@-3 {{'C' initialized here}}
 
   if (!dyn_cast_or_null<Triangle>(S)) {
-    // expected-note@-1 {{Assuming 'S' is a 'Triangle'}}
+    // expected-note@-1 {{Assuming 'S' is a 'const class clang::Triangle *'}}
     // expected-note@-2 {{Taking false branch}}
     return;
   }
 
   if (!dyn_cast_or_null<Triangle>(S)) {
     // expected-note@-1 {{'S' is a 'Triangle'}}
     // expected-note@-2 {{Taking false branch}}
     return;
   }
 
   (void)(1 / (bool)C);
   // expected-note@-1 {{Division by zero}}
   // expected-warning@-2 {{Division by zero}}
 }
+
+// don't crash
+// CastValueChecker was using QualType()->getPointeeCXXRecordDecl(), in
+// getNoteTag which evaluated to nullptr, then crashed when attempting to
+// deref an invocation to getNameAsString(). The fix is to use
+// QualType().getAsString().
+//
+// Example:
+// std::string CastToName =
+//       CastInfo ? CastInfo->to()->getAsCXXRecordDecl()->getNameAsString()
+//                : CastToTy->getPointeeCXXRecordDecl()->getNameAsString();
+// Changed to:
+// std::string CastToName =
+//       CastInfo ? CastInfo->to()->getAsCXXRecordDecl()->getNameAsString()
+//                : CastToTy.getAsString();
+namespace llvm {
+template <typename, typename a> void isa(a &);
+template <typename> class PointerUnion {
+public:
+  template <typename T> T *getAs() {
+    (void)isa<int>(*this);
+    return nullptr;
+  }
+};
+class LLVMContext {
+  PointerUnion<LLVMContext> c;
+  void d() { c.getAs<int>(); }
+};
+} // namespace llvm

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

 class Square : public Shape {};
 } // namespace clang
 
 using namespace llvm;
 using namespace clang;
 
 void evalNonNullParamNonNullReturn(const Shape *S) {
   const auto *C = dyn_cast_or_null<Circle>(S);
-  // expected-note@-1 {{Assuming 'S' is a 'Circle'}}
+  // expected-note@-1 {{Assuming 'S' is a 'const class clang::Circle *'}}
   // expected-note@-2 {{'C' initialized here}}
 
   // FIXME: We assumed that 'S' is a 'Circle' therefore it is not a 'Square'.
   if (dyn_cast_or_null<Square>(S)) {
-    // expected-note@-1 {{Assuming 'S' is not a 'Square'}}
+    // expected-note@-1 {{Assuming 'S' is not a 'const class clang::Square *'}}
     // expected-note@-2 {{Taking false branch}}
     return;
   }
 
   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 }