Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, szepet,
whisperity.
For the following example:
struct Base {
int x;
};
// In a different translation unit
struct Derived : public Base {
Derived() {}
};
For a call to `Derived::Derived()`, we'll receive a note that `this->x` is
uninitialized. Since `x` is not a direct field of `Derived`, it could be a
little confusing. This patch aims to fix this, we well as the case when the
derived object has a field that has the name as an inherited uninitialized data
member:
struct Base {
int x; // note: uninitialized field 'this->Base::x'
};
struct Derived : public Base {
int x = 5;
Derived() {}
};
This patch is also meant as a conversation starter, as the way base classes
were handled has been long debated.
Repository:
rC Clang
https://reviews.llvm.org/D50905
Files:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
test/Analysis/cxx-uninitialized-object-inheritance.cpp
Index: test/Analysis/cxx-uninitialized-object-inheritance.cpp
===================================================================
--- test/Analysis/cxx-uninitialized-object-inheritance.cpp
+++ test/Analysis/cxx-uninitialized-object-inheritance.cpp
@@ -35,7 +35,7 @@
}
class NonPolymorphicBaseClass2 {
- int x; // expected-note{{uninitialized field 'this->x'}}
+ int x; // expected-note{{uninitialized field 'this->NonPolymorphicBaseClass2::x'}}
protected:
int y;
@@ -62,7 +62,7 @@
int x;
protected:
- int y; // expected-note{{uninitialized field 'this->y'}}
+ int y; // expected-note{{uninitialized field 'this->NonPolymorphicBaseClass3::y'}}
public:
NonPolymorphicBaseClass3() = default;
NonPolymorphicBaseClass3(int) : x(7) {}
@@ -140,7 +140,7 @@
}
class PolymorphicRight1 {
- int x; // expected-note{{uninitialized field 'this->x'}}
+ int x; // expected-note{{uninitialized field 'this->PolymorphicRight1::x'}}
protected:
int y;
@@ -168,7 +168,7 @@
int x;
protected:
- int y; // expected-note{{uninitialized field 'this->y'}}
+ int y; // expected-note{{uninitialized field 'this->PolymorphicBaseClass3::y'}}
public:
virtual ~PolymorphicBaseClass3() = default;
PolymorphicBaseClass3() = default;
@@ -248,7 +248,7 @@
}
class VirtualPolymorphicRight1 {
- int x; // expected-note{{uninitialized field 'this->x'}}
+ int x; // expected-note{{uninitialized field 'this->VirtualPolymorphicRight1::x'}}
protected:
int y;
@@ -276,7 +276,7 @@
int x;
protected:
- int y; // expected-note{{uninitialized field 'this->y'}}
+ int y; // expected-note{{uninitialized field 'this->VirtualPolymorphicBaseClass3::y'}}
public:
virtual ~VirtualPolymorphicBaseClass3() = default;
VirtualPolymorphicBaseClass3() = default;
@@ -358,7 +358,7 @@
Left2(int) : x(36) {}
};
struct Right2 {
- int y; // expected-note{{uninitialized field 'this->y'}}
+ int y; // expected-note{{uninitialized field 'this->Right2::y'}}
Right2() = default;
Right2(int) : y(37) {}
};
@@ -378,7 +378,7 @@
}
struct Left3 {
- int x; // expected-note{{uninitialized field 'this->x'}}
+ int x; // expected-note{{uninitialized field 'this->Left3::x'}}
Left3() = default;
Left3(int) : x(39) {}
};
@@ -433,7 +433,7 @@
Left5(int) : x(44) {}
};
struct Right5 {
- int y; // expected-note{{uninitialized field 'this->y'}}
+ int y; // expected-note{{uninitialized field 'this->Right5::y'}}
Right5() = default;
Right5(int) : y(45) {}
};
@@ -514,7 +514,7 @@
}
struct NonVirtualBase2 {
- int x; // expected-note{{uninitialized field 'this->x'}}
+ int x; // expected-note{{uninitialized field 'this->NonVirtualBase2::x'}}
NonVirtualBase2() = default;
NonVirtualBase2(int) : x(52) {}
};
@@ -542,7 +542,7 @@
}
struct NonVirtualBase3 {
- int x; // expected-note{{uninitialized field 'this->x'}}
+ int x; // expected-note{{uninitialized field 'this->NonVirtualBase3::x'}}
NonVirtualBase3() = default;
NonVirtualBase3(int) : x(54) {}
};
@@ -570,8 +570,8 @@
}
struct NonVirtualBase4 {
- int x; // expected-note{{uninitialized field 'this->x'}}
- // expected-note@-1{{uninitialized field 'this->x'}}
+ int x; // expected-note{{uninitialized field 'this->NonVirtualBase4::x'}}
+ // expected-note@-1{{uninitialized field 'this->NonVirtualBase4::x'}}
NonVirtualBase4() = default;
NonVirtualBase4(int) : x(56) {}
};
@@ -626,7 +626,7 @@
}
struct NonVirtualBase6 {
- int x; // expected-note{{uninitialized field 'this->x'}}
+ int x; // expected-note{{uninitialized field 'this->NonVirtualBase6::x'}}
NonVirtualBase6() = default;
NonVirtualBase6(int) : x(59) {}
};
@@ -712,7 +712,7 @@
}
struct VirtualBase2 {
- int x; // expected-note{{uninitialized field 'this->x'}}
+ int x; // expected-note{{uninitialized field 'this->VirtualBase2::x'}}
VirtualBase2() = default;
VirtualBase2(int) : x(63) {}
};
@@ -751,7 +751,7 @@
}
struct VirtualBase3 {
- int x; // expected-note{{uninitialized field 'this->x'}}
+ int x; // expected-note{{uninitialized field 'this->VirtualBase3::x'}}
VirtualBase3() = default;
VirtualBase3(int) : x(66) {}
};
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -93,6 +93,33 @@
}
};
+/// Represents that the FieldNode that comes after this is declared in a base
+/// of the previous FieldNode.
+class BaseClass final : public FieldNode {
+ const QualType BaseClassT;
+
+public:
+ BaseClass(const QualType &T) : FieldNode(nullptr), BaseClassT(T) {
+ assert(!T.isNull());
+ }
+
+ virtual void printNoteMsg(llvm::raw_ostream &Out) const override {
+ llvm_unreachable("This node can never be the final node in the "
+ "fieldchain!");
+ }
+
+ virtual void printPrefix(llvm::raw_ostream &Out) const override {}
+
+ virtual void printNode(llvm::raw_ostream &Out) const override {
+ Out << StringRef(BaseClassT.getAsString()).ltrim("struct ").ltrim("class ")
+ << "::";
+ }
+
+ virtual void printSeparator(llvm::raw_ostream &Out) const override {}
+
+ virtual bool isBase() const { return true; }
+};
+
} // end of anonymous namespace
// Utility function declarations.
@@ -265,7 +292,8 @@
continue;
}
- if (T->isAnyPointerType() || T->isReferenceType() || T->isBlockPointerType()) {
+ if (T->isAnyPointerType() || T->isReferenceType() ||
+ T->isBlockPointerType()) {
if (isPointerOrReferenceUninit(FR, LocalChain))
ContainsUninitField = true;
continue;
@@ -294,8 +322,17 @@
.castAs<loc::MemRegionVal>()
.getRegionAs<TypedValueRegion>();
- if (isNonUnionUninit(BaseRegion, LocalChain))
- ContainsUninitField = true;
+ // If the head of the list is also a BaseClass, we'll overwrite it to avoid
+ // note messages like 'this->A::B::x'.
+ if (!LocalChain.isEmpty() && LocalChain.getHead().isBase()) {
+ if (isNonUnionUninit(BaseRegion, LocalChain.replaceHead(
+ BaseClass(BaseSpec.getType()))))
+ ContainsUninitField = true;
+ } else {
+ if (isNonUnionUninit(BaseRegion,
+ LocalChain.add(BaseClass(BaseSpec.getType()))))
+ ContainsUninitField = true;
+ }
}
return ContainsUninitField;
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
===================================================================
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
@@ -32,10 +32,10 @@
protected:
const FieldRegion *FR;
- ~FieldNode() = default;
+ /* non-virtual */ ~FieldNode() = default;
public:
- FieldNode(const FieldRegion *FR) : FR(FR) { assert(FR); }
+ FieldNode(const FieldRegion *FR) : FR(FR) {}
FieldNode() = delete;
FieldNode(const FieldNode &) = delete;
@@ -47,11 +47,21 @@
/// FoldingSet.
void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddPointer(this); }
- bool operator<(const FieldNode &Other) const { return FR < Other.FR; }
- bool isSameRegion(const FieldRegion *OtherFR) const { return FR == OtherFR; }
+ // Helper method for uniqueing.
+ bool isSameRegion(const FieldRegion *OtherFR) const {
+ // Special FieldNode descendants may wrap nullpointers -- we wouldn't like
+ // to unique these objects.
+ if (FR == nullptr)
+ return false;
+
+ return FR == OtherFR;
+ }
const FieldRegion *getRegion() const { return FR; }
- const FieldDecl *getDecl() const { return FR->getDecl(); }
+ const FieldDecl *getDecl() const {
+ assert(FR);
+ return FR->getDecl();
+ }
// When a fieldchain is printed (a list of FieldNode objects), it will have
// the following format:
@@ -71,6 +81,8 @@
/// Print the separator. For example, fields may be separated with '.' or
/// "->".
virtual void printSeparator(llvm::raw_ostream &Out) const = 0;
+
+ virtual bool isBase() const { return false; }
};
/// Returns with Field's name. This is a helper function to get the correct name
@@ -94,15 +106,24 @@
FieldChain::Factory &ChainFactory;
FieldChain Chain;
+ FieldChainInfo(FieldChain::Factory &F, FieldChain NewChain)
+ : FieldChainInfo(F) {
+ Chain = NewChain;
+ }
+
public:
FieldChainInfo() = delete;
FieldChainInfo(FieldChain::Factory &F) : ChainFactory(F) {}
FieldChainInfo(const FieldChainInfo &Other) = default;
template <class FieldNodeT> FieldChainInfo add(const FieldNodeT &FN);
+ template <class FieldNodeT> FieldChainInfo replaceHead(const FieldNodeT &FN);
bool contains(const FieldRegion *FR) const;
+ bool isEmpty() const { return Chain.isEmpty(); }
+
const FieldRegion *getUninitRegion() const;
+ const FieldNode &getHead() { return Chain.getHead(); }
void printNoteMsg(llvm::raw_ostream &Out) const;
};
@@ -250,6 +271,12 @@
return NewChain;
}
+template <class FieldNodeT>
+inline FieldChainInfo FieldChainInfo::replaceHead(const FieldNodeT &FN) {
+ FieldChainInfo NewChain(ChainFactory, Chain.getTail());
+ return NewChain.add(FN);
+}
+
} // end of namespace ento
} // end of namespace clang
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
