[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/UninitializedObject/UninitializedObjectChecker.cpp

if (T->isUnionType()) {
       continue;
     }
 
     if (T->isArrayType()) {
       IsAnyFieldInitialized = true;
       continue;
     }
 
+    SVal V = State->getSVal(FieldVal);
+
     if (T->isAnyPointerType() || T->isReferenceType() ||
-        T->isBlockPointerType()) {
+        T->isBlockPointerType() || 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/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.
+/// 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 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() << ">(";
+    // 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 override {
     Out << getVariableName(getDecl()) << ')';
   }
 
   virtual void printSeparator(llvm::raw_ostream &Out) const override {
     Out << "->";

 /// reason dereferencing fails, returns with None.
 static llvm::Optional<std::tuple<SVal, QualType, bool>>
 dereference(ProgramStateRef State, const FieldRegion *FR);
 
 //===----------------------------------------------------------------------===//
 //                   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) {
 
+  SVal V = State->getSVal(FR);
+
   assert((FR->getDecl()->getType()->isAnyPointerType() ||
           FR->getDecl()->getType()->isReferenceType() ||
-          FR->getDecl()->getType()->isBlockPointerType()) &&
-         "This method only checks pointer/reference objects!");
-
-  SVal V = State->getSVal(FR);
+          FR->getDecl()->getType()->isBlockPointerType() ||
+          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(
dereference(ProgramStateRef State, const FieldRegion *FR) {
   DynamicTypeInfo DynTInfo;
   QualType DynT;
 
   // 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());
 
   SVal V = State->getSVal(FR);
+  if (auto LocAsInt = V.getAs<nonloc::LocAsInteger>()) {
+    NeedsCastBack = true;
+    V = LocAsInt->getLoc();
+  }
+
   assert(V.getAs<loc::MemRegionVal>() && "V must be loc::MemRegionVal!");
 
   // 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
while (auto Tmp = V.getAs<loc::MemRegionVal>()) {
 
     DynT = DynTInfo.getType();
 
     if (isVoidPointer(DynT)) {
       return None;
     }
 
     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 std::make_tuple(V, DynT, NeedsCastBack);
 }

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;
   };