[analyzer][UninitializedObjectChecker] Support for nonloc::LocAsInteger

[george.karpenkov]

I think a checker for uninitialized values left after a constructor call is very valuable.

It is uncertain whether it is valuable to check that all pointers point to initialized objects for a number of reasons:

• As we have seen, it increases the number of false positives
• It significantly increases the complexity of the checker
• Projects would be more reluctant to try the checker due to the first point

Could we actually go in an opposite direction and try to separate the pointer-chasing into perhaps a separate checker?

[Szelethus]

I just added a new patch as you wrote that comment (D49438)! Separating this functionality to a separate checker sounds great. I didn't actually check it, but I bet that around ~30-40% is at least indirectly in relation with pointer/reference handling, and it is getting a little out of hand.

I'll definitely explore this option too. Thanks for the idea! :)

[george.karpenkov]

I'm fine with this if pointer-chasing is not on by default.

[NoQ]

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

31

This one also suffers from D49228#1176136

[Szelethus]

Re-implemented with the refactored checker.

[Szelethus]

Rebased to D50509.

[Szelethus]

Rebased to D51057.

[NoQ]

Let's see what it finds, i guess.

lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp

if (T->isUnionType()) {
       continue;
     }
 
     if (T->isArrayType()) {
       IsAnyFieldInitialized = true;
       continue;
     }
 
-    if (T->isPointerType() || T->isReferenceType()) {
+    SVal V = State->getSVal(FieldVal);
+
+    if (T->isPointerType() || T->isReferenceType() ||
+        V.getAs<nonloc::LocAsInteger>()) {
       if (isPointerOrReferenceUninit(FR, LocalChain))
         ContainsUninitField = true;
       continue;
     }
 
     if (isPrimitiveType(T)) {
-      SVal V = State->getSVal(FieldVal);
-
       if (isPrimitiveUninit(V)) {
         if (addFieldToUninits(LocalChain.add(RegularField(FR))))
           ContainsUninitField = true;
       }
       continue;
     }
 
     llvm_unreachable("All cases are handled!");

lib/StaticAnalyzer/Checkers/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.
+/// Represents a nonloc::LocAsInteger or void* field, that point to objects, but
+/// 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() << ">(";
+    // If this object is a nonloc::LocAsInteger.
+    if (getDecl()->getType()->isIntegerType())
+      Out << "reinterpret_cast";
+    // If this pointer's dynamic type is different then it's static type.
+    else
+      Out << "static_cast";
+    Out << '<' << 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.
 
 /// 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);
 
-/// Dereferences V, stores the dereferenced value back into it, and stores it's
-/// dynamic type in DynT.
+/// Dereferences V, stores the dereferenced value back into it, it's dynamic
+/// type in DynT, and whether it needs to be casted back in NeedsCastBack.
 ///
 /// V must be loc::MemRegionVal.
 ///
 /// If for whatever reason dereferencing fails, returns with false.
-static bool dereference(ProgramStateRef State, SVal &V, QualType &DynT);
+static bool dereference(ProgramStateRef State, SVal &V, QualType &DynT,
+                        bool &NeedsCastBack);
 
 //===----------------------------------------------------------------------===//
 //                   Methods for FindUninitializedFields.
 //===----------------------------------------------------------------------===//
 
-// Note that pointers/references don't contain fields themselves, so in this
-// function we won't add anything to LocalChain.
 bool FindUninitializedFields::isPointerOrReferenceUninit(
     const FieldRegion *FR, FieldChainInfo LocalChain) {
 
-  assert((FR->getDecl()->getType()->isPointerType() ||
-          FR->getDecl()->getType()->isReferenceType()) &&
-         "This method only checks pointer/reference objects!");
-
   SVal V = State->getSVal(FR);
 
+  assert((FR->getDecl()->getType()->isPointerType() ||
+          FR->getDecl()->getType()->isReferenceType() ||
+          V.getAs<nonloc::LocAsInteger>()) &&
+         "This method only checks pointer/reference or LocAsInteger objects!");
+
   if (V.isUnknown() || V.getAs<loc::ConcreteInt>()) {
     IsAnyFieldInitialized = true;
     return false;
   }
 
   if (V.isUndef()) {
     return addFieldToUninits(
         LocalChain.add(LocField(FR, /*IsDereferenced*/ false)));
   }
 
   if (!CheckPointeeInitialization) {
     IsAnyFieldInitialized = true;
     return false;
   }
 
-  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.
+  // If the static type of the field is a void pointer or is a
+  // nonloc::LocAsInteger, 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
   // location, we'll now check the pointee.
-  if (!dereference(State, V, DynT)) {
+  if (!dereference(State, V, DynT, NeedsCastBack)) {
     IsAnyFieldInitialized = true;
     return false;
   }
 
   // If FR is a pointer pointing to a non-primitive type.
   if (Optional<nonloc::LazyCompoundVal> RecordV =
           V.getAs<nonloc::LazyCompoundVal>()) {
 
static bool isVoidPointer(QualType T) {
   while (!T.isNull()) {
     if (T->isVoidPointerType())
       return true;
     T = T->getPointeeType();
   }
   return false;
 }
 
-static bool dereference(ProgramStateRef State, SVal &V, QualType &DynT) {
+static bool dereference(ProgramStateRef State, SVal &V, QualType &DynT,
+                        bool &NeedsCastBack) {
+  if (auto LocAsInt = V.getAs<nonloc::LocAsInteger>()) {
+    NeedsCastBack = true;
+    V = LocAsInt->getLoc();
+  }
+
   // If V is multiple pointer value, we'll dereference it again (e.g.: int** ->
   // int*).
   while (auto Tmp = V.getAs<loc::MemRegionVal>()) {
     // We can't reason about symbolic regions, assume its initialized.
     // Note that this also avoids a potential infinite recursion, because
     // constructors for list-like classes are checked without being called, and
     // the Static Analyzer will construct a symbolic region for Node *next; or
     // similar code snippets.
     if (Tmp->getRegion()->getSymbolicBase()) {
       return false;
     }
 
     DynamicTypeInfo DynTInfo = getDynamicTypeInfo(State, Tmp->getRegion());
     if (!DynTInfo.isValid()) {
       return false;
     }
 
     DynT = DynTInfo.getType();
 
     if (isVoidPointer(DynT)) {
       return false;
     }
 
     V = State->getSVal(*Tmp, DynT);
+    // TODO: Handle the case where V is nonloc::LocAsInteger, while avoiding
+    // infinite loops in the following sinister case:
+    //   int *ptr = reinterpret_cast<int *>(ptr);
   }
   return true;
 }

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

struct ConcreteIntLocTest {
   ConcreteIntLocTest() : ptr(reinterpret_cast<int *>(0xDEADBEEF)) {}
 };
 
 void fConcreteIntLocTest() {
   ConcreteIntLocTest();
 }
 
 //===----------------------------------------------------------------------===//
+// nonloc::LocAsInteger tests.
+//===----------------------------------------------------------------------===//
+
+using intptr_t = long;
+
+struct LocAsIntegerTest {
+  intptr_t ptr; // expected-note{{uninitialized pointee 'reinterpret_cast<char *>(this->ptr)'}}
+  int dontGetFilteredByNonPedanticMode = 0;
+
+  LocAsIntegerTest(void *ptr) : ptr(reinterpret_cast<intptr_t>(ptr)) {} // expected-warning{{1 uninitialized field}}
+};
+
+void fLocAsIntegerTest() {
+  char c;
+  LocAsIntegerTest t(&c);
+}
+
+//===----------------------------------------------------------------------===//
 // Null pointer tests.
 //===----------------------------------------------------------------------===//
 
 class NullPtrTest {
   struct RecordType {
     int x;
     int y;
   };