[cfe-commits] r139075 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDeclCXX.cpp

[Chandler Carruth]

Author: chandlerc
Date: Fri Sep  2 20:14:15 2011
New Revision: 139075

URL: http://llvm.org/viewvc/llvm-project?rev=139075&view=rev
Log:
Add a simple new warning to catch blatantly dangling pointer and
reference members of classes. We've had several bugs reported because of
this, and there's no reason not to flag it right away in the compiler.

Comments especially welcome on the strategy for implementing this
warning (IE, what should trigger this?) and on the text of the warning
itself.

I'm going to extend this to cover obvious cases with temporaries and
beef up the test cases some in subsequent patches. I'll then run it over
a large codebase and make sure its not misbehaving before I add it to
-Wall or turn it on by default. I think this one might be a good
candidate for on by default.

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/Sema/SemaDeclCXX.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=139075&r1=139074&r2=139075&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Sep  2 20:14:15 2011
@@ -4354,6 +4354,16 @@
 def note_ref_var_local_bind : Note<
   "binding reference variable %0 here">;
 
+// Check for initializing a member variable with the address or a reference to
+// a constructor parameter.
+def warn_bind_ref_member_to_parameter : Warning<
+  "binding reference member %0 to stack allocated parameter %1">,
+  InGroup<DiagGroup<"dangling-field">>, DefaultIgnore;
+def warn_init_ptr_member_to_parameter_addr : Warning<
+  "initializing pointer member %0 with the stack address of parameter %1">,
+  InGroup<DiagGroup<"dangling-field">>, DefaultIgnore;
+def note_ref_or_ptr_member_declared_here : Note<
+  "%select{reference|pointer}0 member declared here">;
 
 // For non-floating point, expressions of the form x == x or x != x
 // should result in a warning, since these always evaluate to a constant.

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=139075&r1=139074&r2=139075&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Fri Sep  2 20:14:15 2011
@@ -1498,6 +1498,53 @@
                               LParenLoc, RParenLoc, ClassDecl, EllipsisLoc);
 }
 
+/// Checks a member initializer expression for cases where reference (or
+/// pointer) members are bound to by-value parameters (or their addresses).
+/// FIXME: We should also flag temporaries here.
+static void CheckForDanglingReferenceOrPointer(Sema &S, ValueDecl *Member,
+                                               Expr *Init,
+                                               SourceLocation IdLoc) {
+  QualType MemberTy = Member->getType();
+
+  // We only handle pointers and references currently.
+  // FIXME: Would this be relevant for ObjC object pointers? Or block pointers?
+  if (!MemberTy->isReferenceType() && !MemberTy->isPointerType())
+    return;
+
+  const bool IsPointer = MemberTy->isPointerType();
+  if (IsPointer) {
+    if (const UnaryOperator *Op
+          = dyn_cast<UnaryOperator>(Init->IgnoreParenImpCasts())) {
+      // The only case we're worried about with pointers requires taking the
+      // address.
+      if (Op->getOpcode() != UO_AddrOf)
+        return;
+
+      Init = Op->getSubExpr();
+    } else {
+      // We only handle address-of expression initializers for pointers.
+      return;
+    }
+  }
+
+  // We only warn when referring to a non-reference declaration.
+  const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Init->IgnoreParenCasts());
+  if (!DRE)
+    return;
+
+  if (const ParmVarDecl *Parameter = dyn_cast<ParmVarDecl>(DRE->getDecl())) {
+    if (Parameter->getType()->isReferenceType())
+      return;
+
+    S.Diag(Init->getExprLoc(),
+           IsPointer ? diag::warn_init_ptr_member_to_parameter_addr
+                     : diag::warn_bind_ref_member_to_parameter)
+      << Member << Parameter << Init->getSourceRange();
+    S.Diag(Member->getLocation(), diag::note_ref_or_ptr_member_declared_here)
+      << (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
@@ -1641,12 +1688,14 @@
     // of the information that we have about the member
     // initializer. However, deconstructing the ASTs is a dicey process,
     // and this approach is far more likely to get the corner cases right.
-    if (CurContext->isDependentContext())
+    if (CurContext->isDependentContext()) {
       Init = new (Context) ParenListExpr(
           Context, LParenLoc, Args, NumArgs, RParenLoc,
           Member->getType().getNonReferenceType());
-    else
+    } else {
       Init = MemberInit.get();
+      CheckForDanglingReferenceOrPointer(*this, Member, Init, IdLoc);
+    }
   }
 
   if (DirectMember) {