Index: include/clang/Analysis/Analyses/Consumed.h =================================================================== --- include/clang/Analysis/Analyses/Consumed.h +++ include/clang/Analysis/Analyses/Consumed.h @@ -39,11 +39,29 @@ /// \brief Emit the warnings and notes left by the analysis. virtual void emitDiagnostics() {} - + + // FIXME: This can be removed when the attr propagation fix for templated + // classes lands. + /// \brief Warn about return typestates set for unconsumable types. + /// + /// \param Loc -- The location of the attributes. + /// + /// \param TypeName -- The name of the unconsumable type. + virtual void warnReturnTypestateForUnconsumableType(SourceLocation Loc, + StringRef TypeName) {} + + /// \brief Warn about return typestate mismatches. + /// \param Loc -- The SourceLocation of the return statement. + virtual void warnReturnTypestateMismatch(SourceLocation Loc, + StringRef ExpectedState, + StringRef ObservedState) {} + /// \brief Warn about unnecessary-test errors. /// \param VariableName -- The name of the variable that holds the unique /// value. /// + /// \param VariableState -- The known state of the value. + /// /// \param Loc -- The SourceLocation of the unnecessary test. virtual void warnUnnecessaryTest(StringRef VariableName, StringRef VariableState, @@ -170,6 +188,8 @@ ConsumedBlockInfo BlockInfo; ConsumedStateMap *CurrStates; + ConsumedState ExpectedReturnState; + bool hasConsumableAttributes(const CXXRecordDecl *RD); bool splitState(const CFGBlock *CurrBlock, const ConsumedStmtVisitor &Visitor); @@ -181,6 +201,8 @@ ConsumedAnalyzer(ConsumedWarningsHandlerBase &WarningsHandler) : WarningsHandler(WarningsHandler) {} + ConsumedState getExpectedReturnState() const { return ExpectedReturnState; } + /// \brief Check a function's CFG for consumed violations. /// /// We traverse the blocks in the CFG, keeping track of the state of each Index: include/clang/Basic/Attr.td =================================================================== --- include/clang/Basic/Attr.td +++ include/clang/Basic/Attr.td @@ -953,6 +953,14 @@ let Subjects = [CXXMethod]; } +def ReturnTypestate : InheritableAttr { + let Spellings = [GNU<"return_typestate">]; + let Subjects = [Function]; + let Args = [EnumArgument<"State", "ConsumedState", + ["unknown", "consumed", "unconsumed"], + ["Unknown", "Consumed", "Unconsumed"]>]; +} + // Type safety attributes for `void *' pointers and type tags. def ArgumentWithTypeTag : InheritableAttr { Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -2192,8 +2192,16 @@ "invocation of method '%0' on a temporary object while it is in the " "'consumed' state">, InGroup, DefaultIgnore; def warn_attr_on_unconsumable_class : Warning< - "consumed analysis attribute is attached to class '%0' which isn't marked " - "as consumable">, InGroup, DefaultIgnore; + "consumed analysis attribute is attached to member of class '%0' which isn't " + "marked as consumable">, InGroup, DefaultIgnore; +def warn_return_typestate_for_unconsumable_type : Warning< + "return state set for an unconsumable type '%0'">, InGroup, + DefaultIgnore; +def warn_unknown_consumed_state : Warning< + "unknown consumed analysis state '%0'">, InGroup, DefaultIgnore; +def warn_return_typestate_mismatch : Warning< + "return value not in expected state; expected '%0', observed '%1'">, + InGroup, DefaultIgnore; // ConsumedStrict warnings def warn_use_in_unknown_state : Warning< Index: lib/Analysis/Consumed.cpp =================================================================== --- lib/Analysis/Consumed.cpp +++ lib/Analysis/Consumed.cpp @@ -31,6 +31,7 @@ #include "llvm/Support/Compiler.h" #include "llvm/Support/raw_ostream.h" +// TODO: Add notes about the actual and expected state for // TODO: Correctly identify unreachable blocks when chaining boolean operators. // TODO: Warn about unreachable code. // TODO: Switch to using a bitmap to track unreachable blocks. @@ -88,6 +89,19 @@ return FunDecl->hasAttr(); } +static ConsumedState mapReturnTypestateAttrState( + const ReturnTypestateAttr *RTSAttr) { + + switch (RTSAttr->getState()) { + case ReturnTypestateAttr::Unknown: + return CS_Unknown; + case ReturnTypestateAttr::Unconsumed: + return CS_Unconsumed; + case ReturnTypestateAttr::Consumed: + return CS_Consumed; + } +} + static StringRef stateToString(ConsumedState State) { switch (State) { case consumed::CS_None: @@ -256,6 +270,8 @@ void forwardInfo(const Stmt *From, const Stmt *To); void handleTestingFunctionCall(const CallExpr *Call, const VarDecl *Var); bool isLikeMoveAssignment(const CXXMethodDecl *MethodDecl); + void propagateReturnType(const Stmt *Call, const FunctionDecl *Fun, + QualType ReturnType); public: @@ -272,6 +288,7 @@ void VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *Temp); void VisitMemberExpr(const MemberExpr *MExpr); void VisitParmVarDecl(const ParmVarDecl *Param); + void VisitReturnStmt(const ReturnStmt *Ret); void VisitUnaryOperator(const UnaryOperator *UOp); void VisitVarDecl(const VarDecl *Var); @@ -373,6 +390,24 @@ MethodDecl->getParamDecl(0)->getType()->isRValueReferenceType()); } +void ConsumedStmtVisitor::propagateReturnType(const Stmt *Call, + const FunctionDecl *Fun, + QualType ReturnType) { + if (isConsumableType(ReturnType)) { + + ConsumedState ReturnState; + + if (Fun->hasAttr()) + ReturnState = mapReturnTypestateAttrState( + Fun->getAttr()); + else + ReturnState = CS_Unknown; + + PropagationMap.insert(PairType(Call, + PropagationInfo(ReturnState))); + } +} + void ConsumedStmtVisitor::Visit(const Stmt *StmtNode) { ConstStmtVisitor::Visit(StmtNode); @@ -469,6 +504,8 @@ StateMap->setState(PInfo.getVar(), consumed::CS_Unknown); } } + + propagateReturnType(Call, FunDecl, FunDecl->getCallResultType()); } } @@ -483,8 +520,7 @@ QualType ThisType = Constructor->getThisType(CurrContext)->getPointeeType(); if (isConsumableType(ThisType)) { - if (Constructor->hasAttr() || - Constructor->isDefaultConstructor()) { + if (Constructor->isDefaultConstructor()) { PropagationMap.insert(PairType(Call, PropagationInfo(consumed::CS_Consumed))); @@ -513,8 +549,7 @@ PropagationMap.insert(PairType(Call, Entry->second)); } else { - PropagationMap.insert(PairType(Call, - PropagationInfo(consumed::CS_Unconsumed))); + propagateReturnType(Call, Constructor, ThisType); } } } @@ -677,6 +712,24 @@ StateMap->setState(Param, consumed::CS_Unknown); } +void ConsumedStmtVisitor::VisitReturnStmt(const ReturnStmt *Ret) { + if (ConsumedState ExpectedState = Analyzer.getExpectedReturnState()) { + InfoEntry Entry = PropagationMap.find(Ret->getRetValue()); + + if (Entry != PropagationMap.end()) { + assert(Entry->second.isState() || Entry->second.isVar()); + + ConsumedState RetState = Entry->second.isState() ? + Entry->second.getState() : StateMap->getState(Entry->second.getVar()); + + if (RetState != ExpectedState) + Analyzer.WarningsHandler.warnReturnTypestateMismatch( + Ret->getReturnLoc(), stateToString(ExpectedState), + stateToString(RetState)); + } + } +} + void ConsumedStmtVisitor::VisitUnaryOperator(const UnaryOperator *UOp) { InfoEntry Entry = PropagationMap.find(UOp->getSubExpr()->IgnoreParens()); if (Entry == PropagationMap.end()) return; @@ -997,6 +1050,53 @@ if (!D) return; + // FIXME: This should be removed when template instantiation propagates + // attributes at template specialization definition, not declaration. + // When it is removed the test needs to be enabled in SemaDeclAttr.cpp. + QualType ReturnType; + if (const CXXConstructorDecl *Constructor = dyn_cast(D)) { + ASTContext &CurrContext = AC.getASTContext(); + ReturnType = Constructor->getThisType(CurrContext)->getPointeeType(); + + } else { + ReturnType = D->getCallResultType(); + } + + // Determine the expected return value. + if (D->hasAttr()) { + + ReturnTypestateAttr *RTSAttr = D->getAttr(); + + const CXXRecordDecl *RD = ReturnType->getAsCXXRecordDecl(); + if (!RD || !RD->hasAttr()) { + // FIXME: This branch can be removed with the code above. + WarningsHandler.warnReturnTypestateForUnconsumableType( + RTSAttr->getLocation(), ReturnType.getAsString()); + ExpectedReturnState = CS_None; + + } else { + switch (RTSAttr->getState()) { + case ReturnTypestateAttr::Unknown: + ExpectedReturnState = CS_Unknown; + break; + + case ReturnTypestateAttr::Unconsumed: + ExpectedReturnState = CS_Unconsumed; + break; + + case ReturnTypestateAttr::Consumed: + ExpectedReturnState = CS_Consumed; + break; + } + } + + } else if (isConsumableType(ReturnType)) { + ExpectedReturnState = CS_Unknown; + + } else { + ExpectedReturnState = CS_None; + } + BlockInfo = ConsumedBlockInfo(AC.getCFG()); PostOrderCFGView *SortedGraph = AC.getAnalysis(); Index: lib/Sema/AnalysisBasedWarnings.cpp =================================================================== --- lib/Sema/AnalysisBasedWarnings.cpp +++ lib/Sema/AnalysisBasedWarnings.cpp @@ -1445,11 +1445,23 @@ } } - /// Warn about unnecessary-test errors. - /// \param VariableName -- The name of the variable that holds the unique - /// value. - /// - /// \param Loc -- The SourceLocation of the unnecessary test. + void warnReturnTypestateForUnconsumableType(SourceLocation Loc, + StringRef TypeName) { + PartialDiagnosticAt Warning(Loc, S.PDiag( + diag::warn_return_typestate_for_unconsumable_type) << TypeName); + + Warnings.push_back(DelayedDiag(Warning, OptionalNotes())); + } + + void warnReturnTypestateMismatch(SourceLocation Loc, StringRef ExpectedState, + StringRef ObservedState) { + + PartialDiagnosticAt Warning(Loc, S.PDiag( + diag::warn_return_typestate_mismatch) << ExpectedState << ObservedState); + + Warnings.push_back(DelayedDiag(Warning, OptionalNotes())); + } + void warnUnnecessaryTest(StringRef VariableName, StringRef VariableState, SourceLocation Loc) { @@ -1459,11 +1471,6 @@ Warnings.push_back(DelayedDiag(Warning, OptionalNotes())); } - /// Warn about use-while-consumed errors. - /// \param MethodName -- The name of the method that was incorrectly - /// invoked. - /// - /// \param Loc -- The SourceLocation of the method invocation. void warnUseOfTempWhileConsumed(StringRef MethodName, SourceLocation Loc) { PartialDiagnosticAt Warning(Loc, S.PDiag( @@ -1472,11 +1479,6 @@ Warnings.push_back(DelayedDiag(Warning, OptionalNotes())); } - /// Warn about use-in-unknown-state errors. - /// \param MethodName -- The name of the method that was incorrectly - /// invoked. - /// - /// \param Loc -- The SourceLocation of the method invocation. void warnUseOfTempInUnknownState(StringRef MethodName, SourceLocation Loc) { PartialDiagnosticAt Warning(Loc, S.PDiag( @@ -1485,14 +1487,6 @@ Warnings.push_back(DelayedDiag(Warning, OptionalNotes())); } - /// Warn about use-while-consumed errors. - /// \param MethodName -- The name of the method that was incorrectly - /// invoked. - /// - /// \param VariableName -- The name of the variable that holds the unique - /// value. - /// - /// \param Loc -- The SourceLocation of the method invocation. void warnUseWhileConsumed(StringRef MethodName, StringRef VariableName, SourceLocation Loc) { @@ -1502,14 +1496,6 @@ Warnings.push_back(DelayedDiag(Warning, OptionalNotes())); } - /// Warn about use-in-unknown-state errors. - /// \param MethodName -- The name of the method that was incorrectly - /// invoked. - /// - /// \param VariableName -- The name of the variable that holds the unique - /// value. - /// - /// \param Loc -- The SourceLocation of the method invocation. void warnUseInUnknownState(StringRef MethodName, StringRef VariableName, SourceLocation Loc) { Index: lib/Sema/SemaDeclAttr.cpp =================================================================== --- lib/Sema/SemaDeclAttr.cpp +++ lib/Sema/SemaDeclAttr.cpp @@ -1069,6 +1069,64 @@ Attr.getAttributeSpellingListIndex())); } +static void handleReturnTypestateAttr(Sema &S, Decl *D, + const AttributeList &Attr) { + if (!checkAttributeNumArgs(S, Attr, 1)) return; + + ReturnTypestateAttr::ConsumedState ReturnState; + + if (Attr.isArgIdent(0)) { + StringRef Param = Attr.getArgAsIdent(0)->Ident->getName(); + + if (Param == "unknown") { + ReturnState = ReturnTypestateAttr::Unknown; + } else if (Param == "consumed") { + ReturnState = ReturnTypestateAttr::Consumed; + } else if (Param == "unconsumed") { + ReturnState = ReturnTypestateAttr::Unconsumed; + } else { + S.Diag(Attr.getLoc(), diag::warn_unknown_consumed_state) << Param; + return; + } + + } else { + S.Diag(Attr.getLoc(), diag::err_attribute_argument_type) << + Attr.getName() << AANT_ArgumentIdentifier; + 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)) { + // ReturnType = Constructor->getThisType(S.getASTContext())->getPointeeType(); + // + //} else { + // + // ReturnType = cast(D)->getCallResultType(); + //} + // + //const CXXRecordDecl *RD = ReturnType->getAsCXXRecordDecl(); + // + //if (!RD || !RD->hasAttr()) { + // S.Diag(Attr.getLoc(), diag::warn_return_state_for_unconsumable_type) << + // ReturnType.getAsString(); + // return; + //} + + D->addAttr(::new (S.Context) + ReturnTypestateAttr(Attr.getRange(), S.Context, ReturnState, + Attr.getAttributeSpellingListIndex())); +} + static void handleExtVectorTypeAttr(Sema &S, Scope *scope, Decl *D, const AttributeList &Attr) { TypedefNameDecl *TD = dyn_cast(D); @@ -5024,6 +5082,9 @@ case AttributeList::AT_TestsUnconsumed: handleTestsUnconsumedAttr(S, D, Attr); break; + case AttributeList::AT_ReturnTypestate: + handleReturnTypestateAttr(S, D, Attr); + break; // Type safety attributes. case AttributeList::AT_ArgumentWithTypeTag: Index: test/SemaCXX/warn-consumed-analysis-strict.cpp =================================================================== --- test/SemaCXX/warn-consumed-analysis-strict.cpp +++ test/SemaCXX/warn-consumed-analysis-strict.cpp @@ -3,6 +3,7 @@ #define CALLABLE_WHEN_UNCONSUMED __attribute__ ((callable_when_unconsumed)) #define CONSUMABLE __attribute__ ((consumable)) #define CONSUMES __attribute__ ((consumes)) +#define RETURN_TYPESTATE(State) __attribute__ ((return_typestate(State))) #define TESTS_UNCONSUMED __attribute__ ((tests_unconsumed)) #define TEST_VAR(Var) Var.isValid() @@ -15,9 +16,10 @@ public: ConsumableClass(); - ConsumableClass(T val); - ConsumableClass(ConsumableClass &other); - ConsumableClass(ConsumableClass &&other); + ConsumableClass(nullptr_t p) RETURN_TYPESTATE(consumed); + ConsumableClass(T val) RETURN_TYPESTATE(unconsumed); + ConsumableClass(ConsumableClass &other) RETURN_TYPESTATE(unconsumed); + ConsumableClass(ConsumableClass &&other) RETURN_TYPESTATE(unconsumed); ConsumableClass& operator=(ConsumableClass &other); ConsumableClass& operator=(ConsumableClass &&other); Index: test/SemaCXX/warn-consumed-analysis.cpp =================================================================== --- test/SemaCXX/warn-consumed-analysis.cpp +++ test/SemaCXX/warn-consumed-analysis.cpp @@ -3,6 +3,7 @@ #define CALLABLE_WHEN_UNCONSUMED __attribute__ ((callable_when_unconsumed)) #define CONSUMABLE __attribute__ ((consumable)) #define CONSUMES __attribute__ ((consumes)) +#define RETURN_TYPESTATE(State) __attribute__ ((return_typestate(State))) #define TESTS_UNCONSUMED __attribute__ ((tests_unconsumed)) typedef decltype(nullptr) nullptr_t; @@ -13,10 +14,10 @@ public: ConsumableClass(); - ConsumableClass(nullptr_t p) CONSUMES; - ConsumableClass(T val); - ConsumableClass(ConsumableClass &other); - ConsumableClass(ConsumableClass &&other); + ConsumableClass(nullptr_t p) RETURN_TYPESTATE(consumed); + ConsumableClass(T val) RETURN_TYPESTATE(unconsumed); + ConsumableClass(ConsumableClass &other) RETURN_TYPESTATE(unconsumed); + ConsumableClass(ConsumableClass &&other) RETURN_TYPESTATE(unconsumed); ConsumableClass& operator=(ConsumableClass &other); ConsumableClass& operator=(ConsumableClass &&other); @@ -48,6 +49,16 @@ void baf3(ConsumableClass &&var); +ConsumableClass returnsUnconsumed() RETURN_TYPESTATE(unconsumed); +ConsumableClass returnsUnconsumed() { + return ConsumableClass(); // expected-warning {{return value not in expected state; expected 'unconsumed', observed 'consumed'}} +} + +ConsumableClass returnsConsumed() RETURN_TYPESTATE(consumed); +ConsumableClass returnsConsumed() { + return ConsumableClass(); +} + void testInitialization() { ConsumableClass var0; ConsumableClass var1 = ConsumableClass(); @@ -253,6 +264,16 @@ *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}} } +void testReturnStates() { + ConsumableClass var; + + var = returnsUnconsumed(); + *var; + + var = returnsConsumed(); + *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}} +} + void testMoveAsignmentish() { ConsumableClass var0; ConsumableClass var1(42); Index: test/SemaCXX/warn-consumed-parsing.cpp =================================================================== --- test/SemaCXX/warn-consumed-parsing.cpp +++ test/SemaCXX/warn-consumed-parsing.cpp @@ -1,21 +1,29 @@ // RUN: %clang_cc1 -fsyntax-only -verify -Wconsumed -std=c++11 %s -#define CONSUMABLE __attribute__ ((consumable)) -#define CONSUMES __attribute__ ((consumes)) -#define TESTS_UNCONSUMED __attribute__ ((tests_unconsumed)) -#define CALLABLE_WHEN_UNCONSUMED __attribute__ ((callable_when_unconsumed)) +#define CALLABLE_WHEN_UNCONSUMED __attribute__ ((callable_when_unconsumed)) +#define CONSUMABLE __attribute__ ((consumable)) +#define CONSUMES __attribute__ ((consumes)) +#define RETURN_TYPESTATE(State) __attribute__ ((return_typestate(State))) +#define TESTS_UNCONSUMED __attribute__ ((tests_unconsumed)) + +// FIXME: This warning is not generated if it appears bellow the AttrTester0 +// class declaration. Why? +int returnTypestateForUnconsumable() RETURN_TYPESTATE(consumed); // expected-warning {{return state set for an unconsumable type 'int'}} +int returnTypestateForUnconsumable() { + return 0; +} class AttrTester0 { - void Consumes() __attribute__ ((consumes(42))); // expected-error {{attribute takes no arguments}} - bool TestsUnconsumed() __attribute__ ((tests_unconsumed(42))); // expected-error {{attribute takes no arguments}} - void CallableWhenUnconsumed() - __attribute__ ((callable_when_unconsumed(42))); // expected-error {{attribute takes no arguments}} + void consumes() __attribute__ ((consumes(42))); // expected-error {{attribute takes no arguments}} + bool testsUnconsumed() __attribute__ ((tests_unconsumed(42))); // expected-error {{attribute takes no arguments}} + void callableWhenUnconsumed() __attribute__ ((callable_when_unconsumed(42))); // expected-error {{attribute takes no arguments}} }; int var0 CONSUMES; // expected-warning {{'consumes' attribute only applies to methods}} int var1 TESTS_UNCONSUMED; // expected-warning {{'tests_unconsumed' attribute only applies to methods}} int var2 CALLABLE_WHEN_UNCONSUMED; // expected-warning {{'callable_when_unconsumed' attribute only applies to methods}} int var3 CONSUMABLE; // expected-warning {{'consumable' attribute only applies to classes}} +int var4 RETURN_TYPESTATE(consumed); // expected-warning {{'return_typestate' attribute only applies to functions}} void function0() CONSUMES; // expected-warning {{'consumes' attribute only applies to methods}} void function1() TESTS_UNCONSUMED; // expected-warning {{'tests_unconsumed' attribute only applies to methods}} @@ -28,8 +36,11 @@ bool testsUnconsumed() TESTS_UNCONSUMED; }; +AttrTester1 returnTypestateTester0() RETURN_TYPESTATE(not_a_state); // expected-warning {{unknown consumed analysis state 'not_a_state'}} +AttrTester1 returnTypestateTester1() RETURN_TYPESTATE(42); // expected-error {{'return_typestate' attribute requires an identifier}} + class AttrTester2 { - void callableWhenUnconsumed() CALLABLE_WHEN_UNCONSUMED; // expected-warning {{consumed analysis attribute is attached to class 'AttrTester2' which isn't marked as consumable}} - void consumes() CONSUMES; // expected-warning {{consumed analysis attribute is attached to class 'AttrTester2' which isn't marked as consumable}} - bool testsUnconsumed() TESTS_UNCONSUMED; // expected-warning {{consumed analysis attribute is attached to class 'AttrTester2' which isn't marked as consumable}} + void callableWhenUnconsumed() CALLABLE_WHEN_UNCONSUMED; // expected-warning {{consumed analysis attribute is attached to member of class 'AttrTester2' which isn't marked as consumable}} + void consumes() CONSUMES; // expected-warning {{consumed analysis attribute is attached to member of class 'AttrTester2' which isn't marked as consumable}} + bool testsUnconsumed() TESTS_UNCONSUMED; // expected-warning {{consumed analysis attribute is attached to member of class 'AttrTester2' which isn't marked as consumable}} };