From: Ted Kremenek <kremenek@apple.com>
Date: Mon, 31 Mar 2008 15:02:58 +0000 (+0000)
Subject: Added path-sensitive check for return statements that return the address
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=02737ed29d7fff2206f7c7ee958cdf0665e35542;p=clang

Added path-sensitive check for return statements that return the address
of a stack variable.  This is the path-sensitive version of a check that
is already done during semantic analysis.


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

diff --git a/include/clang/Analysis/PathSensitive/GRExprEngine.h b/include/clang/Analysis/PathSensitive/GRExprEngine.h
index b04469efc3..f286816f38 100644
--- a/include/clang/Analysis/PathSensitive/GRExprEngine.h
+++ b/include/clang/Analysis/PathSensitive/GRExprEngine.h
@@ -86,8 +86,12 @@ protected:
   typedef llvm::DenseMap<NodeTy*, Expr*> UndefArgsTy;
   typedef llvm::SmallPtrSet<NodeTy*,2> BadDividesTy;
   typedef llvm::SmallPtrSet<NodeTy*,2> NoReturnCallsTy;  
-  typedef llvm::SmallPtrSet<NodeTy*,2> UndefResultsTy;  
+  typedef llvm::SmallPtrSet<NodeTy*,2> UndefResultsTy;
+  typedef llvm::SmallPtrSet<NodeTy*,2> RetsStackAddrTy;
 
+  /// RetsStackAddr - Nodes in the ExplodedGraph that result from returning
+  ///  the address of a stack variable.
+  RetsStackAddrTy RetsStackAddr;
   
   /// UndefBranches - Nodes in the ExplodedGraph that result from
   ///  taking a branch based on an undefined value.
@@ -179,6 +183,10 @@ public:
   ///  in the ExplodedGraph.
   ValueState* getInitialState();
   
+  bool isRetStackAddr(const NodeTy* N) const {
+    return N->isSink() && RetsStackAddr.count(const_cast<NodeTy*>(N)) != 0;
+  }
+  
   bool isUndefControlFlow(const NodeTy* N) const {
     return N->isSink() && UndefBranches.count(const_cast<NodeTy*>(N)) != 0;
   }
@@ -229,6 +237,10 @@ public:
     return N->isSink() && UndefReceivers.count(const_cast<NodeTy*>(N)) != 0;
   }
   
+  typedef RetsStackAddrTy::iterator ret_stackaddr_iterator;
+  ret_stackaddr_iterator ret_stackaddr_begin() { return RetsStackAddr.begin(); }
+  ret_stackaddr_iterator ret_stackaddr_end() { return RetsStackAddr.end(); }  
+  
   typedef UndefBranchesTy::iterator undef_branch_iterator;
   undef_branch_iterator undef_branches_begin() { return UndefBranches.begin(); }
   undef_branch_iterator undef_branches_end() { return UndefBranches.end(); }  
@@ -476,6 +488,9 @@ protected:
   /// VisitLogicalExpr - Transfer function logic for '&&', '||'
   void VisitLogicalExpr(BinaryOperator* B, NodeTy* Pred, NodeSet& Dst);
   
+  /// VisitReturnStmt - Transfer function logic for return statements.
+  void VisitReturnStmt(ReturnStmt* R, NodeTy* Pred, NodeSet& Dst);
+  
   /// VisitSizeOfAlignOfTypeExpr - Transfer function for sizeof(type).
   void VisitSizeOfAlignOfTypeExpr(SizeOfAlignOfTypeExpr* Ex, NodeTy* Pred,
                                   NodeSet& Dst);
diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp
index b70e21977b..bf0d5ea395 100644
--- a/lib/Analysis/GRExprEngine.cpp
+++ b/lib/Analysis/GRExprEngine.cpp
@@ -1190,6 +1190,50 @@ void GRExprEngine::VisitObjCMessageExprDispatchHelper(ObjCMessageExpr* ME,
   MakeNode(Dst, ME, Pred, St);
 }
 
+void GRExprEngine::VisitReturnStmt(ReturnStmt* S, NodeTy* Pred, NodeSet& Dst) {
+
+  Expr* R = S->getRetValue();
+  
+  if (!R) {
+    Dst.Add(Pred);
+    return;
+  }
+  
+  QualType T = R->getType();
+  
+  if (T->isPointerType() || T->isReferenceType()) {
+    
+    // Check if any of the return values return the address of a stack variable.
+    
+    NodeSet Tmp;
+    Visit(R, Pred, Tmp);
+    
+    for (NodeSet::iterator I=Tmp.begin(), E=Tmp.end(); I!=E; ++I) {
+      RVal X = GetRVal((*I)->getState(), R);
+
+      if (isa<lval::DeclVal>(X)) {
+        
+        if (cast<lval::DeclVal>(X).getDecl()->hasLocalStorage()) {
+        
+          // Create a special node representing the v
+          
+          NodeTy* RetStackNode = Builder->generateNode(S, GetState(*I), *I);
+          
+          if (RetStackNode) {
+            RetStackNode->markAsSink();
+            RetsStackAddr.insert(RetStackNode);
+          }
+          
+          continue;
+        }
+      }
+      
+      Dst.Add(*I);
+    }
+  }
+  else
+    Visit(R, Pred, Dst);
+}
 
 void GRExprEngine::VisitBinaryOperator(BinaryOperator* B,
                                        GRExprEngine::NodeTy* Pred,
@@ -1625,14 +1669,8 @@ void GRExprEngine::Visit(Stmt* S, NodeTy* Pred, NodeSet& Dst) {
       //  that users can quickly query what was the state at the
       //  exit points of a function.
       
-    case Stmt::ReturnStmtClass: {
-      if (Expr* R = cast<ReturnStmt>(S)->getRetValue())
-        Visit(R, Pred, Dst);
-      else
-        Dst.Add(Pred);
-      
-      break;
-    }
+    case Stmt::ReturnStmtClass:
+      VisitReturnStmt(cast<ReturnStmt>(S), Pred, Dst); break;
       
     case Stmt::UnaryOperatorClass: {
       UnaryOperator* U = cast<UnaryOperator>(S);
diff --git a/lib/Analysis/GRSimpleVals.cpp b/lib/Analysis/GRSimpleVals.cpp
index 94cedc0352..697d256119 100644
--- a/lib/Analysis/GRSimpleVals.cpp
+++ b/lib/Analysis/GRSimpleVals.cpp
@@ -166,6 +166,11 @@ unsigned RunGRSimpleVals(CFG& cfg, Decl& CD, ASTContext& Ctx,
               CheckerState->undef_receivers_begin(),
               CheckerState->undef_receivers_end(),
               "Receiver in message expression is an uninitialized value.");
+  
+  EmitWarning(Diag, SrcMgr,
+              CheckerState->ret_stackaddr_begin(),
+              CheckerState->ret_stackaddr_end(),
+              "Address of stack-allocated variable returned.");
 
   FoundationCheck.get()->ReportResults(Diag);
 #ifndef NDEBUG
diff --git a/test/Analysis/stack-addr-ps.c b/test/Analysis/stack-addr-ps.c
new file mode 100644
index 0000000000..55c542cebf
--- /dev/null
+++ b/test/Analysis/stack-addr-ps.c
@@ -0,0 +1,21 @@
+// RUN: clang -grsimple -verify %s
+
+int* f1() {
+  int x = 0;
+  return &x; // expected-warning{{Address of stack-allocated variable returned.}} expected-warning{{address of stack memory associated with local variable 'x' returned}}
+}
+
+int* f2(int y) {
+  return &y;  // expected-warning{{Address of stack-allocated variable returned.}} expected-warning{{address of stack memory associated with local variable 'y' returned}}
+}
+
+int* f3(int x, int *y) {
+  int w = 0;
+  
+  if (x)
+    y = &w;
+    
+  return y; // expected-warning{{Address of stack-allocated variable returned.}}
+}
+
+