Follow through references to catch returned stack addresses, local blocks, label addresses or references to temporaries, e.g:

const int& g2() {
  int s1;
  int &s2 = s1; // expected-note {{binding reference variable 's2' here}}
  return s2; // expected-warning {{reference to stack memory associated with local variable 's1' returned}}
}

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@120483 91177308-0d34-0410-b5e6-96231b3b80d8

1 files changed, 127 insertions, 50 deletions

diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index ed45de53f2..46c5967e5b 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -1759,8 +1759,8 @@ void Sema::CheckFormatString(const StringLiteral *FExpr,
 
 //===--- CHECK: Return Address of Stack Variable --------------------------===//
 
-static DeclRefExpr* EvalVal(Expr *E);
-static DeclRefExpr* EvalAddr(Expr* E);
+static Expr *EvalVal(Expr *E, llvm::SmallVectorImpl<DeclRefExpr *> &refVars);
+static Expr *EvalAddr(Expr* E, llvm::SmallVectorImpl<DeclRefExpr *> &refVars);
 
 /// CheckReturnStackAddr - Check if a return statement returns the address
 ///   of a stack variable.
@@ -1768,45 +1768,79 @@ void
 Sema::CheckReturnStackAddr(Expr *RetValExp, QualType lhsType,
                            SourceLocation ReturnLoc) {
 
-  // Perform checking for returned stack addresses.
-  if (lhsType->isPointerType() || lhsType->isBlockPointerType()) {
-    if (DeclRefExpr *DR = EvalAddr(RetValExp))
-      Diag(DR->getLocStart(), diag::warn_ret_stack_addr)
-       << DR->getDecl()->getDeclName() << RetValExp->getSourceRange();
-
-    // Skip over implicit cast expressions when checking for block expressions.
-    RetValExp = RetValExp->IgnoreParenCasts();
+  Expr *stackE = 0;
+  llvm::SmallVector<DeclRefExpr *, 8> refVars;
 
-    if (BlockExpr *C = dyn_cast<BlockExpr>(RetValExp))
-      if (C->hasBlockDeclRefExprs())
-        Diag(C->getLocStart(), diag::err_ret_local_block)
-          << C->getSourceRange();
+  // Perform checking for returned stack addresses, local blocks,
+  // label addresses or references to temporaries.
+  if (lhsType->isPointerType() || lhsType->isBlockPointerType()) {
+    stackE = EvalAddr(RetValExp, refVars);
+  } else if (lhsType->isReferenceType()) {
+    stackE = EvalVal(RetValExp, refVars);
+  }
 
-    if (AddrLabelExpr *ALE = dyn_cast<AddrLabelExpr>(RetValExp))
-      Diag(ALE->getLocStart(), diag::warn_ret_addr_label)
-        << ALE->getSourceRange();
+  if (stackE == 0)
+    return; // Nothing suspicious was found.
 
-  } else if (lhsType->isReferenceType()) {
-    // Perform checking for stack values returned by reference.
-    // Check for a reference to the stack
-    if (DeclRefExpr *DR = EvalVal(RetValExp))
-      Diag(DR->getLocStart(), diag::warn_ret_stack_ref)
-        << DR->getDecl()->getDeclName() << RetValExp->getSourceRange();
+  SourceLocation diagLoc;
+  SourceRange diagRange;
+  if (refVars.empty()) {
+    diagLoc = stackE->getLocStart();
+    diagRange = stackE->getSourceRange();
+  } else {
+    // We followed through a reference variable. 'stackE' contains the
+    // problematic expression but we will warn at the return statement pointing
+    // at the reference variable. We will later display the "trail" of
+    // reference variables using notes.
+    diagLoc = refVars[0]->getLocStart();
+    diagRange = refVars[0]->getSourceRange();
+  }
+
+  if (DeclRefExpr *DR = dyn_cast<DeclRefExpr>(stackE)) { //address of local var.
+    Diag(diagLoc, lhsType->isReferenceType() ? diag::warn_ret_stack_ref
+                                             : diag::warn_ret_stack_addr)
+     << DR->getDecl()->getDeclName() << diagRange;
+  } else if (isa<BlockExpr>(stackE)) { // local block.
+    Diag(diagLoc, diag::err_ret_local_block) << diagRange;
+  } else if (isa<AddrLabelExpr>(stackE)) { // address of label.
+    Diag(diagLoc, diag::warn_ret_addr_label) << diagRange;
+  } else { // local temporary.
+    Diag(diagLoc, lhsType->isReferenceType() ? diag::warn_ret_local_temp_ref
+                                             : diag::warn_ret_local_temp_addr)
+     << diagRange;
+  }
+
+  // Display the "trail" of reference variables that we followed until we
+  // found the problematic expression using notes.
+  for (unsigned i = 0, e = refVars.size(); i != e; ++i) {
+    VarDecl *VD = cast<VarDecl>(refVars[i]->getDecl());
+    // If this var binds to another reference var, show the range of the next
+    // var, otherwise the var binds to the problematic expression, in which case
+    // show the range of the expression.
+    SourceRange range = (i < e-1) ? refVars[i+1]->getSourceRange()
+                                  : stackE->getSourceRange();
+    Diag(VD->getLocation(), diag::note_ref_var_local_bind)
+      << VD->getDeclName() << range;
   }
 }
 
 /// EvalAddr - EvalAddr and EvalVal are mutually recursive functions that
 ///  check if the expression in a return statement evaluates to an address
-///  to a location on the stack.  The recursion is used to traverse the
+///  to a location on the stack, a local block, an address of a label, or a
+///  reference to local temporary. The recursion is used to traverse the
 ///  AST of the return expression, with recursion backtracking when we
-///  encounter a subexpression that (1) clearly does not lead to the address
-///  of a stack variable or (2) is something we cannot determine leads to
-///  the address of a stack variable based on such local checking.
+///  encounter a subexpression that (1) clearly does not lead to one of the
+///  above problematic expressions (2) is something we cannot determine leads to
+///  a problematic expression based on such local checking.
+///
+///  Both EvalAddr and EvalVal follow through reference variables to evaluate
+///  the expression that they point to. Such variables are added to the
+///  'refVars' vector so that we know what the reference variable "trail" was.
 ///
 ///  EvalAddr processes expressions that are pointers that are used as
 ///  references (and not L-values).  EvalVal handles all other values.
-///  At the base case of the recursion is a check for a DeclRefExpr* in
-///  the refers to a stack variable.
+///  At the base case of the recursion is a check for the above problematic
+///  expressions.
 ///
 ///  This implementation handles:
 ///
@@ -1816,7 +1850,10 @@ Sema::CheckReturnStackAddr(Expr *RetValExp, QualType lhsType,
 ///   * arbitrary interplay between "&" and "*" operators
 ///   * pointer arithmetic from an address of a stack variable
 ///   * taking the address of an array element where the array is on the stack
-static DeclRefExpr* EvalAddr(Expr *E) {
+static Expr *EvalAddr(Expr *E, llvm::SmallVectorImpl<DeclRefExpr *> &refVars) {
+  if (E->isTypeDependent())
+      return NULL;
+
   // We should only be called for evaluating pointer expressions.
   assert((E->getType()->isAnyPointerType() ||
           E->getType()->isBlockPointerType() ||
@@ -1829,7 +1866,23 @@ static DeclRefExpr* EvalAddr(Expr *E) {
   switch (E->getStmtClass()) {
   case Stmt::ParenExprClass:
     // Ignore parentheses.
-    return EvalAddr(cast<ParenExpr>(E)->getSubExpr());
+    return EvalAddr(cast<ParenExpr>(E)->getSubExpr(), refVars);
+
+  case Stmt::DeclRefExprClass: {
+    DeclRefExpr *DR = cast<DeclRefExpr>(E);
+
+    if (VarDecl *V = dyn_cast<VarDecl>(DR->getDecl()))
+      // If this is a reference variable, follow through to the expression that
+      // it points to.
+      if (V->hasLocalStorage() &&
+          V->getType()->isReferenceType() && V->hasInit()) {
+        // Add the reference variable to the "trail".
+        refVars.push_back(DR);
+        return EvalAddr(V->getInit(), refVars);
+      }
+
+    return NULL;
+  }
 
   case Stmt::UnaryOperatorClass: {
     // The only unary operator that make sense to handle here
@@ -1837,7 +1890,7 @@ static DeclRefExpr* EvalAddr(Expr *E) {
     UnaryOperator *U = cast<UnaryOperator>(E);
 
     if (U->getOpcode() == UO_AddrOf)
-      return EvalVal(U->getSubExpr());
+      return EvalVal(U->getSubExpr(), refVars);
     else
       return NULL;
   }
@@ -1858,7 +1911,7 @@ static DeclRefExpr* EvalAddr(Expr *E) {
     if (!Base->getType()->isPointerType()) Base = B->getRHS();
 
     assert (Base->getType()->isPointerType());
-    return EvalAddr(Base);
+    return EvalAddr(Base, refVars);
   }
 
   // For conditional operators we need to see if either the LHS or RHS are
@@ -1870,7 +1923,7 @@ static DeclRefExpr* EvalAddr(Expr *E) {
     if (Expr *lhsExpr = C->getLHS()) {
     // In C++, we can have a throw-expression, which has 'void' type.
       if (!lhsExpr->getType()->isVoidType())
-        if (DeclRefExpr* LHS = EvalAddr(lhsExpr))
+        if (Expr* LHS = EvalAddr(lhsExpr, refVars))
           return LHS;
     }
 
@@ -1878,8 +1931,16 @@ static DeclRefExpr* EvalAddr(Expr *E) {
     if (C->getRHS()->getType()->isVoidType())
       return NULL;
 
-    return EvalAddr(C->getRHS());
+    return EvalAddr(C->getRHS(), refVars);
   }
+  
+  case Stmt::BlockExprClass:
+    if (cast<BlockExpr>(E)->hasBlockDeclRefExprs())
+      return E; // local block.
+    return NULL;
+
+  case Stmt::AddrLabelExprClass:
+    return E; // address of label.
 
   // For casts, we need to handle conversions from arrays to
   // pointer values, and pointer-to-pointer conversions.
@@ -1892,9 +1953,9 @@ static DeclRefExpr* EvalAddr(Expr *E) {
     if (SubExpr->getType()->isPointerType() ||
         SubExpr->getType()->isBlockPointerType() ||
         SubExpr->getType()->isObjCQualifiedIdType())
-      return EvalAddr(SubExpr);
+      return EvalAddr(SubExpr, refVars);
     else if (T->isArrayType())
-      return EvalVal(SubExpr);
+      return EvalVal(SubExpr, refVars);
     else
       return 0;
   }
@@ -1913,7 +1974,7 @@ static DeclRefExpr* EvalAddr(Expr *E) {
   case Stmt::CXXReinterpretCastExprClass: {
       Expr *S = cast<CXXNamedCastExpr>(E)->getSubExpr();
       if (S->getType()->isPointerType() || S->getType()->isBlockPointerType())
-        return EvalAddr(S);
+        return EvalAddr(S, refVars);
       else
         return NULL;
   }
@@ -1927,7 +1988,7 @@ static DeclRefExpr* EvalAddr(Expr *E) {
 
 ///  EvalVal - This function is complements EvalAddr in the mutual recursion.
 ///   See the comments for EvalAddr for more details.
-static DeclRefExpr* EvalVal(Expr *E) {
+static Expr *EvalVal(Expr *E, llvm::SmallVectorImpl<DeclRefExpr *> &refVars) {
 do {
   // We should only be called for evaluating non-pointer expressions, or
   // expressions with a pointer type that are not used as references but instead
@@ -1947,13 +2008,24 @@ do {
   }
 
   case Stmt::DeclRefExprClass: {
-    // DeclRefExpr: the base case.  When we hit a DeclRefExpr we are looking
-    //  at code that refers to a variable's name.  We check if it has local
-    //  storage within the function, and if so, return the expression.
+    // When we hit a DeclRefExpr we are looking at code that refers to a
+    // variable's name. If it's not a reference variable we check if it has
+    // local storage within the function, and if so, return the expression.
     DeclRefExpr *DR = cast<DeclRefExpr>(E);
 
     if (VarDecl *V = dyn_cast<VarDecl>(DR->getDecl()))
-      if (V->hasLocalStorage() && !V->getType()->isReferenceType()) return DR;
+      if (V->hasLocalStorage()) {
+        if (!V->getType()->isReferenceType())
+          return DR;
+
+        // Reference variable, follow through to the expression that
+        // it points to.
+        if (V->hasInit()) {
+          // Add the reference variable to the "trail".
+          refVars.push_back(DR);
+          return EvalVal(V->getInit(), refVars);
+        }
+      }
 
     return NULL;
   }
@@ -1971,7 +2043,7 @@ do {
     UnaryOperator *U = cast<UnaryOperator>(E);
 
     if (U->getOpcode() == UO_Deref)
-      return EvalAddr(U->getSubExpr());
+      return EvalAddr(U->getSubExpr(), refVars);
 
     return NULL;
   }
@@ -1980,20 +2052,20 @@ do {
     // Array subscripts are potential references to data on the stack.  We
     // retrieve the DeclRefExpr* for the array variable if it indeed
     // has local storage.
-    return EvalAddr(cast<ArraySubscriptExpr>(E)->getBase());
+    return EvalAddr(cast<ArraySubscriptExpr>(E)->getBase(), refVars);
   }
 
   case Stmt::ConditionalOperatorClass: {
     // For conditional operators we need to see if either the LHS or RHS are
-    // non-NULL DeclRefExpr's.  If one is non-NULL, we return it.
+    // non-NULL Expr's.  If one is non-NULL, we return it.
     ConditionalOperator *C = cast<ConditionalOperator>(E);
 
     // Handle the GNU extension for missing LHS.
     if (Expr *lhsExpr = C->getLHS())
-      if (DeclRefExpr *LHS = EvalVal(lhsExpr))
+      if (Expr *LHS = EvalVal(lhsExpr, refVars))
         return LHS;
 
-    return EvalVal(C->getRHS());
+    return EvalVal(C->getRHS(), refVars);
   }
 
   // Accesses to members are potential references to data on the stack.
@@ -2009,11 +2081,16 @@ do {
     if (M->getMemberDecl()->getType()->isReferenceType())
       return NULL;
 
-    return EvalVal(M->getBase());
+    return EvalVal(M->getBase(), refVars);
   }
 
-  // Everything else: we simply don't reason about them.
   default:
+    // Check that we don't return or take the address of a reference to a
+    // temporary. This is only useful in C++.
+    if (!E->isTypeDependent() && E->isRValue())
+      return E;
+
+    // Everything else: we simply don't reason about them.
     return NULL;
   }
 } while (true);