diff options
| -rw-r--r-- | docs/LanguageExtensions.html | 63 | ||||
| -rw-r--r-- | include/clang/Basic/DiagnosticGroups.td | 3 | ||||
| -rw-r--r-- | include/clang/Basic/DiagnosticSemaKinds.td | 19 | ||||
| -rw-r--r-- | include/clang/Parse/Parser.h | 7 | ||||
| -rw-r--r-- | lib/Parse/ParseDecl.cpp | 80 | ||||
| -rw-r--r-- | lib/Sema/SemaDeclAttr.cpp | 211 | ||||
| -rw-r--r-- | test/SemaCXX/warn-thread-safety.cpp | 518 |
7 files changed, 738 insertions, 163 deletions
diff --git a/docs/LanguageExtensions.html b/docs/LanguageExtensions.html index 0bb64e8dd2..5003d9cad8 100644 --- a/docs/LanguageExtensions.html +++ b/docs/LanguageExtensions.html @@ -1164,12 +1164,12 @@ specify that the pointer must be dereferenced while holding some lock.</p> <h4 id="ts_guardedby">guarded_by(l)</h4> <p>Use <tt>__attribute__((guarded_by(l)))</tt> on a variable declaration to -specify that the variable must be accessed while holding lock l.</p> +specify that the variable must be accessed while holding lock <tt>l</tt>.</p> <h4 id="ts_ptguardedby">pt_guarded_by(l)</h4> <p>Use <tt>__attribute__((pt_guarded_by(l)))</tt> on a pointer declaration to -specify that the pointer must be dereferenced while holding lock l.</p> +specify that the pointer must be dereferenced while holding lock <tt>l</tt>.</p> <h4 id="ts_acquiredbefore">acquired_before(...)</h4> @@ -1189,67 +1189,62 @@ least one argument.</p> <p>Use <tt>__attribute__((exclusive_lock_function(...)))</tt> on a function declaration to specify that the function acquires all listed locks -exclusively. This attribute takes zero or more -arguments: either of lockable type or integers indexing into -function parameters of lockable type. If no arguments are given, the acquired -lock is implicitly <tt>this</tt> of the enclosing object.</p> +exclusively. This attribute takes zero or more arguments: either of lockable +type or integers indexing into function parameters of lockable type. If no +arguments are given, the acquired lock is implicitly <tt>this</tt> of the +enclosing object.</p> <h4 id="ts_slf">shared_lock_function(...)</h4> <p>Use <tt>__attribute__((shared_lock_function(...)))</tt> on a function declaration to specify that the function acquires all listed locks, although - the locks may be shared (e.g. read locks). -This attribute takes zero or more -arguments: either of lockable type or integers indexing into -function parameters of lockable type. If no arguments are given, the acquired -lock is implicitly <tt>this</tt> of the enclosing object.</p> + the locks may be shared (e.g. read locks). This attribute takes zero or more +arguments: either of lockable type or integers indexing into function +parameters of lockable type. If no arguments are given, the acquired lock is +implicitly <tt>this</tt> of the enclosing object.</p> <h4 id="ts_etf">exclusive_trylock_function(...)</h4> <p>Use <tt>__attribute__((exclusive_lock_function(...)))</tt> on a function declaration to specify that the function will try (without blocking) to acquire -all listed locks exclusively. This attribute takes one or more -arguments. The first argument is an integer or boolean value specifying the -return value of a successful lock acquisition. The remaining arugments are -either of lockable type or integers indexing into -function parameters of lockable type. If only one argument is given, the -acquired lock is implicitly <tt>this</tt> of the enclosing object.</p> +all listed locks exclusively. This attribute takes one or more arguments. The +first argument is an integer or boolean value specifying the return value of a +successful lock acquisition. The remaining arugments are either of lockable type +or integers indexing into function parameters of lockable type. If only one +argument is given, the acquired lock is implicitly <tt>this</tt> of the +enclosing object.</p> <h4 id="ts_stf">shared_trylock_function(...)</h4> <p>Use <tt>__attribute__((shared_lock_function(...)))</tt> on a function declaration to specify that the function will try (without blocking) to acquire -all listed locks, although - the locks may be shared (e.g. read locks). -This attribute takes one or more -arguments. The first argument is an integer or boolean value specifying the -return value of a successful lock acquisition. The remaining arugments are -either of lockable type or integers indexing into +all listed locks, although the locks may be shared (e.g. read locks). This +attribute takes one or more arguments. The first argument is an integer or +boolean value specifying the return value of a successful lock acquisition. The +remaining arugments are either of lockable type or integers indexing into function parameters of lockable type. If only one argument is given, the acquired lock is implicitly <tt>this</tt> of the enclosing object.</p> <h4 id="ts_uf">unlock_function(...)</h4> <p>Use <tt>__attribute__((unlock_function(...)))</tt> on a function -declaration to specify that the function release all listed locks. - This attribute takes zero or more -arguments: either of lockable type or integers indexing into -function parameters of lockable type. If no arguments are given, the acquired -lock is implicitly <tt>this</tt> of the enclosing object.</p> +declaration to specify that the function release all listed locks. This +attribute takes zero or more arguments: either of lockable type or integers +indexing into function parameters of lockable type. If no arguments are given, +the acquired lock is implicitly <tt>this</tt> of the enclosing object.</p> <h4 id="ts_lr">lock_returned(l)</h4> <p>Use <tt>__attribute__((lock_returned(l)))</tt> on a function -declaration to specify that the function returns lock l (l must be of lockable -type). This annotation is used -to aid in resolving lock expressions.</p> +declaration to specify that the function returns lock <tt>l</tt> (<tt>l</tt> +must be of lockable type). This annotation is used to aid in resolving lock +expressions.</p> <h4 id="ts_le">locks_excluded(...)</h4> <p>Use <tt>__attribute__((locks_excluded(...)))</tt> on a function declaration -to specify that the function must not be called with the listed locks. -Arguments must be lockable type, and there must be at -least one argument.</p> +to specify that the function must not be called with the listed locks. Arguments +must be lockable type, and there must be at least one argument.</p> <h4 id="ts_elr">exclusive_locks_required(...)</h4> diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td index a300764733..7c5c217ce3 100644 --- a/include/clang/Basic/DiagnosticGroups.td +++ b/include/clang/Basic/DiagnosticGroups.td @@ -274,6 +274,9 @@ def Most : DiagGroup<"most", [ OverloadedVirtual ]>; +// Thread Safety warnings +def : DiagGroup<"thread-safety">; + // -Wall is -Wmost -Wparentheses def : DiagGroup<"all", [Most, Parentheses]>; diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 38db5b3ec6..37409d0900 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -1142,6 +1142,10 @@ def err_attribute_bad_neon_vector_size : Error< "Neon vector size must be 64 or 128 bits">; def err_attribute_argument_not_int : Error< "'%0' attribute requires integer constant">; +def err_attribute_argument_not_class : Error< + "%0 attribute requires arguments that are class type or point to class type">; +def err_attribute_first_argument_not_int_or_bool : Error< + "%0 attribute first argument must be of int or bool type">; def err_attribute_argument_outof_range : Error< "init_priority attribute requires integer constant between " "101 and 65535 inclusive">; @@ -1300,6 +1304,21 @@ def warn_availability_version_ordering : Warning< "feature cannot be %select{introduced|deprecated|obsoleted}0 in %1 version " "%2 before it was %select{introduced|deprecated|obsoleted}3 in version %4; " "attribute ignored">; + +// Thread Safety Attributes +// Errors when parsing the attributes +def err_attribute_argument_out_of_range : Error< + "%0 attribute parameter %1 is out of bounds: " + "%plural{0:no parameters to index into|" + "1:can only be 1, since there is one parameter|" + ":must be between 1 and %2}2">; +def err_attribute_argument_not_lockable : Error< + "%0 attribute requires arguments whose type is annotated " + "with 'lockable' attribute">; +def err_attribute_decl_not_lockable : Error< + "%0 attribute can only be applied in a context annotated " + "with 'lockable' attribute">; + def warn_impcast_vector_scalar : Warning< "implicit conversion turns vector to scalar: %0 to %1">, diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h index af3f40ec40..fc5d11cb6a 100644 --- a/include/clang/Parse/Parser.h +++ b/include/clang/Parse/Parser.h @@ -1691,6 +1691,13 @@ bool ParseAsmOperandsOpt(SmallVectorImpl<IdentifierInfo *> &Names, ParsedAttributes &attrs, SourceLocation *endLoc); + bool IsThreadSafetyAttribute(llvm::StringRef AttrName); + void ParseThreadSafetyAttribute(IdentifierInfo &AttrName, + SourceLocation AttrNameLoc, + ParsedAttributes &Attrs, + SourceLocation *EndLoc); + + void ParseTypeofSpecifier(DeclSpec &DS); void ParseDecltypeSpecifier(DeclSpec &DS); void ParseUnderlyingTypeSpecifier(DeclSpec &DS); diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 1e318fe66e..2a95ecc7c7 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -19,6 +19,7 @@ #include "clang/Sema/PrettyDeclStackTrace.h" #include "RAIIObjectsForParser.h" #include "llvm/ADT/SmallSet.h" +#include "llvm/ADT/StringSwitch.h" using namespace clang; //===----------------------------------------------------------------------===// @@ -121,6 +122,10 @@ void Parser::ParseGNUAttributes(ParsedAttributes &attrs, // Availability attributes have their own grammar. if (AttrName->isStr("availability")) ParseAvailabilityAttribute(*AttrName, AttrNameLoc, attrs, endLoc); + // Thread safety attributes fit into the FIXME case above, so we + // just parse the arguments as a list of expressions + else if (IsThreadSafetyAttribute(AttrName->getName())) + ParseThreadSafetyAttribute(*AttrName, AttrNameLoc, attrs, endLoc); // check if we have a "parameterized" attribute else if (Tok.is(tok::l_paren)) { ConsumeParen(); // ignore the left paren loc for now @@ -650,6 +655,81 @@ void Parser::ParseAvailabilityAttribute(IdentifierInfo &Availability, UnavailableLoc, false, false); } +/// \brief Wrapper around a case statement checking if AttrName is +/// one of the thread safety attributes +bool Parser::IsThreadSafetyAttribute(llvm::StringRef AttrName){ + return llvm::StringSwitch<bool>(AttrName) + .Case("guarded_by", true) + .Case("guarded_var", true) + .Case("pt_guarded_by", true) + .Case("pt_guarded_var", true) + .Case("lockable", true) + .Case("scoped_lockable", true) + .Case("no_thread_safety_analysis", true) + .Case("acquired_after", true) + .Case("acquired_before", true) + .Case("exclusive_lock_function", true) + .Case("shared_lock_function", true) + .Case("exclusive_trylock_function", true) + .Case("shared_trylock_function", true) + .Case("unlock_function", true) + .Case("lock_returned", true) + .Case("locks_excluded", true) + .Case("exclusive_locks_required", true) + .Case("shared_locks_required", true) + .Default(false); +} + +/// \brief Parse the contents of thread safety attributes. These +/// should always be parsed as an expression list. +/// +/// We need to special case the parsing due to the fact that if the first token +/// of the first argument is an identifier, the main parse loop will store +/// that token as a "parameter" and the rest of +/// the arguments will be added to a list of "arguments". However, +/// subsequent tokens in the first argument are lost. We instead parse each +/// argument as an expression and add all arguments to the list of "arguments". +/// In future, we will take advantage of this special case to also +/// deal with some argument scoping issues here (for example, referring to a +/// function parameter in the attribute on that function). +void Parser::ParseThreadSafetyAttribute(IdentifierInfo &AttrName, + SourceLocation AttrNameLoc, + ParsedAttributes &Attrs, + SourceLocation *EndLoc) { + + if (Tok.is(tok::l_paren)) { + SourceLocation LeftParenLoc = Tok.getLocation(); + ConsumeParen(); // ignore the left paren loc for now + + ExprVector ArgExprs(Actions); + bool ArgExprsOk = true; + + // now parse the list of expressions + while (1) { + ExprResult ArgExpr(ParseAssignmentExpression()); + if (ArgExpr.isInvalid()) { + ArgExprsOk = false; + MatchRHSPunctuation(tok::r_paren, LeftParenLoc); + break; + } else { + ArgExprs.push_back(ArgExpr.release()); + } + if (Tok.isNot(tok::comma)) + break; + ConsumeToken(); // Eat the comma, move to the next argument + } + // Match the ')'. + if (ArgExprsOk && Tok.is(tok::r_paren)) { + ConsumeParen(); // ignore the right paren loc for now + Attrs.addNew(&AttrName, AttrNameLoc, 0, AttrNameLoc, 0, SourceLocation(), + ArgExprs.take(), ArgExprs.size()); + } + } else { + Attrs.addNew(&AttrName, AttrNameLoc, 0, AttrNameLoc, + 0, SourceLocation(), 0, 0); + } +} + void Parser::DiagnoseProhibitedAttributes(ParsedAttributesWithRange &attrs) { Diag(attrs.Range.getBegin(), diag::err_attributes_not_allowed) << attrs.Range; diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp index 9fbd73f4b6..0644103b44 100644 --- a/lib/Sema/SemaDeclAttr.cpp +++ b/lib/Sema/SemaDeclAttr.cpp @@ -15,6 +15,7 @@ #include "TargetAttributesSema.h" #include "clang/AST/ASTContext.h" #include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/Expr.h" #include "clang/Basic/SourceManager.h" @@ -27,7 +28,7 @@ using namespace sema; /// These constants match the enumerated choices of /// warn_attribute_wrong_decl_type and err_attribute_wrong_decl_type. -enum AttributeDeclType { +enum AttributeDeclKind { ExpectedFunction, ExpectedUnion, ExpectedVariableOrFunction, @@ -195,6 +196,8 @@ static inline bool isCFStringType(QualType T, ASTContext &Ctx) { return RD->getIdentifier() == &Ctx.Idents.get("__CFString"); } +/// \brief Check if the attribute has exactly as many args as Num. May +/// output an error. static bool checkAttributeNumArgs(Sema &S, const AttributeList &Attr, unsigned int Num) { if (Attr.getNumArgs() != Num) { @@ -205,31 +208,16 @@ static bool checkAttributeNumArgs(Sema &S, const AttributeList &Attr, return true; } -/// -/// \brief Check the total number of argumenation, whether parsed by clang -/// as arguments or parameters. Outputs a warning. -/// \return false if the number of argumenation units does not match expectation -/// -static bool checkAttributeNumArgsPlusParams(Sema &S, const AttributeList &Attr, - unsigned int Num, - bool moreok = false) { - unsigned int numArgsPlusParams = 0; - - if (Attr.getParameterName()) - numArgsPlusParams++; - numArgsPlusParams += Attr.getNumArgs(); - - if (moreok && numArgsPlusParams < Num) { +/// \brief Check if the attribute has at least as many args as Num. May +/// output an error. +static bool checkAttributeAtLeastNumArgs(Sema &S, const AttributeList &Attr, + unsigned int Num) { + if (Attr.getNumArgs() < Num) { S.Diag(Attr.getLoc(), diag::err_attribute_too_few_arguments) << Num; return false; } - if (!moreok && numArgsPlusParams != Num) { - S.Diag(Attr.getLoc(), diag::err_attribute_wrong_number_arguments) << Num; - return false; - } - return true; } @@ -246,6 +234,12 @@ static bool mayBeSharedVariable(const Decl *D) { return false; } +/// \brief Check if the passed-in expression is of type int or bool. +static bool isIntOrBool(Expr *Exp) { + QualType QT = Exp->getType(); + return QT->isBooleanType() || QT->isIntegerType(); +} + /// /// \brief Check if passed in Decl is a pointer type. /// Note that this function may produce an error message. @@ -265,6 +259,71 @@ static bool checkIsPointer(Sema &S, const Decl *D, const AttributeList &Attr) { return false; } +/// \brief Checks that the passed in QualType either is of RecordType or points +/// to RecordType. Returns the relevant RecordType, null if it does not exit. +const RecordType *getRecordType(QualType QT) { + const RecordType *RT = QT->getAs<RecordType>(); + // now check if we point to record type + if(!RT && QT->isPointerType()){ + QualType PT = QT->getAs<PointerType>()->getPointeeType(); + RT = PT->getAs<RecordType>(); + } + return RT; +} + +/// \brief Thread Safety Analysis: Checks that all attribute arguments, starting +/// from Sidx, resolve to a lockable object. May flag an error. +static bool checkAttrArgsAreLockableObjs(Sema & S, Decl *D, + const AttributeList & Attr, + int Sidx = 0, + bool ParamIdxOk = false) { + for(unsigned int Idx = Sidx; Idx < Attr.getNumArgs(); ++Idx) { + Expr *ArgExp = Attr.getArg(Idx); + if (ArgExp->isTypeDependent()) + continue; + + QualType Arg_QT = ArgExp->getType(); + + // Get record type. + // first see if we can just cast to record type, or point to record type + const RecordType *RT = getRecordType(Arg_QT); + + // now check if we idx into a record type function param + if (!RT && ParamIdxOk) { + FunctionDecl *FD = dyn_cast <FunctionDecl>(D); + IntegerLiteral *IL = dyn_cast<IntegerLiteral>(ArgExp); + if(FD && IL) { + unsigned int NumParams = FD->getNumParams(); + llvm::APInt ArgValue = IL->getValue(); + uint64_t ParamIdx_from1 = ArgValue.getZExtValue(); + uint64_t ParamIdx_from0 = ParamIdx_from1 - 1; + if(!ArgValue.isStrictlyPositive() || ParamIdx_from1 > NumParams) { + S.Diag(Attr.getLoc(), diag::err_attribute_argument_out_of_range) + << Attr.getName() << Idx + 1 << NumParams; + return false; + } + Arg_QT = FD->getParamDecl(ParamIdx_from0)->getType(); + RT = getRecordType(Arg_QT); + } + } + + // Flag error if could not get record type for this argument + if (!RT) { + S.Diag(Attr.getLoc(), diag::err_attribute_argument_not_class) + << Attr.getName(); + return false; + } + + // Flag error if the type is not lockable + if (!RT->getDecl()->getAttr<LockableAttr>()) { + S.Diag(Attr.getLoc(), diag::err_attribute_argument_not_lockable) + << Attr.getName(); + return false; + } + } + return true; +} + //===----------------------------------------------------------------------===// // Attribute Implementations //===----------------------------------------------------------------------===// @@ -283,7 +342,7 @@ static void handleGuardedVarAttr(Sema &S, Decl *D, const AttributeList &Attr, // D must be either a member field or global (potentially shared) variable. if (!mayBeSharedVariable(D)) { S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) - << Attr.getName() << 15; /*fields and global vars*/; + << Attr.getName() << ExpectedFieldOrGlobalVar; return; } @@ -297,22 +356,26 @@ static void handleGuardedVarAttr(Sema &S, Decl *D, const AttributeList &Attr, } static void handleGuardedByAttr(Sema &S, Decl *D, const AttributeList &Attr, - bool pointer = false) { + bool pointer = false) { assert(!Attr.isInvalid()); - if (!checkAttributeNumArgsPlusParams(S, Attr, 1)) + if (!checkAttributeNumArgs(S, Attr, 1)) return; // D must be either a member field or global (potentially shared) variable. if (!mayBeSharedVariable(D)) { S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) - << Attr.getName() << 15; /*fields and global vars*/; + << Attr.getName() << ExpectedFieldOrGlobalVar; return; } if (pointer && !checkIsPointer(S, D, Attr)) return; + // check that all arguments are lockable objects + if (!checkAttrArgsAreLockableObjs(S, D, Attr)) + return; + if (pointer) D->addAttr(::new (S.Context) PtGuardedByAttr(Attr.getLoc(), S.Context)); else @@ -346,7 +409,7 @@ static void handleNoThreadSafetyAttr(Sema &S, Decl *D, if (!checkAttributeNumArgs(S, Attr, 0)) return; - if (!isFunction(D)) { + if (!isa<FunctionDecl>(D) && !isa<FunctionTemplateDecl>(D)) { S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) << Attr.getName() << ExpectedFunctionOrMethod; return; @@ -360,16 +423,32 @@ static void handleAcquireOrderAttr(Sema &S, Decl *D, const AttributeList &Attr, bool before) { assert(!Attr.isInvalid()); - if (!checkAttributeNumArgsPlusParams(S, Attr, 1, /*moreok=*/true)) + if (!checkAttributeAtLeastNumArgs(S, Attr, 1)) return; // D must be either a member field or global (potentially shared) variable. - if (!mayBeSharedVariable(D)) { + ValueDecl *VD = dyn_cast<ValueDecl>(D); + if (!VD || !mayBeSharedVariable(D)) { S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) - << Attr.getName() << 15; /*fields and global vars*/; + << Attr.getName() << ExpectedFieldOrGlobalVar; return; } + // Check that this attribute only applies to lockable types + QualType QT = VD->getType(); + if (!QT->isDependentType()) { + const RecordType *RT = getRecordType(QT); + if (!RT || !RT->getDecl()->getAttr<LockableAttr>()) { + S.Diag(Attr.getLoc(), diag::err_attribute_decl_not_lockable) + << Attr.getName(); + return; + } + } + + // check that all arguments are lockable objects + if (!checkAttrArgsAreLockableObjs(S, D, Attr)) + return; + if (before) D->addAttr(::new (S.Context) AcquiredBeforeAttr(Attr.getLoc(), S.Context)); else @@ -377,17 +456,22 @@ static void handleAcquireOrderAttr(Sema &S, Decl *D, const AttributeList &Attr, } static void handleLockFunAttr(Sema &S, Decl *D, const AttributeList &Attr, - bool exclusive = false) { + bool exclusive = false) { assert(!Attr.isInvalid()); // zero or more arguments ok - if (!isFunction(D)) { + // check that the attribute is applied to a function + if (!isa<FunctionDecl>(D) && !isa<FunctionTemplateDecl>(D)) { S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) << Attr.getName() << ExpectedFunctionOrMethod; return; } + // check that all arguments are lockable objects + if (!checkAttrArgsAreLockableObjs(S, D, Attr, 0, /*ParamIdxOk=*/true)) + return; + if (exclusive) D->addAttr(::new (S.Context) ExclusiveLockFunctionAttr(Attr.getLoc(), S.Context)); @@ -397,93 +481,118 @@ static void handleLockFunAttr(Sema &S, Decl *D, const AttributeList &Attr, } static void handleTrylockFunAttr(Sema &S, Decl *D, const AttributeList &Attr, - bool exclusive = false) { + bool exclusive = false) { assert(!Attr.isInvalid()); - if (!checkAttributeNumArgsPlusParams(S, Attr, 1, /*moreok=*/true)) + if (!checkAttributeAtLeastNumArgs(S, Attr, 1)) return; - if (!isFunction(D)) { + + if (!isa<FunctionDecl>(D) && !isa<FunctionTemplateDecl>(D)) { S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) << Attr.getName() << ExpectedFunctionOrMethod; return; } + if (!isIntOrBool(Attr.getArg(0))) { + S.Diag(Attr.getLoc(), diag::err_attribute_first_argument_not_int_or_bool) + << Attr.getName(); + return; + } + + // check that all arguments are lockable objects + if (!checkAttrArgsAreLockableObjs(S, D, Attr, 1)) + return; + if (exclusive) D->addAttr(::new (S.Context) ExclusiveTrylockFunctionAttr(Attr.getLoc(), - S.Context)); + S.Context)); else D->addAttr(::new (S.Context) SharedTrylockFunctionAttr(Attr.getLoc(), - S.Context)); - + S.Context)); } static void handleLocksRequiredAttr(Sema &S, Decl *D, const AttributeList &Attr, - bool exclusive = false) { + bool exclusive = false) { assert(!Attr.isInvalid()); - if (!checkAttributeNumArgsPlusParams(S, Attr, 1, /*moreok=*/true)) + if (!checkAttributeAtLeastNumArgs(S, Attr, 1)) return; - if (!isFunction(D)) { + if (!isa<FunctionDecl>(D) && !isa<FunctionTemplateDecl>(D)) { S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) << Attr.getName() << ExpectedFunctionOrMethod; return; } + // check that all arguments are lockable objects + if (!checkAttrArgsAreLockableObjs(S, D, Attr)) + return; + if (exclusive) D->addAttr(::new (S.Context) ExclusiveLocksRequiredAttr(Attr.getLoc(), - S.Context)); + S.Context)); else D->addAttr(::new (S.Context) SharedLocksRequiredAttr(Attr.getLoc(), - S.Context)); + S.Context)); } - static void handleUnlockFunAttr(Sema &S, Decl *D, - const AttributeList &Attr) { + const AttributeList &Attr) { assert(!Attr.isInvalid()); // zero or more arguments ok - if (!isFunction(D)) { + if (!isa<FunctionDecl>(D) && !isa<FunctionTemplateDecl>(D)) { S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) << Attr.getName() << ExpectedFunctionOrMethod; return; } + // check that all arguments are lockable objects + if (!checkAttrArgsAreLockableObjs(S, D, Attr, 0, /*ParamIdxOk=*/true)) + return; + D->addAttr(::new (S.Context) UnlockFunctionAttr(Attr.getLoc(), S.Context)); } static void handleLockReturnedAttr(Sema &S, Decl *D, - const AttributeList &Attr) { + const AttributeList &Attr) { assert(!Attr.isInvalid()); - if (!checkAttributeNumArgsPlusParams(S, Attr, 1)) + if (!checkAttributeNumArgs(S, Attr, 1)) return; - if (!isFunction(D)) { + if (!isa<FunctionDecl>(D) && !isa<FunctionTemplateDecl>(D)) { S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) << Attr.getName() << ExpectedFunctionOrMethod; return; } + // check that all arguments are lockable objects + if (!checkAttrArgsAreLockableObjs(S, D, Attr)) + return; + D->addAttr(::new (S.Context) LockReturnedAttr(Attr.getLoc(), S.Context)); } static void handleLocksExcludedAttr(Sema &S, Decl *D, - const AttributeList &Attr) { + const AttributeList &Attr) { assert(!Attr.isInvalid()); - if (!checkAttributeNumArgsPlusParams(S, Attr, 1, /*moreok=*/true)) + if (!checkAttributeAtLeastNumArgs(S, Attr, 1)) return; - if (!isFunction(D)) { + if (!isa<FunctionDecl>(D) && !isa<FunctionTemplateDecl>(D)) { S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type) << Attr.getName() << ExpectedFunctionOrMethod; return; } + // check that all arguments are lockable objects + if (!checkAttrArgsAreLockableObjs(S, D, Attr)) + return; + D->addAttr(::new (S.Context) LocksExcludedAttr(Attr.getLoc(), S.Context)); } diff --git a/test/SemaCXX/warn-thread-safety.cpp b/test/SemaCXX/warn-thread-safety.cpp index 4958d2ed0f..c5523e2a13 100644 --- a/test/SemaCXX/warn-thread-safety.cpp +++ b/test/SemaCXX/warn-thread-safety.cpp @@ -1,19 +1,48 @@ -// RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety %s -/** - * Helper fields - */ +//-----------------------------------------// +// Helper fields +//-----------------------------------------// class __attribute__((lockable)) Mu { }; +class UnlockableMu{ +}; + +class MuWrapper { + public: + Mu mu; + Mu getMu() { + return mu; + } + Mu * getMuPointer() { + return μ + } +}; + + +class MuDoubleWrapper { + public: + MuWrapper* muWrapper; + MuWrapper* getWrapper() { + return muWrapper; + } +}; + Mu mu1; +UnlockableMu umu; Mu mu2; +MuWrapper muWrapper; +MuDoubleWrapper muDoubleWrapper; +Mu* muPointer; +Mu ** muDoublePointer = & muPointer; +Mu& muRef = mu1; -/*********************************** - * No Thread Safety Analysis (noanal) - ***********************************/ +//-----------------------------------------// +// No Thread Safety Analysis (noanal) // +//-----------------------------------------// // FIXME: Right now we cannot parse attributes put on function definitions // We would like to patch this at some point. @@ -53,9 +82,9 @@ void noanal_fun_params(int lvar __attribute__((no_thread_safety_analysis))); // expected-warning {{'no_thread_safety_analysis' attribute only applies to functions and methods}} -/*********************************** - * Guarded Var Attribute (gv) - ***********************************/ +//-----------------------------------------// +// Guarded Var Attribute (gv) +//-----------------------------------------// #if !__has_attribute(guarded_var) #error "Should support guarded_var attribute" @@ -89,9 +118,9 @@ int gv_testfn(int y){ return x; } -/*********************************** - * Pt Guarded Var Attribute (pgv) - ***********************************/ +//-----------------------------------------// +// Pt Guarded Var Attribute (pgv) +//-----------------------------------------// //FIXME: add support for boost::scoped_ptr<int> fancyptr and references @@ -133,9 +162,9 @@ void pgv_testfn(int y){ delete x; } -/*********************************** - * Lockable Attribute (l) - ***********************************/ +//-----------------------------------------// +// Lockable Attribute (l) +//-----------------------------------------// //FIXME: In future we may want to add support for structs, ObjC classes, etc. @@ -175,9 +204,9 @@ void l_function_params(int lvar __attribute__((lockable))); // \ expected-warning {{'lockable' attribute only applies to classes}} -/*********************************** - * Scoped Lockable Attribute (sl) - ***********************************/ +//-----------------------------------------// +// Scoped Lockable Attribute (sl) +//-----------------------------------------// #if !__has_attribute(scoped_lockable) #error "Should support scoped_lockable attribute" @@ -215,11 +244,11 @@ void sl_function_params(int lvar __attribute__((scoped_lockable))); // \ expected-warning {{'scoped_lockable' attribute only applies to classes}} -/*********************************** - * Guarded By Attribute (gb) - ***********************************/ +//-----------------------------------------// +// Guarded By Attribute (gb) +//-----------------------------------------// -// FIXME: Would we like this attribute to take more than 1 arg? +// FIXME: Eventually, would we like this attribute to take more than 1 arg? #if !__has_attribute(guarded_by) #error "Should support guarded_by attribute" @@ -258,24 +287,36 @@ int gb_testfn(int y){ return x; } -//2.Deal with argument parsing: -// grab token stream parsing from C++0x branch -// possibly create new, more permissive category for gcc attributes - -//foo -//foo.bar -//foo.bar->baz -//foo.bar()->baz()->a -//&foo -//*foo +//2. Check argument parsing. + +// legal attribute arguments +int gb_var_arg_1 __attribute__((guarded_by(muWrapper.mu))); +int gb_var_arg_2 __attribute__((guarded_by(muDoubleWrapper.muWrapper->mu))); +int gb_var_arg_3 __attribute__((guarded_by(muWrapper.getMu()))); +int gb_var_arg_4 __attribute__((guarded_by(*muWrapper.getMuPointer()))); +int gb_var_arg_5 __attribute__((guarded_by(&mu1))); +int gb_var_arg_6 __attribute__((guarded_by(muRef))); +int gb_var_arg_7 __attribute__((guarded_by(muDoubleWrapper.getWrapper()->getMu()))); +int gb_var_arg_8 __attribute__((guarded_by(muPointer))); + + +// illegal attribute arguments +int gb_var_arg_bad_1 __attribute__((guarded_by(1))); // \ + expected-error {{'guarded_by' attribute requires arguments that are class type or point to class type}} +int gb_var_arg_bad_2 __attribute__((guarded_by("mu"))); // \ + expected-error {{'guarded_by' attribute requires arguments that are class type or point to class type}} +int gb_var_arg_bad_3 __attribute__((guarded_by(muDoublePointer))); // \ + expected-error {{'guarded_by' attribute requires arguments that are class type or point to class type}} +int gb_var_arg_bad_4 __attribute__((guarded_by(umu))); // \ + expected-error {{'guarded_by' attribute requires arguments whose type is annotated with 'lockable' attribute}} //3. // Thread Safety analysis tests -/*********************************** - * Pt Guarded By Attribute (pgb) - ***********************************/ +//-----------------------------------------// +// Pt Guarded By Attribute (pgb) +//-----------------------------------------// #if !__has_attribute(pt_guarded_by) #error "Should support pt_guarded_by attribute" @@ -317,12 +358,35 @@ void pgb_testfn(int y){ delete x; } -/*********************************** - * Acquired After (aa) - ***********************************/ +//2. Check argument parsing. + +// legal attribute arguments +int * pgb_var_arg_1 __attribute__((pt_guarded_by(muWrapper.mu))); +int * pgb_var_arg_2 __attribute__((pt_guarded_by(muDoubleWrapper.muWrapper->mu))); +int * pgb_var_arg_3 __attribute__((pt_guarded_by(muWrapper.getMu()))); +int * pgb_var_arg_4 __attribute__((pt_guarded_by(*muWrapper.getMuPointer()))); +int * pgb_var_arg_5 __attribute__((pt_guarded_by(&mu1))); +int * pgb_var_arg_6 __attribute__((pt_guarded_by(muRef))); +int * pgb_var_arg_7 __attribute__((pt_guarded_by(muDoubleWrapper.getWrapper()->getMu()))); +int * pgb_var_arg_8 __attribute__((pt_guarded_by(muPointer))); + + +// illegal attribute arguments +int * pgb_var_arg_bad_1 __attribute__((pt_guarded_by(1))); // \ + expected-error {{'pt_guarded_by' attribute requires arguments that are class type or point to class type}} +int * pgb_var_arg_bad_2 __attribute__((pt_guarded_by("mu"))); // \ + expected-error {{'pt_guarded_by' attribute requires arguments that are class type or point to class type}} +int * pgb_var_arg_bad_3 __attribute__((pt_guarded_by(muDoublePointer))); // \ + expected-error {{'pt_guarded_by' attribute requires arguments that are class type or point to class type}} +int * pgb_var_arg_bad_4 __attribute__((pt_guarded_by(umu))); // \ + expected-error {{'pt_guarded_by' attribute requires arguments whose type is annotated with 'lockable' attribute}} + + +//-----------------------------------------// +// Acquired After (aa) +//-----------------------------------------// |