[analyzer][UninitializedObjectChecker] Explicit namespace resolution for inherited data members
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, as 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() {}
};
Differential Revision: https://reviews.llvm.org/D50905
llvm-svn: 340272
diff --git a/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h b/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
index bcecfb7..257186e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
+++ b/clang/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
diff --git a/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
index 2281d66..288412e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ b/clang/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());
+ assert(T->getAsCXXRecordDecl());
+ }
+
+ 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 << BaseClassT->getAsCXXRecordDecl()->getName() << "::";
+ }
+
+ virtual void printSeparator(llvm::raw_ostream &Out) const override {}
+
+ virtual bool isBase() const { return true; }
+};
+
} // end of anonymous namespace
// Utility function declarations.
@@ -295,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;