From 36ea1dd4fc5e193d0eb7f6bf69f15d3779b6da24 Mon Sep 17 00:00:00 2001 From: DeLesley Hutchins Date: Thu, 17 Oct 2013 22:53:04 +0000 Subject: [PATCH] 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. llvm-svn: 192932 --- .../clang/Analysis/Analyses/Consumed.h | 26 +++++++++++ clang/include/clang/Basic/Attr.td | 2 +- .../clang/Basic/DiagnosticSemaKinds.td | 3 ++ clang/lib/Analysis/Consumed.cpp | 46 +++++++++++++++++-- clang/lib/Sema/AnalysisBasedWarnings.cpp | 12 +++++ clang/lib/Sema/SemaDeclAttr.cpp | 20 +++++--- clang/test/SemaCXX/warn-consumed-analysis.cpp | 18 ++++++++ clang/test/SemaCXX/warn-consumed-parsing.cpp | 2 + 8 files changed, 118 insertions(+), 11 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/Consumed.h b/clang/include/clang/Analysis/Analyses/Consumed.h index f097e3da7cf9..69bd022ba239 100644 --- a/clang/include/clang/Analysis/Analyses/Consumed.h +++ b/clang/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/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index c88b9a3ba742..0521830efcee 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/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/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 1c4f0bc50a2e..399e9d7a2b8e 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/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, DefaultIgnore; +def warn_param_return_typestate_mismatch : Warning< + "parameter '%0' not in expected state when the function returns: expected " + "'%1', observed '%2'">, InGroup, DefaultIgnore; def warn_impcast_vector_scalar : Warning< "implicit conversion turns vector to scalar: %0 to %1">, diff --git a/clang/lib/Analysis/Consumed.cpp b/clang/lib/Analysis/Consumed.cpp index b06fb155d5b8..021c26dff2f0 100644 --- a/clang/lib/Analysis/Consumed.cpp +++ b/clang/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()) { + StateMap->setState(PInfo.getVar(), + mapReturnTypestateAttrState(Param->getAttr())); + } 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(DMI->first)) { + const ParmVarDecl *Param = cast(DMI->first); + + if (!Param->hasAttr()) continue; + + ExpectedState = + mapReturnTypestateAttrState(Param->getAttr()); + + 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/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 737f59a3f97b..2d65980ec07a 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/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/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index c08767574ceb..c3e6bc4cc347 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/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(D) || isa(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(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(D)) { + //if (const ParmVarDecl *Param = dyn_cast(D)) { + // ReturnType = Param->getType(); + // + //} else if (const CXXConstructorDecl *Constructor = + // dyn_cast(D)) { // ReturnType = Constructor->getThisType(S.getASTContext())->getPointeeType(); // //} else { diff --git a/clang/test/SemaCXX/warn-consumed-analysis.cpp b/clang/test/SemaCXX/warn-consumed-analysis.cpp index 264d27af5e77..dd1bb2312d8e 100644 --- a/clang/test/SemaCXX/warn-consumed-analysis.cpp +++ b/clang/test/SemaCXX/warn-consumed-analysis.cpp @@ -388,6 +388,24 @@ void testFunctionParam(ConsumableClass param) { *param; // expected-warning {{invocation of method 'operator*' on object 'param' while it is in the 'consumed' state}} } +void testParamReturnTypestateCallee(bool cond, ConsumableClass &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 var; + + testParamReturnTypestateCallee(true, var); + + *var; +} + void testCallingConventions() { ConsumableClass var(42); diff --git a/clang/test/SemaCXX/warn-consumed-parsing.cpp b/clang/test/SemaCXX/warn-consumed-parsing.cpp index 80021020ca66..c1bc1078733b 100644 --- a/clang/test/SemaCXX/warn-consumed-parsing.cpp +++ b/clang/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}}