chriswailes added you to the CC list for the revision "Fixes and features for
Consumed analysis".
Hi delesley, dblaikie,
This patch provides fixes to the code based on feedback, as well as a couple of
new features:
* The same functionality is now supported for both CXXOperatorCallExprs and
CXXMemberCallExprs.
* Factored out some code in the StmtVisitor.
* Removed unnecessary asserts from SemaDeclAttr.cpp
* Remove variables from the state map when their destructors are encountered.
* Re-use an existing warning for misplaced attributes.
* Started adding documentation for the consumed analysis attributes.
http://llvm-reviews.chandlerc.com/D1384
Files:
docs/LanguageExtensions.rst
include/clang/Analysis/Analyses/Consumed.h
include/clang/Basic/Attr.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Analysis/Consumed.cpp
lib/Sema/AnalysisBasedWarnings.cpp
lib/Sema/SemaDeclAttr.cpp
test/SemaCXX/warn-consumed-analysis.cpp
Index: docs/LanguageExtensions.rst
===================================================================
--- docs/LanguageExtensions.rst
+++ docs/LanguageExtensions.rst
@@ -2022,6 +2022,33 @@
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
====================
Index: include/clang/Analysis/Analyses/Consumed.h
===================================================================
--- include/clang/Analysis/Analyses/Consumed.h
+++ include/clang/Analysis/Analyses/Consumed.h
@@ -118,6 +118,9 @@
/// \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 {
@@ -188,7 +191,7 @@
};
/// \brief Check to see if a function tests an object's validity.
- bool isTestingFunction(const CXXMethodDecl *MethodDecl);
+ bool isTestingFunction(const FunctionDecl *FunDecl);
}} // end namespace clang::consumed
Index: include/clang/Basic/Attr.td
===================================================================
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -932,7 +932,7 @@
def Consumes : InheritableAttr {
let Spellings = [GNU<"consumes">];
- let Subjects = [CXXMethod];
+ let Subjects = [CXXMethod, CXXConstructor];
}
def TestsConsumed : InheritableAttr {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2189,9 +2189,6 @@
def warn_use_of_temp_while_consumed : Warning<
"invocation of method '%0' on a temporary object while it is in the "
"'consumed' state">, InGroup<Consumed>, DefaultIgnore;
-def warn_uniqueness_attribute_wrong_decl_type : Warning<
- "%0 attribute only applies to methods">,
- InGroup<Consumed>, DefaultIgnore;
// ConsumedStrict warnings
def warn_use_in_unknown_state : Warning<
Index: lib/Analysis/Consumed.cpp
===================================================================
--- lib/Analysis/Consumed.cpp
+++ 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
@@ -91,9 +90,9 @@
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<const Stmt *, PropagationInfo> MapType;
@@ -105,6 +104,9 @@
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);
@@ -128,12 +130,58 @@
ConsumedStmtVisitor(AnalysisDeclContext &AC, ConsumedAnalyzer &Analyzer,
ConsumedStateMap *StateMap)
: AC(AC), Analyzer(Analyzer), StateMap(StateMap) {}
-
+
void reset() {
PropagationMap.clear();
}
};
+// 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<CallableWhenUnconsumedAttr>()) 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;
+
+ default:
+ 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;
+
+ default:
+ break;
+ }
+ }
+}
+
void ConsumedStmtVisitor::forwardInfo(const Stmt *From, const Stmt *To) {
InfoEntry Entry = PropagationMap.find(From);
@@ -278,14 +326,16 @@
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<ConsumesAttr>())
- StateMap->setState(PState.getVar(), consumed::CS_Consumed);
- else if (!Method->isConst())
- StateMap->setState(PState.getVar(), consumed::CS_Unknown);
+ if (PState.IsVar) {
+ if (MethodDecl->hasAttr<ConsumesAttr>())
+ StateMap->setState(PState.getVar(), consumed::CS_Consumed);
+ else if (!MethodDecl->isConst())
+ StateMap->setState(PState.getVar(), consumed::CS_Unknown);
+ }
}
}
@@ -374,58 +424,22 @@
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<CallableWhenUnconsumedAttr>()) {
- 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<ConsumesAttr>()) {
+ // Handle consuming operators.
+ StateMap->setState(PState.getVar(), consumed::CS_Consumed);
+ } else if (const CXXMethodDecl *MethodDecl =
+ dyn_cast_or_null<CXXMethodDecl>(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<CXXMethodDecl>(FunDecl)) {
-
- if (!MethodDecl->isConst() && PState.IsVar)
- StateMap->setState(PState.getVar(), consumed::CS_Unknown);
- }
}
}
}
@@ -505,10 +519,10 @@
};
bool TestedVarsVisitor::VisitCallExpr(CallExpr *Call) {
- if (const CXXMethodDecl *Method =
- dyn_cast_or_null<CXXMethodDecl>(Call->getDirectCallee())) {
+ if (const FunctionDecl *FunDecl =
+ dyn_cast_or_null<FunctionDecl>(Call->getDirectCallee())) {
- if (isTestingFunction(Method)) {
+ if (isTestingFunction(FunDecl)) {
CurrTestLoc = Call->getExprLoc();
IsUsefulConditional = true;
return true;
@@ -521,16 +535,11 @@
}
bool TestedVarsVisitor::VisitDeclRefExpr(DeclRefExpr *DeclRef) {
- if (const VarDecl *Var = dyn_cast_or_null<VarDecl>(DeclRef->getDecl())) {
- if (StateMap->getState(Var) != consumed::CS_None) {
+ if (const VarDecl *Var = dyn_cast_or_null<VarDecl>(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 +646,9 @@
Map[Var] = State;
}
+void ConsumedStateMap::remove(const VarDecl *Var) {
+ Map.erase(Var);
+}
bool ConsumedAnalyzer::isConsumableType(QualType Type) {
const CXXRecordDecl *RD =
@@ -734,20 +746,24 @@
if (CurrStates == NULL)
CurrStates = BlockInfo.getInfo(CurrBlock);
-
+
ConsumedStmtVisitor Visitor(AC, *this, CurrStates);
-
+
// Visit all of the basic block's statements.
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<CFGStmt>().getStmt());
+ break;
+ case CFGElement::AutomaticObjectDtor:
+ CurrStates->remove(BI->castAs<CFGAutomaticObjDtor>().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<IfStmt>(CurrBlock->getTerminator().getStmt())) {
@@ -786,8 +802,8 @@
WarningsHandler.emitDiagnostics();
}
-bool isTestingFunction(const CXXMethodDecl *Method) {
- return Method->hasAttr<TestsUnconsumedAttr>();
+bool isTestingFunction(const FunctionDecl *FunDecl) {
+ return FunDecl->hasAttr<TestsUnconsumedAttr>();
}
}} // end namespace clang::consumed
Index: lib/Sema/AnalysisBasedWarnings.cpp
===================================================================
--- lib/Sema/AnalysisBasedWarnings.cpp
+++ lib/Sema/AnalysisBasedWarnings.cpp
@@ -1551,10 +1551,9 @@
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) {
Index: lib/Sema/SemaDeclAttr.cpp
===================================================================
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -497,8 +497,6 @@
static bool checkGuardedVarAttrCommon(Sema &S, Decl *D,
const AttributeList &Attr) {
- assert(!Attr.isInvalid());
-
if (!checkAttributeNumArgs(S, Attr, 0))
return false;
@@ -537,8 +535,6 @@
static bool checkGuardedByAttrCommon(Sema &S, Decl *D,
const AttributeList &Attr,
Expr* &Arg) {
- assert(!Attr.isInvalid());
-
if (!checkAttributeNumArgs(S, Attr, 1))
return false;
@@ -584,8 +580,6 @@
static bool checkLockableAttrCommon(Sema &S, Decl *D,
const AttributeList &Attr) {
- assert(!Attr.isInvalid());
-
if (!checkAttributeNumArgs(S, Attr, 0))
return false;
@@ -618,8 +612,6 @@
static void handleNoThreadSafetyAnalysis(Sema &S, Decl *D,
const AttributeList &Attr) {
- assert(!Attr.isInvalid());
-
if (!checkAttributeNumArgs(S, Attr, 0))
return;
@@ -635,8 +627,6 @@
static void handleNoSanitizeAddressAttr(Sema &S, Decl *D,
const AttributeList &Attr) {
- assert(!Attr.isInvalid());
-
if (!checkAttributeNumArgs(S, Attr, 0))
return;
@@ -653,8 +643,6 @@
static void handleNoSanitizeMemory(Sema &S, Decl *D,
const AttributeList &Attr) {
- assert(!Attr.isInvalid());
-
if (!checkAttributeNumArgs(S, Attr, 0))
return;
@@ -670,8 +658,6 @@
static void handleNoSanitizeThread(Sema &S, Decl *D,
const AttributeList &Attr) {
- assert(!Attr.isInvalid());
-
if (!checkAttributeNumArgs(S, Attr, 0))
return;
@@ -688,8 +674,6 @@
static bool checkAcquireOrderAttrCommon(Sema &S, Decl *D,
const AttributeList &Attr,
SmallVectorImpl<Expr *> &Args) {
- assert(!Attr.isInvalid());
-
if (!checkAttributeAtLeastNumArgs(S, Attr, 1))
return false;
@@ -749,8 +733,6 @@
static bool checkLockFunAttrCommon(Sema &S, Decl *D,
const AttributeList &Attr,
SmallVectorImpl<Expr *> &Args) {
- assert(!Attr.isInvalid());
-
// zero or more arguments ok
// check that the attribute is applied to a function
@@ -824,8 +806,7 @@
static bool checkTryLockFunAttrCommon(Sema &S, Decl *D,
const AttributeList &Attr,
SmallVectorImpl<Expr *> &Args) {
- assert(!Attr.isInvalid());
-
+
if (!checkAttributeAtLeastNumArgs(S, Attr, 1))
return false;
@@ -878,8 +859,6 @@
static bool checkLocksRequiredCommon(Sema &S, Decl *D,
const AttributeList &Attr,
SmallVectorImpl<Expr *> &Args) {
- assert(!Attr.isInvalid());
-
if (!checkAttributeAtLeastNumArgs(S, Attr, 1))
return false;
@@ -925,8 +904,6 @@
static void handleUnlockFunAttr(Sema &S, Decl *D,
const AttributeList &Attr) {
- assert(!Attr.isInvalid());
-
// zero or more arguments ok
if (!isa<FunctionDecl>(D) && !isa<FunctionTemplateDecl>(D)) {
@@ -948,8 +925,6 @@
static void handleLockReturnedAttr(Sema &S, Decl *D,
const AttributeList &Attr) {
- assert(!Attr.isInvalid());
-
if (!checkAttributeNumArgs(S, Attr, 1))
return;
@@ -973,8 +948,6 @@
static void handleLocksExcludedAttr(Sema &S, Decl *D,
const AttributeList &Attr) {
- assert(!Attr.isInvalid());
-
if (!checkAttributeAtLeastNumArgs(S, Attr, 1))
return;
@@ -999,12 +972,11 @@
static void handleConsumesAttr(Sema &S, Decl *D,
const AttributeList &Attr) {
- assert(!Attr.isInvalid());
if (!checkAttributeNumArgs(S, Attr, 0)) return;
if (!(isa<CXXMethodDecl>(D) || isa<CXXConstructorDecl>(D))) {
- S.Diag(Attr.getLoc(), diag::warn_uniqueness_attribute_wrong_decl_type) <<
- Attr.getName();
+ S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
+ Attr.getName() << "methods";
return;
}
@@ -1015,12 +987,11 @@
static void handleCallableWhenUnconsumedAttr(Sema &S, Decl *D,
const AttributeList &Attr) {
- assert(!Attr.isInvalid());
if (!checkAttributeNumArgs(S, Attr, 0)) return;
if (!isa<CXXMethodDecl>(D)) {
- S.Diag(Attr.getLoc(), diag::warn_uniqueness_attribute_wrong_decl_type) <<
- Attr.getName();
+ S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
+ Attr.getName() << "methods";
return;
}
@@ -1031,12 +1002,11 @@
static void handleTestsConsumedAttr(Sema &S, Decl *D,
const AttributeList &Attr) {
- assert(!Attr.isInvalid());
if (!checkAttributeNumArgs(S, Attr, 0)) return;
if (!isa<CXXMethodDecl>(D)) {
- S.Diag(Attr.getLoc(), diag::warn_uniqueness_attribute_wrong_decl_type) <<
- Attr.getName();
+ S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
+ Attr.getName() << "methods";
return;
}
@@ -1047,12 +1017,11 @@
static void handleTestsUnconsumedAttr(Sema &S, Decl *D,
const AttributeList &Attr) {
- assert(!Attr.isInvalid());
if (!checkAttributeNumArgs(S, Attr, 0)) return;
if (!isa<CXXMethodDecl>(D)) {
- S.Diag(Attr.getLoc(), diag::warn_uniqueness_attribute_wrong_decl_type) <<
- Attr.getName();
+ S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) <<
+ Attr.getName() << "methods";
return;
}
Index: test/SemaCXX/warn-consumed-analysis.cpp
===================================================================
--- test/SemaCXX/warn-consumed-analysis.cpp
+++ test/SemaCXX/warn-consumed-analysis.cpp
@@ -27,10 +27,13 @@
template <typename U>
Bar<T>& operator=(Bar<U> &&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);
@@ -98,6 +101,13 @@
} 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 +174,15 @@
*var; // expected-warning {{invocation of method 'operator*' on object 'var' while it is in the 'consumed' state}}
}
+void testConsumes2(void) {
+ Bar<int> 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<int> var;
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
