[analyzer][UninitializedObjectChecker] Void pointer objects are casted back to their dynmic type in note message

[Szelethus]

Rebased to the latest trunk.

[george.karpenkov]

Cf. my comments to https://reviews.llvm.org/D49437: while this change looks great, is it possible to separate the pointer chasing from the rest of the checker?

[NoQ]

test/Analysis/cxx-uninitialized-object-ptr-ref.cpp

295

Shouldn't this rather say something like static_cast<int *>(this->vptr)? That's the normal syntax to do this sort of stuff, i guess.

[whisperity]

test/Analysis/cxx-uninitialized-object-ptr-ref.cpp

295

Not just "normal syntax", this syntax of the output does not compile.

foo.cpp:6:13: error: expected unqualified-id
      this->static_cast<int*>(vptr) = new int();

[Szelethus]

The solution now relies on the refactored version of the checker. Note that it modifies roughly 75% less code ;).

lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp

public:
   virtual void printSeparator(llvm::raw_ostream &Out) const override {
     if (getDecl()->getType()->isPointerType())
       Out << "->";
     else
       Out << '.';
   }
 };
 
+/// Represents a void* field that needs to be casted back to its dynamic type
+/// for a correct note message.
+class NeedsCastLocField final : public FieldNode {
+  QualType CastBackType;
+
+public:
+  NeedsCastLocField(const FieldRegion *FR, const QualType &T)
+      : FieldNode(FR), CastBackType(T) {}
+
+  virtual void printNoteMsg(llvm::raw_ostream &Out) const override {
+    Out << "uninitialized pointee ";
+  }
+
+  virtual void printPrefix(llvm::raw_ostream &Out) const override {
+    Out << "static_cast" << '<' << CastBackType.getAsString() << ">(";
+  }
+
+  virtual void printNode(llvm::raw_ostream &Out) const override {
+    Out << getVariableName(getDecl()) << ')';
+  }
+
+  virtual void printSeparator(llvm::raw_ostream &Out) const override {
+    Out << "->";
+  }
+};
+
 } // end of anonymous namespace
 
 // Utility function declarations.
 
 /// Returns whether T can be (transitively) dereferenced to a void pointer type
 /// (void*, void**, ...). The type of the region behind a void pointer isn't
 /// known, and thus FD can not be analyzed.
 static bool isVoidPointer(QualType T);
bool FindUninitializedFields::isPointerOrReferenceUninit(
   DynamicTypeInfo DynTInfo = getDynamicTypeInfo(State, L.getRegion());
   if (!DynTInfo.isValid()) {
     IsAnyFieldInitialized = true;
     return false;
   }
 
   QualType DynT = DynTInfo.getType();
 
+  // If the static type of the field is a void pointer, we need to cast it back
+  // to the dynamic type before dereferencing.
+  bool NeedsCastBack = isVoidPointer(FR->getDecl()->getType());
+
   if (isVoidPointer(DynT)) {
     IsAnyFieldInitialized = true;
     return false;
   }
 
   // At this point the pointer itself is initialized and points to a valid
   // location, we'll now check the pointee.
   SVal DerefdV = State->getSVal(V.castAs<Loc>(), DynT);
bool FindUninitializedFields::isPointerOrReferenceUninit(
   }
 
   // If FR is a pointer pointing to a non-primitive type.
   if (Optional<nonloc::LazyCompoundVal> RecordV =
           DerefdV.getAs<nonloc::LazyCompoundVal>()) {
 
     const TypedValueRegion *R = RecordV->getRegion();
 
-    if (DynT->getPointeeType()->isStructureOrClassType())
+    if (DynT->getPointeeType()->isStructureOrClassType()) {
+      if (NeedsCastBack)
+        return isNonUnionUninit(R, LocalChain.add(NeedsCastLocField(FR, DynT)));
       return isNonUnionUninit(R, LocalChain.add(LocField(FR)));
+    }
 
     if (DynT->getPointeeType()->isUnionType()) {
       if (isUnionUninit(R)) {
+        if (NeedsCastBack)
+          return addFieldToUninits(LocalChain.add(NeedsCastLocField(FR, DynT)));
         return addFieldToUninits(LocalChain.add(LocField(FR)));
       } else {
         IsAnyFieldInitialized = true;
         return false;
       }
     }
 
     if (DynT->getPointeeType()->isArrayType()) {
       IsAnyFieldInitialized = true;
       return false;
     }
 
     llvm_unreachable("All cases are handled!");
   }
 
   assert((isPrimitiveType(DynT->getPointeeType()) || DynT->isPointerType() ||
           DynT->isReferenceType()) &&
          "At this point FR must either have a primitive dynamic type, or it "
          "must be a null, undefined, unknown or concrete pointer!");
 
-  if (isPrimitiveUninit(DerefdV))
+  if (isPrimitiveUninit(DerefdV)) {
+    if (NeedsCastBack)
+      return addFieldToUninits(LocalChain.add(NeedsCastLocField(FR, DynT)));
     return addFieldToUninits(LocalChain.add(LocField(FR)));
+  }
 
   IsAnyFieldInitialized = true;
   return false;
 }
 
 //===----------------------------------------------------------------------===//
 //                           Utility functions.
 //===----------------------------------------------------------------------===//

test/Analysis/cxx-uninitialized-object-ptr-ref.cpp

struct CyclicVoidPointerTest {
   CyclicVoidPointerTest() : vptr(&vptr) {}
 };
 
 void fCyclicVoidPointerTest() {
   CyclicVoidPointerTest();
 }
 
 struct IntDynTypedVoidPointerTest1 {
-  void *vptr; // expected-note{{uninitialized pointee 'this->vptr'}}
+  void *vptr; // expected-note{{uninitialized pointee 'static_cast<int *>(this->vptr)'}}
   int dontGetFilteredByNonPedanticMode = 0;
 
   IntDynTypedVoidPointerTest1(void *vptr) : vptr(vptr) {} // expected-warning{{1 uninitialized field}}
 };
 
 void fIntDynTypedVoidPointerTest1() {
   int a;
   IntDynTypedVoidPointerTest1 tmp(&a);
 }
 
 struct RecordDynTypedVoidPointerTest {
   struct RecordType {
-    int x; // expected-note{{uninitialized field 'this->vptr->x'}}
-    int y; // expected-note{{uninitialized field 'this->vptr->y'}}
+    int x; // expected-note{{uninitialized field 'static_cast<struct RecordDynTypedVoidPointerTest::RecordType *>(this->vptr)->x'}}
+    int y; // expected-note{{uninitialized field 'static_cast<struct RecordDynTypedVoidPointerTest::RecordType *>(this->vptr)->y'}}
   };
 
   void *vptr;
   int dontGetFilteredByNonPedanticMode = 0;
 
   RecordDynTypedVoidPointerTest(void *vptr) : vptr(vptr) {} // expected-warning{{2 uninitialized fields}}
 };
 
 void fRecordDynTypedVoidPointerTest() {
   RecordDynTypedVoidPointerTest::RecordType a;
   RecordDynTypedVoidPointerTest tmp(&a);
 }
 
 struct NestedNonVoidDynTypedVoidPointerTest {
   struct RecordType {
-    int x;      // expected-note{{uninitialized field 'this->vptr->x'}}
-    int y;      // expected-note{{uninitialized field 'this->vptr->y'}}
-    void *vptr; // expected-note{{uninitialized pointee 'this->vptr->vptr'}}
+    int x;      // expected-note{{uninitialized field 'static_cast<struct NestedNonVoidDynTypedVoidPointerTest::RecordType *>(this->vptr)->x'}}
+    int y;      // expected-note{{uninitialized field 'static_cast<struct NestedNonVoidDynTypedVoidPointerTest::RecordType *>(this->vptr)->y'}}
+    void *vptr; // expected-note{{uninitialized pointee 'static_cast<char *>(static_cast<struct NestedNonVoidDynTypedVoidPointerTest::RecordType *>(this->vptr)->vptr)'}}
   };
 
   void *vptr;
   int dontGetFilteredByNonPedanticMode = 0;
 
   NestedNonVoidDynTypedVoidPointerTest(void *vptr, void *c) : vptr(vptr) {
     static_cast<RecordType *>(vptr)->vptr = c; // expected-warning{{3 uninitialized fields}}
   }