Index: include/clang/Analysis/Analyses/Consumed.h =================================================================== --- include/clang/Analysis/Analyses/Consumed.h +++ include/clang/Analysis/Analyses/Consumed.h @@ -89,6 +89,16 @@ StringRef ExpectedState, StringRef ObservedState) {} + // FIXME: This can be removed when the attr propagation fix for templated + // classes lands. + /// 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 warnParamAttrForUnconsumableType(SourceLocation Loc, + StringRef TypeName) {} + // FIXME: This can be removed when the attr propagation fix for templated // classes lands. /// Warn about return typestates set for unconsumable types. Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -3185,6 +3185,15 @@ def warn_attr_on_unconsumable_class : Warning< "consumed analysis attribute is attached to member of class '%0' which isn't " "marked as consumable">, InGroup, DefaultIgnore; +def warn_consumable_attr_on_static : Warning< + "consumed analysis attribute is attached to a " + "%select{static method|constructor}0 of class '%1'">, + InGroup, DefaultIgnore; +def warn_param_attr_for_unconsumable_type : Warning< + "consumed analysis attribute is attached to a parameter of type '%0': " + "expected a type marked as consumable passed by value, reference, or " + "rvalue reference">, + InGroup, DefaultIgnore; def warn_return_typestate_for_unconsumable_type : Warning< "return state set for an unconsumable type '%0'">, InGroup, DefaultIgnore; Index: lib/Analysis/Consumed.cpp =================================================================== --- lib/Analysis/Consumed.cpp +++ lib/Analysis/Consumed.cpp @@ -36,8 +36,6 @@ #include #include -// TODO: Adjust states of args to constructors in the same way that arguments to -// function calls are handled. // TODO: Use information from tests in for- and while-loop conditional. // TODO: Add notes about the actual and expected state for // TODO: Correctly identify unreachable blocks when chaining boolean operators. @@ -494,8 +492,8 @@ void checkCallability(const PropagationInfo &PInfo, const FunctionDecl *FunDecl, SourceLocation BlameLoc); - bool handleCall(const CallExpr *Call, const Expr *ObjArg, - const FunctionDecl *FunD); + bool handleCall(const Expr *Call, const Expr *ObjArg, + ArrayRef Args, const FunctionDecl *FunD); void VisitBinaryOperator(const BinaryOperator *BinOp); void VisitCallExpr(const CallExpr *Call); @@ -608,22 +606,21 @@ // Factors out common behavior for function, method, and operator calls. // Check parameters and set parameter state if necessary. // Returns true if the state of ObjArg is set, or false otherwise. -bool ConsumedStmtVisitor::handleCall(const CallExpr *Call, const Expr *ObjArg, +bool ConsumedStmtVisitor::handleCall(const Expr *Call, + const Expr *ObjArg, + ArrayRef Args, const FunctionDecl *FunD) { - unsigned Offset = 0; - if (isa(Call) && isa(FunD)) - Offset = 1; // first argument is 'this' - // check explicit parameters - for (unsigned Index = Offset; Index < Call->getNumArgs(); ++Index) { + unsigned Index = 0; + for (const Expr *Arg : Args) { // Skip variable argument lists. - if (Index - Offset >= FunD->getNumParams()) + if (Index >= FunD->getNumParams()) break; - const ParmVarDecl *Param = FunD->getParamDecl(Index - Offset); + const ParmVarDecl *Param = FunD->getParamDecl(Index++); QualType ParamType = Param->getType(); - InfoEntry Entry = findInfo(Call->getArg(Index)); + InfoEntry Entry = findInfo(Arg); if (Entry == PropagationMap.end() || Entry->second.isTest()) continue; @@ -636,7 +633,7 @@ if (ParamState != ExpectedState) Analyzer.WarningsHandler.warnParamTypestateMismatch( - Call->getArg(Index)->getExprLoc(), + Arg->getExprLoc(), stateToString(ExpectedState), stateToString(ParamState)); } @@ -644,10 +641,10 @@ continue; // Adjust state on the caller side. - if (isRValueRef(ParamType)) - setStateForVarOrTmp(StateMap, PInfo, consumed::CS_Consumed); - else if (ReturnTypestateAttr *RT = Param->getAttr()) + if (ReturnTypestateAttr *RT = Param->getAttr()) setStateForVarOrTmp(StateMap, PInfo, mapReturnTypestateAttrState(RT)); + else if (isRValueRef(ParamType) || isConsumableType(ParamType)) + setStateForVarOrTmp(StateMap, PInfo, consumed::CS_Consumed); else if (isPointerOrRef(ParamType) && (!ParamType->getPointeeType().isConstQualified() || isSetOnReadPtrType(ParamType))) @@ -749,7 +746,9 @@ return; } - handleCall(Call, nullptr, FunDecl); + handleCall(Call, nullptr, + llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()), + FunDecl); propagateReturnType(Call, FunDecl); } @@ -773,30 +772,25 @@ QualType ThisType = Constructor->getThisType()->getPointeeType(); - if (!isConsumableType(ThisType)) - return; - - // FIXME: What should happen if someone annotates the move constructor? - if (ReturnTypestateAttr *RTA = Constructor->getAttr()) { - // TODO: Adjust state of args appropriately. - ConsumedState RetState = mapReturnTypestateAttrState(RTA); - PropagationMap.insert(PairType(Call, PropagationInfo(RetState))); - } else if (Constructor->isDefaultConstructor()) { - PropagationMap.insert(PairType(Call, - PropagationInfo(consumed::CS_Consumed))); - } else if (Constructor->isMoveConstructor()) { - copyInfo(Call->getArg(0), Call, CS_Consumed); - } else if (Constructor->isCopyConstructor()) { - // Copy state from arg. If setStateOnRead then set arg to CS_Unknown. - ConsumedState NS = - isSetOnReadPtrType(Constructor->getThisType()) ? - CS_Unknown : CS_None; - copyInfo(Call->getArg(0), Call, NS); - } else { - // TODO: Adjust state of args appropriately. - ConsumedState RetState = mapConsumableAttrState(ThisType); - PropagationMap.insert(PairType(Call, PropagationInfo(RetState))); + if (isConsumableType(ThisType)) { + if (auto *RTA = Constructor->getAttr()) { + ConsumedState RetState = mapReturnTypestateAttrState(RTA); + PropagationMap.insert(PairType(Call, PropagationInfo(RetState))); + } else if (Constructor->isDefaultConstructor()) { + PropagationMap.insert(PairType(Call, + PropagationInfo(consumed::CS_Consumed))); + } else if (Constructor->isMoveConstructor() || + Constructor->isCopyConstructor()) { + copyInfo(Call->getArg(0), Call, CS_None); + } else { + ConsumedState RetState = mapConsumableAttrState(ThisType); + PropagationMap.insert(PairType(Call, PropagationInfo(RetState))); + } } + + handleCall(Call, nullptr, + llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()), + Constructor); } void ConsumedStmtVisitor::VisitCXXMemberCallExpr( @@ -805,7 +799,8 @@ if (!MD) return; - handleCall(Call, Call->getImplicitObjectArgument(), MD); + handleCall(Call, Call->getImplicitObjectArgument(), + llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()), MD); propagateReturnType(Call, MD); } @@ -813,18 +808,19 @@ const CXXOperatorCallExpr *Call) { const auto *FunDecl = dyn_cast_or_null(Call->getDirectCallee()); if (!FunDecl) return; + ArrayRef Args(Call->getArgs(), Call->getNumArgs()); if (Call->getOperator() == OO_Equal) { - ConsumedState CS = getInfo(Call->getArg(1)); - if (!handleCall(Call, Call->getArg(0), FunDecl)) - setInfo(Call->getArg(0), CS); + ConsumedState CS = getInfo(Args[1]); + if (!handleCall(Call, Args[0], Args.drop_front(1), FunDecl)) + setInfo(Args[0], CS); return; } - if (const auto *MCall = dyn_cast(Call)) - handleCall(MCall, MCall->getImplicitObjectArgument(), FunDecl); + if (isa(FunDecl)) + handleCall(Call, Args[0], Args.drop_front(1), FunDecl); else - handleCall(Call, Call->getArg(0), FunDecl); + handleCall(Call, nullptr, Args, FunDecl); propagateReturnType(Call, FunDecl); } @@ -858,9 +854,7 @@ QualType ParamType = Param->getType(); ConsumedState ParamState = consumed::CS_None; - if (const ParamTypestateAttr *PTA = Param->getAttr()) - ParamState = mapParamTypestateAttrState(PTA); - else if (isConsumableType(ParamType)) + if (isConsumableType(ParamType)) ParamState = mapConsumableAttrState(ParamType); else if (isRValueRef(ParamType) && isConsumableType(ParamType->getPointeeType())) @@ -869,6 +863,23 @@ isConsumableType(ParamType->getPointeeType())) ParamState = consumed::CS_Unknown; + if (const ParamTypestateAttr *PTA = Param->getAttr()) { + if (ParamState == CS_None) { + // 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. + Analyzer.WarningsHandler.warnParamAttrForUnconsumableType( + PTA->getLocation(), ParamType.getAsString()); + } + ParamState = mapParamTypestateAttrState(PTA); + } else if (auto *RTA = Param->getAttr()) { + if (ParamState == CS_None) { + Analyzer.WarningsHandler.warnParamAttrForUnconsumableType( + RTA->getLocation(), ParamType.getAsString()); + } + } + if (ParamState != CS_None) StateMap->setState(Param, ParamState); } Index: lib/Sema/AnalysisBasedWarnings.cpp =================================================================== --- lib/Sema/AnalysisBasedWarnings.cpp +++ lib/Sema/AnalysisBasedWarnings.cpp @@ -1914,6 +1914,14 @@ Warnings.emplace_back(std::move(Warning), OptionalNotes()); } + void warnParamAttrForUnconsumableType(SourceLocation Loc, + StringRef TypeName) override { + PartialDiagnosticAt Warning(Loc, S.PDiag( + diag::warn_param_attr_for_unconsumable_type) << TypeName); + + Warnings.emplace_back(std::move(Warning), OptionalNotes()); + } + void warnReturnTypestateForUnconsumableType(SourceLocation Loc, StringRef TypeName) override { PartialDiagnosticAt Warning(Loc, S.PDiag( Index: lib/Sema/SemaDeclAttr.cpp =================================================================== --- lib/Sema/SemaDeclAttr.cpp +++ lib/Sema/SemaDeclAttr.cpp @@ -1161,15 +1161,20 @@ static bool checkForConsumableClass(Sema &S, const CXXMethodDecl *MD, const ParsedAttr &AL) { - QualType ThisType = MD->getThisType()->getPointeeType(); + const CXXRecordDecl *RD = MD->getParent(); - if (const CXXRecordDecl *RD = ThisType->getAsCXXRecordDecl()) { - if (!RD->hasAttr()) { - S.Diag(AL.getLoc(), diag::warn_attr_on_unconsumable_class) << - RD->getNameAsString(); + if (!RD->hasAttr()) { + S.Diag(AL.getLoc(), diag::warn_attr_on_unconsumable_class) << + RD->getNameAsString(); - return false; - } + return false; + } + + if (MD->isStatic() || isa(MD)) { + S.Diag(AL.getLoc(), diag::warn_consumable_attr_on_static) << + (MD->isStatic() ? 0 : 1) << + RD->getNameAsString(); + return false; } return true; Index: test/SemaCXX/warn-consumed-analysis.cpp =================================================================== --- test/SemaCXX/warn-consumed-analysis.cpp +++ test/SemaCXX/warn-consumed-analysis.cpp @@ -19,7 +19,7 @@ ConsumableClass(); ConsumableClass(nullptr_t p) RETURN_TYPESTATE(consumed); ConsumableClass(T val) RETURN_TYPESTATE(unconsumed); - ConsumableClass(ConsumableClass &other); + ConsumableClass(const ConsumableClass &other); ConsumableClass(ConsumableClass &&other); ConsumableClass& operator=(ConsumableClass &other); @@ -53,10 +53,15 @@ public: DestructorTester(); DestructorTester(int); + DestructorTester(nullptr_t) RETURN_TYPESTATE(unconsumed); + DestructorTester(DestructorTester &&); void operator*() CALLABLE_WHEN("unconsumed"); ~DestructorTester() CALLABLE_WHEN("consumed"); + + static void byVal(DestructorTester); + static void byValMarkUnconsumed(DestructorTester RETURN_TYPESTATE(unconsumed)); }; void baf0(const ConsumableClass var); @@ -120,6 +125,19 @@ expected-warning {{invalid invocation of method '~DestructorTester' on object 'D1' while it is in the 'unconsumed' state}} } +void testDestructionByVal() { + { + // both the var and the temporary are consumed: + DestructorTester D0(nullptr); + DestructorTester::byVal((DestructorTester &&)D0); + } + { + // the var is consumed but the temporary isn't: + DestructorTester D1(nullptr); + DestructorTester::byValMarkUnconsumed((DestructorTester &&)D1); // expected-warning {{invalid invocation of method '~DestructorTester' on a temporary object while it is in the 'unconsumed' state}} + } +} + void testTempValue() { *ConsumableClass(); // expected-warning {{invalid invocation of method 'operator*' on a temporary object while it is in the 'consumed' state}} } @@ -413,10 +431,15 @@ Param.consume(); } +void testRvalueRefParamReturnTypestateCallee(ConsumableClass &&Param RETURN_TYPESTATE(unconsumed)) { + Param.unconsume(); +} + void testParamReturnTypestateCaller() { ConsumableClass var; testParamReturnTypestateCallee(true, var); + testRvalueRefParamReturnTypestateCallee((ConsumableClass &&)var); *var; } @@ -480,6 +503,9 @@ baf2(&var); *var; + + baf3(var); + *var; baf4(var); *var; // expected-warning {{invalid invocation of method 'operator*' on object 'var' while it is in the 'unknown' state}} @@ -950,3 +976,52 @@ } } // end namespace PR18260 +namespace NonConstCopyConstructor { + struct CONSUMABLE(unknown) Foo { + Foo() RETURN_TYPESTATE(unconsumed); + Foo(Foo &); + void unconsumedCall() const CALLABLE_WHEN(unconsumed); + }; + void test() { + Foo foo; + Foo bar(foo); + foo.unconsumedCall(); // expected-warning {{invalid invocation of method 'unconsumedCall' on object 'foo' while it is in the 'unknown' state}} + } +} // end namespace NonConstCopyConstructor + +namespace ParamAttributesOnConstructor { + struct CONSUMABLE(unknown) Foo { + Foo() RETURN_TYPESTATE(consumed); + Foo(PARAM_TYPESTATE(consumed) RETURN_TYPESTATE(unconsumed) Foo &); + void unconsumedCall() const CALLABLE_WHEN(unconsumed); + }; + void test() { + Foo foo; + Foo bar(foo); + foo.unconsumedCall(); + Foo baz(foo); // expected-warning {{argument not in expected state; expected 'consumed', observed 'unconsumed'}} + } +} // end namespace NonConstCopyConstructor + +namespace CustomConstructors { + struct Foo { + Foo(int, ConsumableClass &&c); + Foo(int *, RETURN_TYPESTATE(unconsumed) ConsumableClass &&c); + }; + + void test() { + ConsumableClass i1(nullptr); + Foo j1(42, (ConsumableClass &&)i1); + *i1; // expected-warning {{invalid invocation of method 'operator*' on object 'i1' while it is in the 'consumed' state}} + ConsumableClass i2(nullptr); + Foo j2(nullptr, (ConsumableClass &&)i2); + *i2; + } +} + +// This needs to be here instead of warn-consumed-parsing.cpp because these +// warnings come from analysis. +struct CONSUMABLE(unknown) MoreUselessAttrs { + void a([[clang::param_typestate(consumed)]] int) {} // expected-warning {{consumed analysis attribute is attached to a parameter of type 'int': expected a type marked as consumable passed by value, reference, or rvalue reference}} + void b([[clang::return_typestate(consumed)]] MoreUselessAttrs *) {} // expected-warning {{consumed analysis attribute is attached to a parameter of type 'struct MoreUselessAttrs *': expected a type marked as consumable passed by value, reference, or rvalue reference}} +}; Index: test/SemaCXX/warn-consumed-parsing.cpp =================================================================== --- test/SemaCXX/warn-consumed-parsing.cpp +++ test/SemaCXX/warn-consumed-parsing.cpp @@ -62,5 +62,12 @@ Status { }; - +struct CONSUMABLE(unknown) UselessAttrs { + static void x() SET_TYPESTATE(consumed); // expected-warning {{consumed analysis attribute is attached to a static method of class 'UselessAttrs'}} + UselessAttrs() SET_TYPESTATE(consumed); // expected-warning {{consumed analysis attribute is attached to a constructor of class 'UselessAttrs'}} + UselessAttrs(int) CALLABLE_WHEN(consumed); // expected-warning {{consumed analysis attribute is attached to a constructor of class 'UselessAttrs'}} + void operator+(UselessAttrs) SET_TYPESTATE(consumed); // OK + static void *operator new(unsigned long) SET_TYPESTATE(consumed); // expected-warning {{consumed analysis attribute is attached to a static method of class 'UselessAttrs'}} +}; +void operator-(UselessAttrs, UselessAttrs) SET_TYPESTATE(consumed); // expected-warning {{'set_typestate' attribute only applies to functions}}