From c2ecf0d815450b8a4c8ff3f9f78ecd9bd29b57ab Mon Sep 17 00:00:00 2001 From: DeLesley Hutchins Date: Thu, 22 Aug 2013 20:44:47 +0000 Subject: [PATCH] Update to consumed analysis. Patch by chris.wailes@gmail.com. The following functionality was added: * The same functionality is now supported for both CXXOperatorCallExprs and CXXMemberCallExprs. * Factored out some code in StmtVisitor. * Removed variables from the state map when their destructors are encountered. * Started adding documentation for the consumed analysis attributes. llvm-svn: 189059 --- clang/docs/LanguageExtensions.rst | 27 ++++ clang/include/clang/Analysis/Analyses/Consumed.h | 7 +- clang/include/clang/Basic/Attr.td | 2 +- clang/lib/Analysis/Consumed.cpp | 169 +++++++++++++---------- clang/lib/Sema/AnalysisBasedWarnings.cpp | 7 +- clang/test/SemaCXX/warn-consumed-analysis.cpp | 23 +++ 6 files changed, 150 insertions(+), 85 deletions(-) diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst index 37a587e..dc4563c 100644 --- a/clang/docs/LanguageExtensions.rst +++ b/clang/docs/LanguageExtensions.rst @@ -2022,6 +2022,33 @@ to specify that the function must be called while holding the listed shared locks. Arguments must be lockable type, and there must be at least one argument. +Consumed Annotation Checking +============================ + +Clang supports additional attributes for checking basic resource management +properties, specifically for unique objects that have a single owning reference. +The following attributes are currently supported, although **the implementation +for these annotations is currently in development and are subject to change.** + +``consumes`` +------------ + +Use ``__attribute__((consumes))`` on a method that transitions an object into +the consumed state. + +``callable_when_unconsumed`` +---------------------------- + +Use ``__attribute__((callable_when_unconsumed))`` to indicate that a method may +only be called when the object is not in the consumed state. + +``tests_unconsumed`` +-------------------- + +Use `__attribute__((tests_unconsumed))`` to indicate that a method returns true +if the object is in the unconsumed state. + + Type Safety Checking ==================== diff --git a/clang/include/clang/Analysis/Analyses/Consumed.h b/clang/include/clang/Analysis/Analyses/Consumed.h index 8ff9b6d..e951211 100644 --- a/clang/include/clang/Analysis/Analyses/Consumed.h +++ b/clang/include/clang/Analysis/Analyses/Consumed.h @@ -118,6 +118,9 @@ namespace consumed { /// \brief Set the consumed state of a given variable. void setState(const VarDecl *Var, ConsumedState State); + + /// \brief Remove the variable from our state map. + void remove(const VarDecl *Var); }; class ConsumedBlockInfo { @@ -186,10 +189,6 @@ namespace consumed { /// exactly once. void run(AnalysisDeclContext &AC); }; - - /// \brief Check to see if a function tests an object's validity. - bool isTestingFunction(const CXXMethodDecl *MethodDecl); - }} // end namespace clang::consumed #endif diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index f81098d..fe3b6ee 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -932,7 +932,7 @@ def TestsUnconsumed : InheritableAttr { def Consumes : InheritableAttr { let Spellings = [GNU<"consumes">]; - let Subjects = [CXXMethod]; + let Subjects = [CXXMethod, CXXConstructor]; } def TestsConsumed : InheritableAttr { diff --git a/clang/lib/Analysis/Consumed.cpp b/clang/lib/Analysis/Consumed.cpp index e64a26c..469ba71 100644 --- a/clang/lib/Analysis/Consumed.cpp +++ b/clang/lib/Analysis/Consumed.cpp @@ -30,7 +30,6 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/Support/raw_ostream.h" -// TODO: Add support for methods with CallableWhenUnconsumed. // TODO: Mark variables as Unknown going into while- or for-loops only if they // are referenced inside that block. (Deferred) // TODO: Add a method(s) to identify which method calls perform what state @@ -48,6 +47,10 @@ using namespace consumed; // Key method definition ConsumedWarningsHandlerBase::~ConsumedWarningsHandlerBase() {} +static bool isTestingFunction(const FunctionDecl *FunDecl) { + return FunDecl->hasAttr(); +} + static StringRef stateToString(ConsumedState State) { switch (State) { case consumed::CS_None: @@ -91,9 +94,9 @@ class ConsumedStmtVisitor : public ConstStmtVisitor { StateOrVar.Var = Var; } - ConsumedState getState() { return StateOrVar.State; }; + ConsumedState getState() const { return StateOrVar.State; } - const VarDecl * getVar() { return IsVar ? StateOrVar.Var : NULL; }; + const VarDecl * getVar() const { return IsVar ? StateOrVar.Var : NULL; } }; typedef llvm::DenseMap MapType; @@ -105,6 +108,9 @@ class ConsumedStmtVisitor : public ConstStmtVisitor { ConsumedStateMap *StateMap; MapType PropagationMap; + void checkCallability(const PropagationInfo &PState, + const FunctionDecl *FunDecl, + const CallExpr *Call); void forwardInfo(const Stmt *From, const Stmt *To); bool isLikeMoveAssignment(const CXXMethodDecl *MethodDecl); @@ -134,6 +140,54 @@ public: } }; +// TODO: When we support CallableWhenConsumed this will have to check for +// the different attributes and change the behavior bellow. (Deferred) +void ConsumedStmtVisitor::checkCallability(const PropagationInfo &PState, + const FunctionDecl *FunDecl, + const CallExpr *Call) { + + if (!FunDecl->hasAttr()) return; + + if (PState.IsVar) { + const VarDecl *Var = PState.getVar(); + + switch (StateMap->getState(Var)) { + case CS_Consumed: + Analyzer.WarningsHandler.warnUseWhileConsumed( + FunDecl->getNameAsString(), Var->getNameAsString(), + Call->getExprLoc()); + break; + + case CS_Unknown: + Analyzer.WarningsHandler.warnUseInUnknownState( + FunDecl->getNameAsString(), Var->getNameAsString(), + Call->getExprLoc()); + break; + + case CS_None: + case CS_Unconsumed: + break; + } + + } else { + switch (PState.getState()) { + case CS_Consumed: + Analyzer.WarningsHandler.warnUseOfTempWhileConsumed( + FunDecl->getNameAsString(), Call->getExprLoc()); + break; + + case CS_Unknown: + Analyzer.WarningsHandler.warnUseOfTempInUnknownState( + FunDecl->getNameAsString(), Call->getExprLoc()); + break; + + case CS_None: + case CS_Unconsumed: + break; + } + } +} + void ConsumedStmtVisitor::forwardInfo(const Stmt *From, const Stmt *To) { InfoEntry Entry = PropagationMap.find(From); @@ -278,14 +332,16 @@ void ConsumedStmtVisitor::VisitCXXMemberCallExpr( if (Entry != PropagationMap.end()) { PropagationInfo PState = Entry->second; - if (!PState.IsVar) return; + const CXXMethodDecl *MethodDecl = Call->getMethodDecl(); - const CXXMethodDecl *Method = Call->getMethodDecl(); + checkCallability(PState, MethodDecl, Call); - if (Method->hasAttr()) - StateMap->setState(PState.getVar(), consumed::CS_Consumed); - else if (!Method->isConst()) - StateMap->setState(PState.getVar(), consumed::CS_Unknown); + if (PState.IsVar) { + if (MethodDecl->hasAttr()) + StateMap->setState(PState.getVar(), consumed::CS_Consumed); + else if (!MethodDecl->isConst()) + StateMap->setState(PState.getVar(), consumed::CS_Unknown); + } } } @@ -374,58 +430,22 @@ void ConsumedStmtVisitor::VisitCXXOperatorCallExpr( InfoEntry Entry = PropagationMap.find(Call->getArg(0)); if (Entry != PropagationMap.end()) { - PropagationInfo PState = Entry->second; - // TODO: When we support CallableWhenConsumed this will have to check for - // the different attributes and change the behavior bellow. - // (Deferred) - if (FunDecl->hasAttr()) { - if (PState.IsVar) { - const VarDecl *Var = PState.getVar(); - - switch (StateMap->getState(Var)) { - case CS_Consumed: - Analyzer.WarningsHandler.warnUseWhileConsumed( - FunDecl->getNameAsString(), Var->getNameAsString(), - Call->getExprLoc()); - break; - - case CS_Unknown: - Analyzer.WarningsHandler.warnUseInUnknownState( - FunDecl->getNameAsString(), Var->getNameAsString(), - Call->getExprLoc()); - break; - - default: - break; - } - - } else { - switch (PState.getState()) { - case CS_Consumed: - Analyzer.WarningsHandler.warnUseOfTempWhileConsumed( - FunDecl->getNameAsString(), Call->getExprLoc()); - break; + checkCallability(PState, FunDecl, Call); + + if (PState.IsVar) { + if (FunDecl->hasAttr()) { + // Handle consuming operators. + StateMap->setState(PState.getVar(), consumed::CS_Consumed); + } else if (const CXXMethodDecl *MethodDecl = + dyn_cast_or_null(FunDecl)) { - case CS_Unknown: - Analyzer.WarningsHandler.warnUseOfTempInUnknownState( - FunDecl->getNameAsString(), Call->getExprLoc()); - break; - - default: - break; - } + // Handle non-constant member operators. + if (!MethodDecl->isConst()) + StateMap->setState(PState.getVar(), consumed::CS_Unknown); } } - - // Handle non-constant member operators. - if (const CXXMethodDecl *MethodDecl = - dyn_cast_or_null(FunDecl)) { - - if (!MethodDecl->isConst() && PState.IsVar) - StateMap->setState(PState.getVar(), consumed::CS_Unknown); - } } } } @@ -505,10 +525,10 @@ public: }; bool TestedVarsVisitor::VisitCallExpr(CallExpr *Call) { - if (const CXXMethodDecl *Method = - dyn_cast_or_null(Call->getDirectCallee())) { + if (const FunctionDecl *FunDecl = + dyn_cast_or_null(Call->getDirectCallee())) { - if (isTestingFunction(Method)) { + if (isTestingFunction(FunDecl)) { CurrTestLoc = Call->getExprLoc(); IsUsefulConditional = true; return true; @@ -521,16 +541,11 @@ bool TestedVarsVisitor::VisitCallExpr(CallExpr *Call) { } bool TestedVarsVisitor::VisitDeclRefExpr(DeclRefExpr *DeclRef) { - if (const VarDecl *Var = dyn_cast_or_null(DeclRef->getDecl())) { - if (StateMap->getState(Var) != consumed::CS_None) { + if (const VarDecl *Var = dyn_cast_or_null(DeclRef->getDecl())) + if (StateMap->getState(Var) != consumed::CS_None) Test = VarTestResult(Var, CurrTestLoc, !Invert); - } - - } else { - IsUsefulConditional = false; - } - return IsUsefulConditional; + return true; } bool TestedVarsVisitor::VisitUnaryOperator(UnaryOperator *UnaryOp) { @@ -637,6 +652,9 @@ void ConsumedStateMap::setState(const VarDecl *Var, ConsumedState State) { Map[Var] = State; } +void ConsumedStateMap::remove(const VarDecl *Var) { + Map.erase(Var); +} bool ConsumedAnalyzer::isConsumableType(QualType Type) { const CXXRecordDecl *RD = @@ -741,13 +759,17 @@ void ConsumedAnalyzer::run(AnalysisDeclContext &AC) { for (CFGBlock::const_iterator BI = CurrBlock->begin(), BE = CurrBlock->end(); BI != BE; ++BI) { - if (BI->getKind() == CFGElement::Statement) + switch (BI->getKind()) { + case CFGElement::Statement: Visitor.Visit(BI->castAs().getStmt()); + break; + case CFGElement::AutomaticObjectDtor: + CurrStates->remove(BI->castAs().getVarDecl()); + default: + break; + } } - // TODO: Remove any variables that have reached the end of their - // lifetimes from the state map. (Deferred) - if (const IfStmt *Terminator = dyn_cast_or_null(CurrBlock->getTerminator().getStmt())) { @@ -785,9 +807,4 @@ void ConsumedAnalyzer::run(AnalysisDeclContext &AC) { WarningsHandler.emitDiagnostics(); } - -bool isTestingFunction(const CXXMethodDecl *Method) { - return Method->hasAttr(); -} - }} // end namespace clang::consumed diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 41f4974..74de09a 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -1551,10 +1551,9 @@ clang::sema::AnalysisBasedWarnings::AnalysisBasedWarnings(Sema &s) DefaultPolicy.enableThreadSafetyAnalysis = (unsigned) (D.getDiagnosticLevel(diag::warn_double_lock, SourceLocation()) != DiagnosticsEngine::Ignored); - DefaultPolicy.enableConsumedAnalysis = - (unsigned)(D.getDiagnosticLevel(diag::warn_use_while_consumed, - SourceLocation()) != - DiagnosticsEngine::Ignored); + DefaultPolicy.enableConsumedAnalysis = (unsigned) + (D.getDiagnosticLevel(diag::warn_use_while_consumed, SourceLocation()) != + DiagnosticsEngine::Ignored); } static void flushDiagnostics(Sema &S, sema::FunctionScopeInfo *fscope) { diff --git a/clang/test/SemaCXX/warn-consumed-analysis.cpp b/clang/test/SemaCXX/warn-consumed-analysis.cpp index fc86317..2b40cb2 100644 --- a/clang/test/SemaCXX/warn-consumed-analysis.cpp +++ b/clang/test/SemaCXX/warn-consumed-analysis.cpp @@ -27,10 +27,13 @@ class Bar { template Bar& operator=(Bar &&other); + void operator()(int a) CONSUMES; void operator*(void) const CALLABLE_WHEN_UNCONSUMED; + void unconsumedCall(void) const CALLABLE_WHEN_UNCONSUMED; bool isValid(void) const TESTS_UNCONSUMED; operator bool() const TESTS_UNCONSUMED; + bool operator!=(nullptr_t) const TESTS_UNCONSUMED; void constCall(void) const; void nonconstCall(void); @@ -62,6 +65,10 @@ void testInitialization(void) { } } +void testTempValue(void) { + *Bar(); // expected-warning {{invocation of method 'operator*' on a temporary object while it is in the 'consumed' state}} +} + void testSimpleRValueRefs(void) { Bar var0; Bar var1(42); @@ -98,6 +105,13 @@ void testIfStmt(void) { } else { *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}} } + + if (var != nullptr) { + // Empty + + } else { + *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}} + } } void testCallingConventions(void) { @@ -164,6 +178,15 @@ void testConsumes1(void) { *var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}} } +void testConsumes2(void) { + Bar var(42); + + var.unconsumedCall(); + var(6); + + var.unconsumedCall(); // expected-warning {{invocation of method 'unconsumedCall' on object 'var' while it is in the 'consumed' state}} +} + void testSimpleForLoop(void) { Bar var; -- 2.7.4