Consumed Analysis: Allow parameters that are passed by non-const reference

to be treated as return values, and marked with the "returned_typestate"
attribute.  Patch by chris.wailes@gmail.com; reviewed by delesley@google.com.

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

diff --git a/include/clang/Analysis/Analyses/Consumed.h b/include/clang/Analysis/Analyses/Consumed.h
index f097e3da7cf9182716192ffdca1cbc34b7f90446..69bd022ba239d51915af37dec9c0f73d4f3f6e50 100644 (file)
--- a/include/clang/Analysis/Analyses/Consumed.h
+++ b/include/clang/Analysis/Analyses/Consumed.h
@@ -59,6 +59,20 @@ namespace consumed {
     virtual void warnLoopStateMismatch(SourceLocation Loc,
                                        StringRef VariableName) {}
     
+    /// \brief Warn about parameter typestate mismatches upon return.
+    ///
+    /// \param Loc -- The SourceLocation of the return statement.
+    ///
+    /// \param ExpectedState -- The state the return value was expected to be
+    /// in.
+    ///
+    /// \param ObservedState -- The state the return value was observed to be
+    /// in.
+    virtual void warnParamReturnTypestateMismatch(SourceLocation Loc,
+                                                  StringRef VariableName,
+                                                  StringRef ExpectedState,
+                                                  StringRef ObservedState) {};
+    
     // FIXME: This can be removed when the attr propagation fix for templated
     //        classes lands.
     /// \brief Warn about return typestates set for unconsumable types.
@@ -70,7 +84,14 @@ namespace consumed {
                                                         StringRef TypeName) {}
     
     /// \brief Warn about return typestate mismatches.
+    ///
     /// \param Loc -- The SourceLocation of the return statement.
+    ///
+    /// \param ExpectedState -- The state the return value was expected to be
+    /// in.
+    ///
+    /// \param ObservedState -- The state the return value was observed to be
+    /// in.
     virtual void warnReturnTypestateMismatch(SourceLocation Loc,
                                              StringRef ExpectedState,
                                              StringRef ObservedState) {}
@@ -118,6 +139,11 @@ namespace consumed {
     ConsumedStateMap(const ConsumedStateMap &Other)
       : Reachable(Other.Reachable), From(Other.From), Map(Other.Map) {}
     
+    /// \brief Warn if any of the parameters being tracked are not in the state
+    /// they were declared to be in upon return from a function.
+    void checkParamsForReturnTypestate(SourceLocation BlameLoc,
+      ConsumedWarningsHandlerBase &WarningsHandler) const;
+    
     /// \brief Get the consumed state of a given variable.
     ConsumedState getState(const VarDecl *Var) const;
     

diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td
index c88b9a3ba742ac52ae6466e1e927cb255626ce76..0521830efcee671609657bcf6905bab3a04b62fc 100644 (file)
--- a/include/clang/Basic/Attr.td
+++ b/include/clang/Basic/Attr.td
@@ -969,7 +969,7 @@ def CallableWhen : InheritableAttr {
 
 def ReturnTypestate : InheritableAttr {
   let Spellings = [GNU<"return_typestate">];
-  let Subjects = [Function];
+  let Subjects = [Function, ParmVar];
   let Args = [EnumArgument<"State", "ConsumedState",
                            ["unknown", "consumed", "unconsumed"],
                            ["Unknown", "Consumed", "Unconsumed"]>];

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 1c4f0bc50a2eb6b8061cc4a692d5539888e28d4d..399e9d7a2b8e93323fe4e686911425dd717216af 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2222,6 +2222,9 @@ def warn_return_typestate_mismatch : Warning<
 def warn_loop_state_mismatch : Warning<
   "state of variable '%0' must match at the entry and exit of loop">,
   InGroup<Consumed>, DefaultIgnore;
+def warn_param_return_typestate_mismatch : Warning<
+  "parameter '%0' not in expected state when the function returns: expected "
+  "'%1', observed '%2'">, InGroup<Consumed>, DefaultIgnore;
 
 def warn_impcast_vector_scalar : Warning<
   "implicit conversion turns vector to scalar: %0 to %1">,

diff --git a/lib/Analysis/Consumed.cpp b/lib/Analysis/Consumed.cpp
index b06fb155d5b8ec06720c200c33be840d8c1157d0..021c26dff2f014241b1c1a4254ea4a69728399ef 100644 (file)
--- a/lib/Analysis/Consumed.cpp
+++ b/lib/Analysis/Consumed.cpp
@@ -87,7 +87,7 @@ static SourceLocation getLastStmtLoc(const CFGBlock *Block) {
     Loc = getFirstStmtLoc(*Block->succ_begin());
   if (Loc.isValid())
     return Loc;
-  
+
   // If we have one predecessor, return the last statement in that block
   if (Block->pred_size() == 1 && *Block->pred_begin())
     return getLastStmtLoc(*Block->pred_begin());
@@ -572,7 +572,8 @@ void ConsumedStmtVisitor::VisitCallExpr(const CallExpr *Call) {
     unsigned Offset = Call->getNumArgs() - FunDecl->getNumParams();
     
     for (unsigned Index = Offset; Index < Call->getNumArgs(); ++Index) {
-      QualType ParamType = FunDecl->getParamDecl(Index - Offset)->getType();
+      const ParmVarDecl *Param = FunDecl->getParamDecl(Index - Offset);
+      QualType ParamType = Param->getType();
       
       InfoEntry Entry = PropagationMap.find(Call->getArg(Index));
       
@@ -588,6 +589,10 @@ void ConsumedStmtVisitor::VisitCallExpr(const CallExpr *Call) {
         
         StateMap->setState(PInfo.getVar(), consumed::CS_Consumed);
         
+      } else if (Param->hasAttr<ReturnTypestateAttr>()) {
+        StateMap->setState(PInfo.getVar(),
+          mapReturnTypestateAttrState(Param->getAttr<ReturnTypestateAttr>()));
+        
       } else if (!(ParamType.isConstQualified() ||
                    ((ParamType->isReferenceType() ||
                      ParamType->isPointerType()) &&
@@ -820,7 +825,9 @@ void ConsumedStmtVisitor::VisitParmVarDecl(const ParmVarDecl *Param) {
 }
 
 void ConsumedStmtVisitor::VisitReturnStmt(const ReturnStmt *Ret) {
-  if (ConsumedState ExpectedState = Analyzer.getExpectedReturnState()) {
+  ConsumedState ExpectedState = Analyzer.getExpectedReturnState();
+  
+  if (ExpectedState != CS_None) {
     InfoEntry Entry = PropagationMap.find(Ret->getRetValue());
     
     if (Entry != PropagationMap.end()) {
@@ -835,6 +842,9 @@ void ConsumedStmtVisitor::VisitReturnStmt(const ReturnStmt *Ret) {
           stateToString(RetState));
     }
   }
+  
+  StateMap->checkParamsForReturnTypestate(Ret->getLocStart(),
+                                          Analyzer.WarningsHandler);
 }
 
 void ConsumedStmtVisitor::VisitUnaryOperator(const UnaryOperator *UOp) {
@@ -1054,6 +1064,31 @@ bool ConsumedBlockInfo::isBackEdgeTarget(const CFGBlock *Block) {
   return false;
 }
 
+void ConsumedStateMap::checkParamsForReturnTypestate(SourceLocation BlameLoc,
+  ConsumedWarningsHandlerBase &WarningsHandler) const {
+  
+  ConsumedState ExpectedState;
+  
+  for (MapType::const_iterator DMI = Map.begin(), DME = Map.end(); DMI != DME;
+       ++DMI) {
+    
+    if (isa<ParmVarDecl>(DMI->first)) {
+      const ParmVarDecl *Param = cast<ParmVarDecl>(DMI->first);
+      
+      if (!Param->hasAttr<ReturnTypestateAttr>()) continue;
+      
+      ExpectedState =
+        mapReturnTypestateAttrState(Param->getAttr<ReturnTypestateAttr>());
+      
+      if (DMI->second != ExpectedState) {
+        WarningsHandler.warnParamReturnTypestateMismatch(BlameLoc,
+          Param->getNameAsString(), stateToString(ExpectedState),
+          stateToString(DMI->second));
+      }
+    }
+  }
+}
+
 ConsumedState ConsumedStateMap::getState(const VarDecl *Var) const {
   MapType::const_iterator Entry = Map.find(Var);
   
@@ -1375,6 +1410,11 @@ void ConsumedAnalyzer::run(AnalysisDeclContext &AC) {
         CurrStates = NULL;
       }
     }
+    
+    if (CurrBlock == &AC.getCFG()->getExit() &&
+        D->getCallResultType()->isVoidType())
+      CurrStates->checkParamsForReturnTypestate(D->getLocation(),
+                                                WarningsHandler);
   } // End of block iterator.
   
   // Delete the last existing state map.

diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp
index 737f59a3f97b43bf1402a7bd10130c8e49a066ac..2d65980ec07ac53254650bf1c7888780a1870a81 100644 (file)
--- a/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1484,6 +1484,18 @@ public:
     Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
   }
   
+  void warnParamReturnTypestateMismatch(SourceLocation Loc,
+                                        StringRef VariableName,
+                                        StringRef ExpectedState,
+                                        StringRef ObservedState) {
+    
+    PartialDiagnosticAt Warning(Loc, S.PDiag(
+      diag::warn_param_return_typestate_mismatch) << VariableName <<
+        ExpectedState << ObservedState);
+    
+    Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
+  }
+  
   void warnReturnTypestateForUnconsumableType(SourceLocation Loc,
                                               StringRef TypeName) {
     PartialDiagnosticAt Warning(Loc, S.PDiag(

diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp
index c08767574ceb260388d8035259febde0fcffe988..c3e6bc4cc347fc578aa55cbd1840e5cc8e1550cc 100644 (file)
--- a/lib/Sema/SemaDeclAttr.cpp
+++ b/lib/Sema/SemaDeclAttr.cpp
@@ -1068,6 +1068,14 @@ static void handleCallableWhenAttr(Sema &S, Decl *D,
 
 static void handleReturnTypestateAttr(Sema &S, Decl *D,
                                       const AttributeList &Attr) {
+  if (!checkAttributeNumArgs(S, Attr, 1)) return;
+  
+  if (!(isa<FunctionDecl>(D) || isa<ParmVarDecl>(D))) {
+    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
+      Attr.getName() << ExpectedFunctionMethodOrParameter;
+    return;
+  }
+
   ReturnTypestateAttr::ConsumedState ReturnState;
   
   if (Attr.isArgIdent(0)) {
@@ -1084,18 +1092,16 @@ static void handleReturnTypestateAttr(Sema &S, Decl *D,
     return;
   }
   
-  if (!isa<FunctionDecl>(D)) {
-    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
-      Attr.getName() << ExpectedFunction;
-    return;
-  }
-  
   // FIXME: This check is currently being done in the analysis.  It can be
   //        enabled here only after the parser propagates attributes at
   //        template specialization definition, not declaration.
   //QualType ReturnType;
   //
-  //if (const CXXConstructorDecl *Constructor = dyn_cast<CXXConstructorDecl>(D)) {
+  //if (const ParmVarDecl *Param = dyn_cast<ParmVarDecl>(D)) {
+  //  ReturnType = Param->getType();
+  //
+  //} else if (const CXXConstructorDecl *Constructor =
+  //             dyn_cast<CXXConstructorDecl>(D)) {
   //  ReturnType = Constructor->getThisType(S.getASTContext())->getPointeeType();
   //  
   //} else {

diff --git a/test/SemaCXX/warn-consumed-analysis.cpp b/test/SemaCXX/warn-consumed-analysis.cpp
index 264d27af5e77a318b8014eea84ac0f3ad6aaef57..dd1bb2312d8ebecab255397a4bd8b22c2f3443cd 100644 (file)
--- a/test/SemaCXX/warn-consumed-analysis.cpp
+++ b/test/SemaCXX/warn-consumed-analysis.cpp
@@ -388,6 +388,24 @@ void testFunctionParam(ConsumableClass<int> param) {
   *param; // expected-warning {{invocation of method 'operator*' on object 'param' while it is in the 'consumed' state}}
 }
 
+void testParamReturnTypestateCallee(bool cond, ConsumableClass<int> &Param RETURN_TYPESTATE(unconsumed)) { // expected-warning {{parameter 'Param' not in expected state when the function returns: expected 'unconsumed', observed 'consumed'}}
+  
+  if (cond) {
+    Param.consume();
+    return; // expected-warning {{parameter 'Param' not in expected state when the function returns: expected 'unconsumed', observed 'consumed'}}
+  }
+  
+  Param.consume();
+}
+
+void testParamReturnTypestateCaller() {
+  ConsumableClass<int> var;
+  
+  testParamReturnTypestateCallee(true, var);
+  
+  *var;
+}
+
 void testCallingConventions() {
   ConsumableClass<int> var(42);
   

diff --git a/test/SemaCXX/warn-consumed-parsing.cpp b/test/SemaCXX/warn-consumed-parsing.cpp
index 80021020ca66cc4c80b1fee5cb7c8939603b6920..c1bc1078733b6b7fc803fcb4dc2cbd124c81f072 100644 (file)
--- a/test/SemaCXX/warn-consumed-parsing.cpp
+++ b/test/SemaCXX/warn-consumed-parsing.cpp
@@ -44,6 +44,8 @@ class CONSUMABLE(unknown) AttrTester1 {
 AttrTester1 returnTypestateTester0() RETURN_TYPESTATE(not_a_state); // expected-warning {{'return_typestate' attribute argument not supported: 'not_a_state'}}
 AttrTester1 returnTypestateTester1() RETURN_TYPESTATE(42); // expected-error {{'return_typestate' attribute requires an identifier}}
 
+void returnTypestateTester2(AttrTester1 &Param RETURN_TYPESTATE(unconsumed));
+
 class AttrTester2 {
   void callableWhen()    CALLABLE_WHEN("unconsumed"); // expected-warning {{consumed analysis attribute is attached to member of class 'AttrTester2' which isn't marked as consumable}}
   void consumes()        SET_TYPESTATE(consumed); // expected-warning {{consumed analysis attribute is attached to member of class 'AttrTester2' which isn't marked as consumable}}