[cfe-commits] r158477 - in /cfe/trunk: lib/Sema/SemaDeclCXX.cpp test/SemaCXX/constructor-initializer.cpp test/SemaCXX/uninitialized.cpp
Richard Trieu
rtrieu at google.com
Thu Jun 14 16:11:34 PDT 2012
Author: rtrieu
Date: Thu Jun 14 18:11:34 2012
New Revision: 158477
URL: http://llvm.org/viewvc/llvm-project?rev=158477&view=rev
Log:
Use a proper visitor to recursively check for uninitialized use in constructors.
Modified:
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/SemaCXX/constructor-initializer.cpp
cfe/trunk/test/SemaCXX/uninitialized.cpp
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=158477&r1=158476&r2=158477&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Jun 14 18:11:34 2012
@@ -23,6 +23,7 @@
#include "clang/AST/CharUnits.h"
#include "clang/AST/CXXInheritance.h"
#include "clang/AST/DeclVisitor.h"
+#include "clang/AST/EvaluatedExprVisitor.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/RecordLayout.h"
#include "clang/AST/RecursiveASTVisitor.h"
@@ -2038,73 +2039,95 @@
<< (unsigned)IsPointer;
}
-/// Checks an initializer expression for use of uninitialized fields, such as
-/// containing the field that is being initialized. Returns true if there is an
-/// uninitialized field was used an updates the SourceLocation parameter; false
-/// otherwise.
-static bool InitExprContainsUninitializedFields(const Stmt *S,
- const ValueDecl *LhsField,
- SourceLocation *L) {
- assert(isa<FieldDecl>(LhsField) || isa<IndirectFieldDecl>(LhsField));
-
- if (isa<CallExpr>(S)) {
- // Do not descend into function calls or constructors, as the use
- // of an uninitialized field may be valid. One would have to inspect
- // the contents of the function/ctor to determine if it is safe or not.
- // i.e. Pass-by-value is never safe, but pass-by-reference and pointers
- // may be safe, depending on what the function/ctor does.
- return false;
- }
- if (const MemberExpr *ME = dyn_cast<MemberExpr>(S)) {
- const NamedDecl *RhsField = ME->getMemberDecl();
-
- if (const VarDecl *VD = dyn_cast<VarDecl>(RhsField)) {
- // The member expression points to a static data member.
- assert(VD->isStaticDataMember() &&
- "Member points to non-static data member!");
- (void)VD;
- return false;
- }
-
- if (isa<EnumConstantDecl>(RhsField)) {
- // The member expression points to an enum.
- return false;
- }
+namespace {
+ class UninitializedFieldVisitor
+ : public EvaluatedExprVisitor<UninitializedFieldVisitor> {
+ Sema &S;
+ ValueDecl *VD;
+ public:
+ typedef EvaluatedExprVisitor<UninitializedFieldVisitor> Inherited;
+ UninitializedFieldVisitor(Sema &S, ValueDecl *VD) : Inherited(S.Context),
+ S(S), VD(VD) {
+ }
+
+ void HandleExpr(Expr *E) {
+ if (!E) return;
+
+ // Expressions like x(x) sometimes lack the surrounding expressions
+ // but need to be checked anyways.
+ HandleValue(E);
+ Visit(E);
+ }
+
+ void HandleValue(Expr *E) {
+ E = E->IgnoreParens();
+
+ if (MemberExpr* ME = dyn_cast<MemberExpr>(E)) {
+ if (isa<EnumConstantDecl>(ME->getMemberDecl()))
+ return;
+ Expr* Base = E;
+ while (isa<MemberExpr>(Base)) {
+ ME = dyn_cast<MemberExpr>(Base);
+ if (VarDecl *VarD = dyn_cast<VarDecl>(ME->getMemberDecl()))
+ if (VarD->hasGlobalStorage())
+ return;
+ Base = ME->getBase();
+ }
- if (RhsField == LhsField) {
- // Initializing a field with itself. Throw a warning.
- // But wait; there are exceptions!
- // Exception #1: The field may not belong to this record.
- // e.g. Foo(const Foo& rhs) : A(rhs.A) {}
- const Expr *base = ME->getBase();
- if (base != NULL && !isa<CXXThisExpr>(base->IgnoreParenCasts())) {
- // Even though the field matches, it does not belong to this record.
- return false;
+ if (VD == ME->getMemberDecl() && isa<CXXThisExpr>(Base)) {
+ S.Diag(ME->getExprLoc(), diag::warn_field_is_uninit);
+ return;
+ }
+ }
+
+ if (ConditionalOperator *CO = dyn_cast<ConditionalOperator>(E)) {
+ HandleValue(CO->getTrueExpr());
+ HandleValue(CO->getFalseExpr());
+ return;
+ }
+
+ if (BinaryConditionalOperator *BCO =
+ dyn_cast<BinaryConditionalOperator>(E)) {
+ HandleValue(BCO->getCommon());
+ HandleValue(BCO->getFalseExpr());
+ return;
+ }
+
+ if (BinaryOperator *BO = dyn_cast<BinaryOperator>(E)) {
+ switch (BO->getOpcode()) {
+ default:
+ return;
+ case(BO_PtrMemD):
+ case(BO_PtrMemI):
+ HandleValue(BO->getLHS());
+ return;
+ case(BO_Comma):
+ HandleValue(BO->getRHS());
+ return;
+ }
}
- // None of the exceptions triggered; return true to indicate an
- // uninitialized field was used.
- *L = ME->getMemberLoc();
- return true;
}
- } else if (isa<UnaryExprOrTypeTraitExpr>(S)) {
- // sizeof/alignof doesn't reference contents, do not warn.
- return false;
- } else if (const UnaryOperator *UOE = dyn_cast<UnaryOperator>(S)) {
- // address-of doesn't reference contents (the pointer may be dereferenced
- // in the same expression but it would be rare; and weird).
- if (UOE->getOpcode() == UO_AddrOf)
- return false;
- }
- for (Stmt::const_child_range it = S->children(); it; ++it) {
- if (!*it) {
- // An expression such as 'member(arg ?: "")' may trigger this.
- continue;
+
+ void VisitImplicitCastExpr(ImplicitCastExpr *E) {
+ if (E->getCastKind() == CK_LValueToRValue)
+ HandleValue(E->getSubExpr());
+
+ Inherited::VisitImplicitCastExpr(E);
}
- if (InitExprContainsUninitializedFields(*it, LhsField, L))
- return true;
+
+ void VisitCXXMemberCallExpr(CXXMemberCallExpr *E) {
+ Expr *Callee = E->getCallee();
+ if (isa<MemberExpr>(Callee))
+ HandleValue(Callee);
+
+ Inherited::VisitCXXMemberCallExpr(E);
+ }
+ };
+ static void CheckInitExprContainsUninitializedFields(Sema &S, Expr *E,
+ ValueDecl *VD) {
+ UninitializedFieldVisitor(S, VD).HandleExpr(E);
}
- return false;
-}
+} // namespace
MemInitResult
Sema::BuildMemberInitializer(ValueDecl *Member, Expr *Init,
@@ -2153,18 +2176,16 @@
}
}
- for (unsigned i = 0; i < NumArgs; ++i) {
- SourceLocation L;
- if (InitExprContainsUninitializedFields(Args[i], Member, &L)) {
- // FIXME: Return true in the case when other fields are used before being
+ if (getDiagnostics().getDiagnosticLevel(diag::warn_field_is_uninit, IdLoc)
+ != DiagnosticsEngine::Ignored)
+ for (unsigned i = 0; i < NumArgs; ++i)
+ // FIXME: Warn about the case when other fields are used before being
// uninitialized. For example, let this field be the i'th field. When
// initializing the i'th field, throw a warning if any of the >= i'th
// fields are used, as they are not yet initialized.
// Right now we are only handling the case where the i'th field uses
// itself in its initializer.
- Diag(L, diag::warn_field_is_uninit);
- }
- }
+ CheckInitExprContainsUninitializedFields(*this, Args[i], Member);
SourceRange InitRange = Init->getSourceRange();
Modified: cfe/trunk/test/SemaCXX/constructor-initializer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constructor-initializer.cpp?rev=158477&r1=158476&r2=158477&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/constructor-initializer.cpp (original)
+++ cfe/trunk/test/SemaCXX/constructor-initializer.cpp Thu Jun 14 18:11:34 2012
@@ -126,21 +126,24 @@
// A silly class used to demonstrate field-is-uninitialized in constructors with
// multiple params.
+int IntParam(int i) { return 0; };
class TwoInOne { public: TwoInOne(TwoInOne a, TwoInOne b) {} };
class InitializeUsingSelfTest {
bool A;
char* B;
int C;
TwoInOne D;
- InitializeUsingSelfTest(int E)
+ int E;
+ InitializeUsingSelfTest(int F)
: A(A), // expected-warning {{field is uninitialized when used here}}
B((((B)))), // expected-warning {{field is uninitialized when used here}}
C(A && InitializeUsingSelfTest::C), // expected-warning {{field is uninitialized when used here}}
D(D, // expected-warning {{field is uninitialized when used here}}
- D) {} // expected-warning {{field is uninitialized when used here}}
+ D), // expected-warning {{field is uninitialized when used here}}
+ E(IntParam(E)) {} // expected-warning {{field is uninitialized when used here}}
};
-int IntWrapper(int i) { return 0; };
+int IntWrapper(int &i) { return 0; };
class InitializeUsingSelfExceptions {
int A;
int B;
Modified: cfe/trunk/test/SemaCXX/uninitialized.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/uninitialized.cpp?rev=158477&r1=158476&r2=158477&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/uninitialized.cpp (original)
+++ cfe/trunk/test/SemaCXX/uninitialized.cpp Thu Jun 14 18:11:34 2012
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -Wall -Wuninitialized -std=c++11 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wall -Wuninitialized -Wno-unused-value -std=c++11 -verify %s
int foo(int x);
int bar(int* x);
@@ -152,9 +152,9 @@
S(bool (*)[1]) : x(x) {} // expected-warning {{field is uninitialized when used here}}
S(bool (*)[2]) : x(x + 1) {} // expected-warning {{field is uninitialized when used here}}
- S(bool (*)[3]) : x(x + x) {} // expected-warning {{field is uninitialized when used here}}
+ S(bool (*)[3]) : x(x + x) {} // expected-warning 2{{field is uninitialized when used here}}
S(bool (*)[4]) : x(static_cast<long>(x) + 1) {} // expected-warning {{field is uninitialized when used here}}
- S(bool (*)[5]) : x(foo(x)) {} // FIXME: This should warn!
+ S(bool (*)[5]) : x(foo(x)) {} // expected-warning {{field is uninitialized when used here}}
// These don't actually require the value of x and so shouldn't warn.
S(char (*)[1]) : x(sizeof(x)) {} // rdar://8610363
@@ -213,3 +213,83 @@
auto f1 = [] (int x, int y) { int z; return x + y + z; }; // expected-warning{{variable 'z' is uninitialized when used here}} expected-note {{initialize the variable 'z' to silence this warning}}
return f1(1, 2);
}
+
+namespace {
+ struct A {
+ enum { A1 };
+ static int A2() {return 5;}
+ int A3;
+ int A4() { return 5;}
+ };
+
+ struct B {
+ A a;
+ };
+
+ struct C {
+ C() {}
+ C(int x) {}
+ static A a;
+ B b;
+ };
+ A C::a = A();
+
+ // Accessing non-static members will give a warning.
+ struct D {
+ C c;
+ D(char (*)[1]) : c(c.b.a.A1) {}
+ D(char (*)[2]) : c(c.b.a.A2()) {}
+ D(char (*)[3]) : c(c.b.a.A3) {} // expected-warning {{field is uninitialized when used here}}
+ D(char (*)[4]) : c(c.b.a.A4()) {} // expected-warning {{field is uninitialized when used here}}
+
+ // c::a is static, so it is already initialized
+ D(char (*)[5]) : c(c.a.A1) {}
+ D(char (*)[6]) : c(c.a.A2()) {}
+ D(char (*)[7]) : c(c.a.A3) {}
+ D(char (*)[8]) : c(c.a.A4()) {}
+ };
+
+ struct E {
+ int a, b, c;
+ E(char (*)[1]) : a(a ? b : c) {} // expected-warning {{field is uninitialized when used here}}
+ E(char (*)[2]) : a(b ? a : a) {} // expected-warning 2{{field is uninitialized when used here}}
+ E(char (*)[3]) : a(b ? (a) : c) {} // expected-warning {{field is uninitialized when used here}}
+ E(char (*)[4]) : a(b ? c : (a+c)) {} // expected-warning {{field is uninitialized when used here}}
+ E(char (*)[5]) : a(b ? c : b) {}
+
+ E(char (*)[6]) : a(a ?: a) {} // expected-warning 2{{field is uninitialized when used here}}
+ E(char (*)[7]) : a(b ?: a) {} // expected-warning {{field is uninitialized when used here}}
+ E(char (*)[8]) : a(a ?: c) {} // expected-warning {{field is uninitialized when used here}}
+ E(char (*)[9]) : a(b ?: c) {}
+
+ E(char (*)[10]) : a((a, a, b)) {}
+ E(char (*)[11]) : a((c + a, a + 1, b)) {} // expected-warning 2{{field is uninitialized when used here}}
+ E(char (*)[12]) : a((b + c, c, a)) {} // expected-warning {{field is uninitialized when used here}}
+ E(char (*)[13]) : a((a, a, a, a)) {} // expected-warning {{field is uninitialized when used here}}
+ E(char (*)[14]) : a((b, c, c)) {}
+ };
+
+ struct F {
+ int a;
+ F* f;
+ F(int) {}
+ F() {}
+ };
+
+ int F::*ptr = &F::a;
+ F* F::*f_ptr = &F::f;
+ struct G {
+ F f1, f2;
+ F *f3, *f4;
+ G(char (*)[1]) : f1(f1) {} // expected-warning {{field is uninitialized when used here}}
+ G(char (*)[2]) : f2(f1) {}
+ G(char (*)[3]) : f2(F()) {}
+
+ G(char (*)[4]) : f1(f1.*ptr) {} // expected-warning {{field is uninitialized when used here}}
+ G(char (*)[5]) : f2(f1.*ptr) {}
+
+ G(char (*)[6]) : f3(f3) {} // expected-warning {{field is uninitialized when used here}}
+ G(char (*)[7]) : f3(f3->*f_ptr) {} // expected-warning {{field is uninitialized when used here}}
+ G(char (*)[8]) : f3(new F(f3->*ptr)) {} // expected-warning {{field is uninitialized when used here}}
+ };
+}
More information about the cfe-commits
mailing list