From a33ab6074a2cc60fe895d6669f9ee776c5fea335 Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Fri, 6 Sep 2013 01:28:43 +0000 Subject: [PATCH] Consumed Analysis: The 'consumable' attribute now takes a identifier specifying the default assumed state for objects of this class This information is used for return states and pass-by-value parameter states. Patch by Chris Wailes. Review by DeLesley Hutchins and Aaron Ballman. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@190116 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Analysis/Analyses/Consumed.h | 2 + include/clang/Basic/Attr.td | 3 + lib/Analysis/Consumed.cpp | 110 ++++++++++-------- lib/Sema/SemaDeclAttr.cpp | 27 ++++- .../SemaCXX/warn-consumed-analysis-strict.cpp | 20 ++-- test/SemaCXX/warn-consumed-analysis.cpp | 13 +-- test/SemaCXX/warn-consumed-parsing.cpp | 12 +- 7 files changed, 113 insertions(+), 74 deletions(-) diff --git a/include/clang/Analysis/Analyses/Consumed.h b/include/clang/Analysis/Analyses/Consumed.h index 082baad176..e473d1e427 100644 --- a/include/clang/Analysis/Analyses/Consumed.h +++ b/include/clang/Analysis/Analyses/Consumed.h @@ -190,6 +190,8 @@ namespace consumed { ConsumedState ExpectedReturnState; + void determineExpectedReturnState(AnalysisDeclContext &AC, + const FunctionDecl *D); bool hasConsumableAttributes(const CXXRecordDecl *RD); bool splitState(const CFGBlock *CurrBlock, const ConsumedStmtVisitor &Visitor); diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td index 5e0ecdb16a..48cd640a86 100644 --- a/include/clang/Basic/Attr.td +++ b/include/clang/Basic/Attr.td @@ -931,6 +931,9 @@ def SharedLocksRequired : InheritableAttr { def Consumable : InheritableAttr { let Spellings = [GNU<"consumable">]; let Subjects = [CXXRecord]; + let Args = [EnumArgument<"DefaultState", "ConsumedState", + ["unknown", "consumed", "unconsumed"], + ["Unknown", "Consumed", "Unconsumed"]>]; } def CallableWhenUnconsumed : InheritableAttr { diff --git a/lib/Analysis/Consumed.cpp b/lib/Analysis/Consumed.cpp index 78153a0a68..b540d75898 100644 --- a/lib/Analysis/Consumed.cpp +++ b/lib/Analysis/Consumed.cpp @@ -89,9 +89,25 @@ static bool isTestingFunction(const FunctionDecl *FunDecl) { return FunDecl->hasAttr(); } +static ConsumedState mapConsumableAttrState(const QualType QT) { + assert(isConsumableType(QT)); + + const ConsumableAttr *CAttr = + QT->getAsCXXRecordDecl()->getAttr(); + + switch (CAttr->getDefaultState()) { + case ConsumableAttr::Unknown: + return CS_Unknown; + case ConsumableAttr::Unconsumed: + return CS_Unconsumed; + case ConsumableAttr::Consumed: + return CS_Consumed; + } + llvm_unreachable("invalid enum"); +} + static ConsumedState mapReturnTypestateAttrState(const ReturnTypestateAttr *RTSAttr) { - switch (RTSAttr->getState()) { case ReturnTypestateAttr::Unknown: return CS_Unknown; @@ -402,7 +418,7 @@ void ConsumedStmtVisitor::propagateReturnType(const Stmt *Call, ReturnState = mapReturnTypestateAttrState( Fun->getAttr()); else - ReturnState = CS_Unknown; + ReturnState = mapConsumableAttrState(ReturnType); PropagationMap.insert(PairType(Call, PropagationInfo(ReturnState))); @@ -709,8 +725,18 @@ void ConsumedStmtVisitor::VisitMemberExpr(const MemberExpr *MExpr) { void ConsumedStmtVisitor::VisitParmVarDecl(const ParmVarDecl *Param) { - if (isConsumableType(Param->getType())) - StateMap->setState(Param, consumed::CS_Unknown); + QualType ParamType = Param->getType(); + ConsumedState ParamState = consumed::CS_None; + + if (!(ParamType->isPointerType() || ParamType->isReferenceType()) && + isConsumableType(ParamType)) + ParamState = mapConsumableAttrState(ParamType); + else if (ParamType->isReferenceType() && + isConsumableType(ParamType->getPointeeType())) + ParamState = consumed::CS_Unknown; + + if (ParamState) + StateMap->setState(Param, ParamState); } void ConsumedStmtVisitor::VisitReturnStmt(const ReturnStmt *Ret) { @@ -952,6 +978,35 @@ void ConsumedStateMap::remove(const VarDecl *Var) { Map.erase(Var); } +void ConsumedAnalyzer::determineExpectedReturnState(AnalysisDeclContext &AC, + const FunctionDecl *D) { + QualType ReturnType; + if (const CXXConstructorDecl *Constructor = dyn_cast(D)) { + ASTContext &CurrContext = AC.getASTContext(); + ReturnType = Constructor->getThisType(CurrContext)->getPointeeType(); + } else + ReturnType = D->getCallResultType(); + + if (D->hasAttr()) { + const ReturnTypestateAttr *RTSAttr = D->getAttr(); + + const CXXRecordDecl *RD = ReturnType->getAsCXXRecordDecl(); + if (!RD || !RD->hasAttr()) { + // 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. + WarningsHandler.warnReturnTypestateForUnconsumableType( + RTSAttr->getLocation(), ReturnType.getAsString()); + ExpectedReturnState = CS_None; + } else + ExpectedReturnState = mapReturnTypestateAttrState(RTSAttr); + } else if (isConsumableType(ReturnType)) + ExpectedReturnState = mapConsumableAttrState(ReturnType); + else + ExpectedReturnState = CS_None; +} + bool ConsumedAnalyzer::splitState(const CFGBlock *CurrBlock, const ConsumedStmtVisitor &Visitor) { @@ -1051,52 +1106,7 @@ void ConsumedAnalyzer::run(AnalysisDeclContext &AC) { 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; - } + determineExpectedReturnState(AC, D); BlockInfo = ConsumedBlockInfo(AC.getCFG()); diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp index 97d12d57dc..39347fa394 100644 --- a/lib/Sema/SemaDeclAttr.cpp +++ b/lib/Sema/SemaDeclAttr.cpp @@ -968,7 +968,30 @@ static void handleLocksExcludedAttr(Sema &S, Decl *D, } static void handleConsumableAttr(Sema &S, Decl *D, const AttributeList &Attr) { - if (!checkAttributeNumArgs(S, Attr, 0)) return; + if (!checkAttributeNumArgs(S, Attr, 1)) + return; + + ConsumableAttr::ConsumedState DefaultState; + + if (Attr.isArgIdent(0)) { + StringRef Param = Attr.getArgAsIdent(0)->Ident->getName(); + + if (Param == "unknown") + DefaultState = ConsumableAttr::Unknown; + else if (Param == "consumed") + DefaultState = ConsumableAttr::Consumed; + else if (Param == "unconsumed") + DefaultState = ConsumableAttr::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) << @@ -977,7 +1000,7 @@ static void handleConsumableAttr(Sema &S, Decl *D, const AttributeList &Attr) { } D->addAttr(::new (S.Context) - ConsumableAttr(Attr.getRange(), S.Context, + ConsumableAttr(Attr.getRange(), S.Context, DefaultState, Attr.getAttributeSpellingListIndex())); } diff --git a/test/SemaCXX/warn-consumed-analysis-strict.cpp b/test/SemaCXX/warn-consumed-analysis-strict.cpp index 1bc604de47..98d6894261 100644 --- a/test/SemaCXX/warn-consumed-analysis-strict.cpp +++ b/test/SemaCXX/warn-consumed-analysis-strict.cpp @@ -1,9 +1,9 @@ // RUN: %clang_cc1 -fsyntax-only -verify -Wconsumed-strict -std=c++11 %s #define CALLABLE_WHEN_UNCONSUMED __attribute__ ((callable_when_unconsumed)) -#define CONSUMABLE __attribute__ ((consumable)) +#define CONSUMABLE(state) __attribute__ ((consumable(state))) #define CONSUMES __attribute__ ((consumes)) -#define RETURN_TYPESTATE(State) __attribute__ ((return_typestate(State))) +#define RETURN_TYPESTATE(state) __attribute__ ((return_typestate(state))) #define TESTS_UNCONSUMED __attribute__ ((tests_unconsumed)) #define TEST_VAR(Var) Var.isValid() @@ -11,15 +11,15 @@ typedef decltype(nullptr) nullptr_t; template -class CONSUMABLE ConsumableClass { +class CONSUMABLE(unconsumed) ConsumableClass { T var; public: ConsumableClass(); 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(T val); + ConsumableClass(ConsumableClass &other); + ConsumableClass(ConsumableClass &&other); ConsumableClass& operator=(ConsumableClass &other); ConsumableClass& operator=(ConsumableClass &&other); @@ -187,6 +187,10 @@ void testConstAndNonConstMemberFunctions() { *var; } +void testFunctionParam0(ConsumableClass ¶m) { + *param; // expected-warning {{invocation of method 'operator*' on object 'param' while it is in an unknown state}} +} + void testNoWarnTestFromMacroExpansion() { ConsumableClass var(42); @@ -195,10 +199,6 @@ void testNoWarnTestFromMacroExpansion() { } } -void testFunctionParam(ConsumableClass param) { - *param; // expected-warning {{invocation of method 'operator*' on object 'param' while it is in an unknown state}} -} - void testSimpleForLoop() { ConsumableClass var; diff --git a/test/SemaCXX/warn-consumed-analysis.cpp b/test/SemaCXX/warn-consumed-analysis.cpp index 53a29b128d..8d4bff705c 100644 --- a/test/SemaCXX/warn-consumed-analysis.cpp +++ b/test/SemaCXX/warn-consumed-analysis.cpp @@ -1,23 +1,23 @@ // RUN: %clang_cc1 -fsyntax-only -verify -Wconsumed -std=c++11 %s #define CALLABLE_WHEN_UNCONSUMED __attribute__ ((callable_when_unconsumed)) -#define CONSUMABLE __attribute__ ((consumable)) +#define CONSUMABLE(state) __attribute__ ((consumable(state))) #define CONSUMES __attribute__ ((consumes)) -#define RETURN_TYPESTATE(State) __attribute__ ((return_typestate(State))) +#define RETURN_TYPESTATE(state) __attribute__ ((return_typestate(state))) #define TESTS_UNCONSUMED __attribute__ ((tests_unconsumed)) typedef decltype(nullptr) nullptr_t; template -class CONSUMABLE ConsumableClass { +class CONSUMABLE(unconsumed) ConsumableClass { T var; public: ConsumableClass(); 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(ConsumableClass &other); + ConsumableClass(ConsumableClass &&other); ConsumableClass& operator=(ConsumableClass &other); ConsumableClass& operator=(ConsumableClass &&other); @@ -49,7 +49,6 @@ void baf2(const ConsumableClass *var); 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'}} } @@ -241,7 +240,7 @@ void testFunctionParam(ConsumableClass param) { if (param.isValid()) { *param; } else { - *param; // expected-warning {{invocation of method 'operator*' on object 'param' while it is in the 'consumed' state}} + *param; } param = nullptr; diff --git a/test/SemaCXX/warn-consumed-parsing.cpp b/test/SemaCXX/warn-consumed-parsing.cpp index 4408dfc7d7..001fb866b8 100644 --- a/test/SemaCXX/warn-consumed-parsing.cpp +++ b/test/SemaCXX/warn-consumed-parsing.cpp @@ -1,9 +1,9 @@ // RUN: %clang_cc1 -fsyntax-only -verify -Wconsumed -std=c++11 %s #define CALLABLE_WHEN_UNCONSUMED __attribute__ ((callable_when_unconsumed)) -#define CONSUMABLE __attribute__ ((consumable)) +#define CONSUMABLE(state) __attribute__ ((consumable(state))) #define CONSUMES __attribute__ ((consumes)) -#define RETURN_TYPESTATE(State) __attribute__ ((return_typestate(State))) +#define RETURN_TYPESTATE(state) __attribute__ ((return_typestate(state))) #define TESTS_UNCONSUMED __attribute__ ((tests_unconsumed)) // FIXME: This test is here because the warning is issued by the Consumed @@ -25,15 +25,15 @@ class AttrTester0 { 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 var3 CONSUMABLE(consumed); // 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}} void function2() CALLABLE_WHEN_UNCONSUMED; // expected-warning {{'callable_when_unconsumed' attribute only applies to methods}} -void function3() CONSUMABLE; // expected-warning {{'consumable' attribute only applies to classes}} +void function3() CONSUMABLE(consumed); // expected-warning {{'consumable' attribute only applies to classes}} -class CONSUMABLE AttrTester1 { +class CONSUMABLE(unknown) AttrTester1 { void callableWhenUnconsumed() CALLABLE_WHEN_UNCONSUMED; void consumes() CONSUMES; bool testsUnconsumed() TESTS_UNCONSUMED; @@ -47,3 +47,5 @@ class AttrTester2 { 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}} }; + +class CONSUMABLE(42) AttrTester3; // expected-error {{'consumable' attribute requires an identifier}} -- 2.50.1