[clang] c7fa4e8 - [analyzer] Fix null pointer deref in CastValueChecker

via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 7 10:35:14 PDT 2022


Author: Vince Bridgers
Date: 2022-06-07T13:34:06-04:00
New Revision: c7fa4e8a8bc41e52cc49a27e0495d67d730fa167

URL: https://github.com/llvm/llvm-project/commit/c7fa4e8a8bc41e52cc49a27e0495d67d730fa167
DIFF: https://github.com/llvm/llvm-project/commit/c7fa4e8a8bc41e52cc49a27e0495d67d730fa167.diff

LOG: [analyzer] Fix null pointer deref in CastValueChecker

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.

Reviewed By: steakhal

Differential Revision: https://reviews.llvm.org/D127105

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
    clang/test/Analysis/cast-value-notes.cpp
    clang/test/Analysis/cast-value-state-dump.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
index e2b99fca7d148..45f9a82a9d0a4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
@@ -108,7 +108,7 @@ static const NoteTag *getNoteTag(CheckerContext &C,
                                  bool CastSucceeds, bool IsKnownCast) {
   std::string CastToName =
       CastInfo ? CastInfo->to()->getAsCXXRecordDecl()->getNameAsString()
-               : CastToTy->getPointeeCXXRecordDecl()->getNameAsString();
+               : CastToTy.getAsString();
   Object = Object->IgnoreParenImpCasts();
 
   return C.getNoteTag(
@@ -163,9 +163,9 @@ static const NoteTag *getNoteTag(CheckerContext &C,
         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
               << '\'';

diff  --git a/clang/test/Analysis/cast-value-notes.cpp b/clang/test/Analysis/cast-value-notes.cpp
index ddfaf07b2a0af..b3d32552e6ec8 100644
--- a/clang/test/Analysis/cast-value-notes.cpp
+++ b/clang/test/Analysis/cast-value-notes.cpp
@@ -73,7 +73,7 @@ void clang_analyzer_printState();
 #if defined(X86)
 void evalReferences(const Shape &S) {
   const auto &C = dyn_cast<Circle>(S);
-  // expected-note at -1 {{Assuming 'S' is not a 'Circle'}}
+  // expected-note at -1 {{Assuming 'S' is not a 'const class clang::Circle &'}}
   // expected-note at -2 {{Dereference of null pointer}}
   // expected-warning at -3 {{Dereference of null pointer}}
   clang_analyzer_printState();
@@ -93,7 +93,7 @@ void evalReferences_addrspace(const Shape &S) {
 #if defined(NOT_SUPPRESSED)
 void evalReferences_addrspace(const Shape &S) {
   const auto &C = dyn_cast<DEVICE Circle>(S);
-  // expected-note at -1 {{Assuming 'S' is not a 'Circle'}}
+  // expected-note at -1 {{Assuming 'S' is not a 'const __attribute__((address_space(3))) class clang::Circle &'}}
   // expected-note at -2 {{Dereference of null pointer}}
   // expected-warning at -3 {{Dereference of null pointer}}
   clang_analyzer_printState();
@@ -105,7 +105,7 @@ void evalReferences_addrspace(const Shape &S) {
 #elif defined(MIPS)
 void evalReferences(const Shape &S) {
   const auto &C = dyn_cast<Circle>(S);
-  // expected-note at -1 {{Assuming 'S' is not a 'Circle'}}
+  // expected-note at -1 {{Assuming 'S' is not a 'const class clang::Circle &'}}
   // expected-note at -2 {{Dereference of null pointer}}
   // expected-warning at -3 {{Dereference of null pointer}}
 }
@@ -122,25 +122,25 @@ void evalNonNullParamNonNullReturnReference(const Shape &S) {
   // expected-note at -1 {{'C' initialized here}}
 
   if (!dyn_cast_or_null<Circle>(C)) {
-    // expected-note at -1 {{'C' is a 'Circle'}}
+    // expected-note at -1 {{Assuming 'C' is a 'const class clang::Circle *'}}
     // expected-note at -2 {{Taking false branch}}
     return;
   }
 
   if (dyn_cast_or_null<Triangle>(C)) {
-    // expected-note at -1 {{Assuming 'C' is not a 'Triangle'}}
+    // expected-note at -1 {{Assuming 'C' is not a 'const class clang::Triangle *'}}
     // expected-note at -2 {{Taking false branch}}
     return;
   }
 
   if (dyn_cast_or_null<Rectangle>(C)) {
-    // expected-note at -1 {{Assuming 'C' is not a 'Rectangle'}}
+    // expected-note at -1 {{Assuming 'C' is not a 'const class clang::Rectangle *'}}
     // expected-note at -2 {{Taking false branch}}
     return;
   }
 
   if (dyn_cast_or_null<Hexagon>(C)) {
-    // expected-note at -1 {{Assuming 'C' is not a 'Hexagon'}}
+    // expected-note at -1 {{Assuming 'C' is not a 'const class clang::Hexagon *'}}
     // expected-note at -2 {{Taking false branch}}
     return;
   }
@@ -176,29 +176,29 @@ void evalNonNullParamNonNullReturnReference(const Shape &S) {
 
 void evalNonNullParamNonNullReturn(const Shape *S) {
   const auto *C = cast<Circle>(S);
-  // expected-note at -1 {{'S' is a 'Circle'}}
+  // expected-note at -1 {{'S' is a 'const class clang::Circle *'}}
   // expected-note at -2 {{'C' initialized here}}
 
   if (!dyn_cast_or_null<Circle>(C)) {
-    // expected-note at -1 {{'C' is a 'Circle'}}
+    // expected-note at -1 {{Assuming 'C' is a 'const class clang::Circle *'}}
     // expected-note at -2 {{Taking false branch}}
     return;
   }
 
   if (dyn_cast_or_null<Triangle>(C)) {
-    // expected-note at -1 {{Assuming 'C' is not a 'Triangle'}}
+    // expected-note at -1 {{Assuming 'C' is not a 'const class clang::Triangle *'}}
     // expected-note at -2 {{Taking false branch}}
     return;
   }
 
   if (dyn_cast_or_null<Rectangle>(C)) {
-    // expected-note at -1 {{Assuming 'C' is not a 'Rectangle'}}
+    // expected-note at -1 {{Assuming 'C' is not a 'const class clang::Rectangle *'}}
     // expected-note at -2 {{Taking false branch}}
     return;
   }
 
   if (dyn_cast_or_null<Hexagon>(C)) {
-    // expected-note at -1 {{Assuming 'C' is not a 'Hexagon'}}
+    // expected-note at -1 {{Assuming 'C' is not a 'const class clang::Hexagon *'}}
     // expected-note at -2 {{Taking false branch}}
     return;
   }
@@ -234,10 +234,10 @@ void evalNonNullParamNonNullReturn(const Shape *S) {
 
 void evalNonNullParamNullReturn(const Shape *S) {
   const auto *C = dyn_cast_or_null<Circle>(S);
-  // expected-note at -1 {{Assuming 'S' is not a 'Circle'}}
+  // expected-note at -1 {{Assuming 'S' is not a 'const class clang::Circle *'}}
 
   if (const auto *T = dyn_cast_or_null<Triangle>(S)) {
-    // expected-note at -1 {{Assuming 'S' is a 'Triangle'}}
+    // expected-note at -1 {{Assuming 'S' is a 'const class clang::Triangle *'}}
     // expected-note at -2 {{'T' initialized here}}
     // expected-note at -3 {{'T' is non-null}}
     // expected-note at -4 {{Taking true branch}}
@@ -261,7 +261,7 @@ void evalNullParamNullReturn(const Shape *S) {
 
 void evalZeroParamNonNullReturnPointer(const Shape *S) {
   const auto *C = S->castAs<Circle>();
-  // expected-note at -1 {{'S' is a 'Circle'}}
+  // expected-note at -1 {{'S' is a 'const class clang::Circle *'}}
   // expected-note at -2 {{'C' initialized here}}
 
   (void)(1 / !C);
@@ -282,12 +282,12 @@ void evalZeroParamNonNullReturn(const Shape &S) {
 
 void evalZeroParamNullReturn(const Shape *S) {
   const auto &C = S->getAs<Circle>();
-  // expected-note at -1 {{Assuming 'S' is not a 'Circle'}}
+  // expected-note at -1 {{Assuming 'S' is not a 'const class clang::Circle *'}}
   // expected-note at -2 {{Storing null pointer value}}
   // expected-note at -3 {{'C' initialized here}}
 
   if (!dyn_cast_or_null<Triangle>(S)) {
-    // expected-note at -1 {{Assuming 'S' is a 'Triangle'}}
+    // expected-note at -1 {{Assuming 'S' is a 'const class clang::Triangle *'}}
     // expected-note at -2 {{Taking false branch}}
     return;
   }
@@ -302,3 +302,32 @@ void evalZeroParamNullReturn(const Shape *S) {
   // expected-note at -1 {{Division by zero}}
   // expected-warning at -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

diff  --git a/clang/test/Analysis/cast-value-state-dump.cpp b/clang/test/Analysis/cast-value-state-dump.cpp
index 3e6a40cf1319b..c9d85f0b4460f 100644
--- a/clang/test/Analysis/cast-value-state-dump.cpp
+++ b/clang/test/Analysis/cast-value-state-dump.cpp
@@ -18,12 +18,12 @@ using namespace clang;
 
 void evalNonNullParamNonNullReturn(const Shape *S) {
   const auto *C = dyn_cast_or_null<Circle>(S);
-  // expected-note at -1 {{Assuming 'S' is a 'Circle'}}
+  // expected-note at -1 {{Assuming 'S' is a 'const class clang::Circle *'}}
   // expected-note at -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 at -1 {{Assuming 'S' is not a 'Square'}}
+    // expected-note at -1 {{Assuming 'S' is not a 'const class clang::Square *'}}
     // expected-note at -2 {{Taking false branch}}
     return;
   }


        


More information about the cfe-commits mailing list