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

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 159918.
Szelethus added a comment.

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


https://reviews.llvm.org/D49228

Files:
  lib/StaticAnalyzer/Checkers/UninitializedPointee.cpp
  test/Analysis/cxx-uninitialized-object-ptr-ref.cpp


Index: test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
===================================================================
--- test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
+++ test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
@@ -292,7 +292,7 @@
 }
 
 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}}
@@ -305,8 +305,8 @@
 
 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;
@@ -322,9 +322,9 @@
 
 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;
Index: lib/StaticAnalyzer/Checkers/UninitializedPointee.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/UninitializedPointee.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedPointee.cpp
@@ -60,6 +60,32 @@
   }
 };
 
+/// Represents a void* field that needs to be casted back to its dynamic type
+/// for a correct note message.
+class NeedsCastLocField : 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 {
+    Out << getVariableName(getDecl()) << ')';
+  }
+
+  virtual void printSeparator(llvm::raw_ostream &Out) const override {
+    Out << "->";
+  }
+};
+
 } // end of anonymous namespace
 
 // Utility function declarations.
@@ -110,6 +136,10 @@
   assert(V.getAs<loc::MemRegionVal>() &&
          "At this point V must be loc::MemRegionVal!");
 
+  // 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());
+
   QualType DynT;
 
   // At this point the pointer itself is initialized and points to a valid
@@ -125,11 +155,16 @@
 
     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;
@@ -150,8 +185,11 @@
          "At this point FR must either have a primitive dynamic type, or it "
          "must be a null, undefined, unknown or concrete pointer!");
 
-  if (isPrimitiveUninit(V))
+  if (isPrimitiveUninit(V)) {
+    if (NeedsCastBack)
+      return addFieldToUninits(LocalChain.add(NeedsCastLocField(FR, DynT)));
     return addFieldToUninits(LocalChain.add(LocField(FR)));
+  }
 
   IsAnyFieldInitialized = true;
   return false;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits