[clang][ThreadSafety] Revert stricter typing on trylock attributes #97293
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…alysis.rst (llvm#96494)" This reverts commit 3402620.
…rn types (llvm#95290)" This reverts commit c1bde0a.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Dan McArdle (dmcardle) ChangesThis PR reverts #95290 and the one-liner followup PR #96494. I received some substantial feedback on #95290, which I plan to address in a future PR. I've also received feedback that because the change emits errors where they were not emitted before, we should at least have a flag to disable the stricter warnings. Patch is 32.75 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/97293.diff 10 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b7a2d97f00087..c7bb25d611971 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -71,11 +71,6 @@ C++ Specific Potentially Breaking Changes
To fix this, update libstdc++ to version 14.1.1 or greater.
-- Clang now emits errors when Thread Safety Analysis trylock attributes are
- applied to functions or methods with incompatible return values, such as
- constructors, destructors, and void-returning functions. This only affects the
- ``TRY_ACQUIRE`` and ``TRY_ACQUIRE_SHARED`` attributes (and any synonyms).
-
ABI Changes in This Version
---------------------------
- Fixed Microsoft name mangling of implicitly defined variables used for thread
@@ -751,11 +746,6 @@ Bug Fixes in This Version
- Fixed `static_cast` to array of unknown bound. Fixes (#GH62863).
-- Clang's Thread Safety Analysis now evaluates trylock success arguments of enum
- types rather than silently defaulting to false. This fixes a class of false
- negatives where the analysis failed to detect unchecked access to guarded
- data.
-
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst
index 1513719caa464..dcde0c706c704 100644
--- a/clang/docs/ThreadSafetyAnalysis.rst
+++ b/clang/docs/ThreadSafetyAnalysis.rst
@@ -420,17 +420,10 @@ TRY_ACQUIRE(<bool>, ...), TRY_ACQUIRE_SHARED(<bool>, ...)
*Previously:* ``EXCLUSIVE_TRYLOCK_FUNCTION``, ``SHARED_TRYLOCK_FUNCTION``
These are attributes on a function or method that tries to acquire the given
-capability, and returns a boolean, integer, or pointer value indicating success
-or failure.
-
-The attribute's first argument defines whether a zero or non-zero return value
-indicates success. Syntactically, it accepts ``NULL`` or ``nullptr``, ``bool``
-and ``int`` literals, as well as enumerator values. *The analysis only cares
-whether this success value is zero or non-zero.* This leads to some subtle
-consequences, discussed in the next section.
-
-The remaining arguments are interpreted in the same way as ``ACQUIRE``. See
-:ref:`mutexheader`, below, for example uses.
+capability, and returns a boolean value indicating success or failure.
+The first argument must be ``true`` or ``false``, to specify which return value
+indicates success, and the remaining arguments are interpreted in the same way
+as ``ACQUIRE``. See :ref:`mutexheader`, below, for example uses.
Because the analysis doesn't support conditional locking, a capability is
treated as acquired after the first branch on the return value of a try-acquire
@@ -452,44 +445,6 @@ function.
}
}
-Subtle Consequences of Non-Boolean Success Values
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-The trylock attributes accept non-boolean expressions for the success value, but
-the analysis only cares whether the value is zero or non-zero.
-
-Suppose you define an enum with two non-zero enumerators: ``LockAcquired = 1``
-and ``LockNotAcquired = 2``. If your trylock function returns ``LockAcquired``
-on success and ``LockNotAcquired`` on failure, the analysis may fail to detect
-access to guarded data without holding the mutex because they are both non-zero.
-
-.. code-block:: c++
-
- // *** Beware: this code demonstrates incorrect usage. ***
-
- enum TrylockResult { LockAcquired = 1, LockNotAcquired = 2 };
-
- class CAPABILITY("mutex") Mutex {
- public:
- TrylockResult TryLock() TRY_ACQUIRE(LockAcquired);
- };
-
- Mutex mu;
- int a GUARDED_BY(mu);
-
- void foo() {
- if (mu.TryLock()) { // This branch satisfies the analysis, but permits
- // unguarded access to `a`!
- a = 0;
- mu.Unlock();
- }
- }
-
-It's also possible to return a pointer from the trylock function. Similarly, all
-that matters is whether the success value is zero or non-zero. For instance, a
-success value of `true` means the function returns a non-null pointer on
-success.
-
ASSERT_CAPABILITY(...) and ASSERT_SHARED_CAPABILITY(...)
--------------------------------------------------------
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 5dc36c594bcb7..ffd3d81e588b4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3282,23 +3282,11 @@ def warn_aligned_attr_underaligned : Warning<err_alignas_underaligned.Summary>,
def err_attribute_sizeless_type : Error<
"%0 attribute cannot be applied to sizeless type %1">;
def err_attribute_argument_n_type : Error<
- "%0 attribute requires parameter %1 to be %select{"
- "int or bool"
- "|an integer constant"
- "|a string"
- "|an identifier"
- "|a constant expression"
- "|a builtin function"
- "|nullptr or a bool, int, or enum literal}2">;
+ "%0 attribute requires parameter %1 to be %select{int or bool|an integer "
+ "constant|a string|an identifier|a constant expression|a builtin function}2">;
def err_attribute_argument_type : Error<
- "%0 attribute requires %select{"
- "int or bool"
- "|an integer constant"
- "|a string"
- "|an identifier"
- "|a constant expression"
- "|a builtin function"
- "|a pointer or a bool, int, or enum literal}1">;
+ "%0 attribute requires %select{int or bool|an integer "
+ "constant|a string|an identifier}1">;
def err_attribute_argument_out_of_range : Error<
"%0 attribute requires integer constant between %1 and %2 inclusive">;
def err_init_priority_object_attr : Error<
@@ -3750,8 +3738,7 @@ def warn_attribute_wrong_decl_type : Warning<
"|types and namespaces"
"|variables, functions and classes"
"|kernel functions"
- "|non-K&R-style functions"
- "|functions that return bool, integer, or a pointer type}2">,
+ "|non-K&R-style functions}2">,
InGroup<IgnoredAttributes>;
def err_attribute_wrong_decl_type : Error<warn_attribute_wrong_decl_type.Summary>;
def warn_type_attribute_wrong_type : Warning<
diff --git a/clang/include/clang/Sema/ParsedAttr.h b/clang/include/clang/Sema/ParsedAttr.h
index 65d73f6cd44f6..22cbd0d90ee43 100644
--- a/clang/include/clang/Sema/ParsedAttr.h
+++ b/clang/include/clang/Sema/ParsedAttr.h
@@ -1083,7 +1083,6 @@ enum AttributeArgumentNType {
AANT_ArgumentIdentifier,
AANT_ArgumentConstantExpr,
AANT_ArgumentBuiltinFunction,
- AANT_ArgumentNullptrOrBoolIntOrEnumLiteral,
};
/// These constants match the enumerated choices of
@@ -1102,7 +1101,6 @@ enum AttributeDeclKind {
ExpectedFunctionVariableOrClass,
ExpectedKernelFunction,
ExpectedFunctionWithProtoType,
- ExpectedFunctionReturningPointerBoolIntOrEnum,
};
inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index 550c2a3ffe522..e25b843c9bf83 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -37,7 +37,6 @@
#include "clang/Basic/OperatorKinds.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/Specifiers.h"
-#include "llvm/ADT/APSInt.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/ImmutableMap.h"
@@ -1035,10 +1034,9 @@ class ThreadSafetyAnalyzer {
template <class AttrType>
void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, const Expr *Exp,
- const NamedDecl *D, const CFGBlock *PredBlock,
- const CFGBlock *CurrBlock,
- const ASTContext &TrylockAttrContext,
- Expr *TrylockAttrSuccessValue, bool Neg);
+ const NamedDecl *D,
+ const CFGBlock *PredBlock, const CFGBlock *CurrBlock,
+ Expr *BrE, bool Neg);
const CallExpr* getTrylockCallExpr(const Stmt *Cond, LocalVarContext C,
bool &Negate);
@@ -1361,18 +1359,17 @@ void ThreadSafetyAnalyzer::getMutexIDs(CapExprSet &Mtxs, AttrType *Attr,
const Expr *Exp, const NamedDecl *D,
const CFGBlock *PredBlock,
const CFGBlock *CurrBlock,
- const ASTContext &TrylockAttrContext,
- Expr *TrylockAttrSuccess,
- bool Neg) {
- // Evaluate the trylock's success value as a boolean.
- bool trylockSuccessValue = false;
- if (!TrylockAttrSuccess->EvaluateAsBooleanCondition(
- trylockSuccessValue, TrylockAttrContext,
- /*InConstantContext=*/true)) {
- llvm_unreachable("Trylock success value could not be evaluated.");
- }
-
- const int branchnum = Neg ? !!trylockSuccessValue : !trylockSuccessValue;
+ Expr *BrE, bool Neg) {
+ // Find out which branch has the lock
+ bool branch = false;
+ if (const auto *BLE = dyn_cast_or_null<CXXBoolLiteralExpr>(BrE))
+ branch = BLE->getValue();
+ else if (const auto *ILE = dyn_cast_or_null<IntegerLiteral>(BrE))
+ branch = ILE->getValue().getBoolValue();
+
+ int branchnum = branch ? 0 : 1;
+ if (Neg)
+ branchnum = !branchnum;
// If we've taken the trylock branch, then add the lock
int i = 0;
@@ -1393,15 +1390,8 @@ static bool getStaticBooleanValue(Expr *E, bool &TCond) {
} else if (const auto *ILE = dyn_cast<IntegerLiteral>(E)) {
TCond = ILE->getValue().getBoolValue();
return true;
- } else if (auto *CE = dyn_cast<ImplicitCastExpr>(E)) {
+ } else if (auto *CE = dyn_cast<ImplicitCastExpr>(E))
return getStaticBooleanValue(CE->getSubExpr(), TCond);
- } else if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
- if (auto *ECD = dyn_cast<EnumConstantDecl>(DRE->getDecl())) {
- llvm::APSInt IV = ECD->getInitVal();
- TCond = IV.getBoolValue();
- return true;
- }
- }
return false;
}
@@ -1507,27 +1497,27 @@ void ThreadSafetyAnalyzer::getEdgeLockset(FactSet& Result,
// If the condition is a call to a Trylock function, then grab the attributes
for (const auto *Attr : FunDecl->attrs()) {
switch (Attr->getKind()) {
- case attr::TryAcquireCapability: {
- auto *A = cast<TryAcquireCapabilityAttr>(Attr);
- getMutexIDs(A->isShared() ? SharedLocksToAdd : ExclusiveLocksToAdd, A,
- Exp, FunDecl, PredBlock, CurrBlock, FunDecl->getASTContext(),
- A->getSuccessValue(), Negate);
- break;
- };
- case attr::ExclusiveTrylockFunction: {
- const auto *A = cast<ExclusiveTrylockFunctionAttr>(Attr);
- getMutexIDs(ExclusiveLocksToAdd, A, Exp, FunDecl, PredBlock, CurrBlock,
- FunDecl->getASTContext(), A->getSuccessValue(), Negate);
- break;
- }
- case attr::SharedTrylockFunction: {
- const auto *A = cast<SharedTrylockFunctionAttr>(Attr);
- getMutexIDs(SharedLocksToAdd, A, Exp, FunDecl, PredBlock, CurrBlock,
- FunDecl->getASTContext(), A->getSuccessValue(), Negate);
- break;
- }
- default:
- break;
+ case attr::TryAcquireCapability: {
+ auto *A = cast<TryAcquireCapabilityAttr>(Attr);
+ getMutexIDs(A->isShared() ? SharedLocksToAdd : ExclusiveLocksToAdd, A,
+ Exp, FunDecl, PredBlock, CurrBlock, A->getSuccessValue(),
+ Negate);
+ break;
+ };
+ case attr::ExclusiveTrylockFunction: {
+ const auto *A = cast<ExclusiveTrylockFunctionAttr>(Attr);
+ getMutexIDs(ExclusiveLocksToAdd, A, Exp, FunDecl, PredBlock, CurrBlock,
+ A->getSuccessValue(), Negate);
+ break;
+ }
+ case attr::SharedTrylockFunction: {
+ const auto *A = cast<SharedTrylockFunctionAttr>(Attr);
+ getMutexIDs(SharedLocksToAdd, A, Exp, FunDecl, PredBlock, CurrBlock,
+ A->getSuccessValue(), Negate);
+ break;
+ }
+ default:
+ break;
}
}
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 41489789919d0..b8842e9003e10 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -14,7 +14,6 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTMutationListener.h"
#include "clang/AST/CXXInheritance.h"
-#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DeclTemplate.h"
@@ -26,7 +25,6 @@
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/Cuda.h"
#include "clang/Basic/DarwinSDKInfo.h"
-#include "clang/Basic/DiagnosticSema.h"
#include "clang/Basic/HLSLRuntime.h"
#include "clang/Basic/IdentifierTable.h"
#include "clang/Basic/LangOptions.h"
@@ -67,13 +65,10 @@
#include "llvm/Demangle/Demangle.h"
#include "llvm/IR/Assumptions.h"
#include "llvm/MC/MCSectionMachO.h"
-#include "llvm/Support/Casting.h"
#include "llvm/Support/Error.h"
-#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/MathExtras.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/TargetParser/Triple.h"
-#include <cassert>
#include <optional>
using namespace clang;
@@ -171,6 +166,13 @@ bool Sema::checkStringLiteralArgumentAttr(const ParsedAttr &AL, unsigned ArgNum,
return checkStringLiteralArgumentAttr(AL, ArgExpr, Str, ArgLocation);
}
+/// 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();
+}
+
+
// Check to see if the type is a smart pointer of some kind. We assume
// it's a smart pointer if it defines both operator-> and operator*.
static bool threadSafetyCheckIsSmartPointer(Sema &S, const RecordType* RT) {
@@ -606,31 +608,15 @@ static bool checkTryLockFunAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL,
if (!AL.checkAtLeastNumArgs(S, 1))
return false;
- // The attribute's first argument defines the success value.
- const Expr *SuccessArg = AL.getArgAsExpr(0);
- if (!isa<CXXNullPtrLiteralExpr>(SuccessArg) &&
- !isa<GNUNullExpr>(SuccessArg) && !isa<CXXBoolLiteralExpr>(SuccessArg) &&
- !isa<IntegerLiteral>(SuccessArg) && !SuccessArg->getEnumConstantDecl()) {
+ if (!isIntOrBool(AL.getArgAsExpr(0))) {
S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type)
- << AL << 1 << AANT_ArgumentNullptrOrBoolIntOrEnumLiteral;
+ << AL << 1 << AANT_ArgumentIntOrBool;
return false;
}
- // All remaining arguments must be lockable objects.
+ // check that all arguments are lockable objects
checkAttrArgsAreCapabilityObjs(S, D, AL, Args, 1);
- // The function must return a pointer, boolean, integer, or enum. We already
- // know that `D` is a function because `ExclusiveTrylockFunction` and friends
- // are defined in Attr.td with subject lists that only include functions.
- QualType ReturnType = D->getAsFunction()->getReturnType();
- if (!ReturnType->isPointerType() && !ReturnType->isBooleanType() &&
- !ReturnType->isIntegerType() && !ReturnType->isEnumeralType()) {
- S.Diag(AL.getLoc(), diag::err_attribute_wrong_decl_type)
- << AL << AL.isRegularKeywordAttribute()
- << ExpectedFunctionReturningPointerBoolIntOrEnum;
- return false;
- }
-
return true;
}
diff --git a/clang/test/Sema/attr-capabilities.c b/clang/test/Sema/attr-capabilities.c
index 91a43c79d5b91..5138803bd5eb7 100644
--- a/clang/test/Sema/attr-capabilities.c
+++ b/clang/test/Sema/attr-capabilities.c
@@ -54,14 +54,14 @@ void Func18(void) __attribute__((release_capability())) {} // expected-warning {
void Func19(void) __attribute__((release_shared_capability())) {} // expected-warning {{'release_shared_capability' attribute without capability arguments can only be applied to non-static methods of a class}}
void Func20(void) __attribute__((release_generic_capability())) {} // expected-warning {{'release_generic_capability' attribute without capability arguments can only be applied to non-static methods of a class}}
-int Func21(void) __attribute__((try_acquire_capability(1))) {} // expected-warning {{'try_acquire_capability' attribute without capability arguments can only be applied to non-static methods of a class}}
-int Func22(void) __attribute__((try_acquire_shared_capability(1))) {} // expected-warning {{'try_acquire_shared_capability' attribute without capability arguments can only be applied to non-static methods of a class}}
+void Func21(void) __attribute__((try_acquire_capability(1))) {} // expected-warning {{'try_acquire_capability' attribute without capability arguments can only be applied to non-static methods of a class}}
+void Func22(void) __attribute__((try_acquire_shared_capability(1))) {} // expected-warning {{'try_acquire_shared_capability' attribute without capability arguments can only be applied to non-static methods of a class}}
-int Func23(void) __attribute__((try_acquire_capability(1, GUI))) {}
-int Func24(void) __attribute__((try_acquire_shared_capability(1, GUI))) {}
+void Func23(void) __attribute__((try_acquire_capability(1, GUI))) {}
+void Func24(void) __attribute__((try_acquire_shared_capability(1, GUI))) {}
-int Func25(void) __attribute__((try_acquire_capability())) {} // expected-error {{'try_acquire_capability' attribute takes at least 1 argument}}
-int Func26(void) __attribute__((try_acquire_shared_capability())) {} // expected-error {{'try_acquire_shared_capability' attribute takes at least 1 argument}}
+void Func25(void) __attribute__((try_acquire_capability())) {} // expected-error {{'try_acquire_capability' attribute takes at least 1 argument}}
+void Func26(void) __attribute__((try_acquire_shared_capability())) {} // expected-error {{'try_acquire_shared_capability' attribute takes at least 1 argument}}
// Test that boolean logic works with capability attributes
void Func27(void) __attribute__((requires_capability(!GUI)));
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index a6ddf028e1fad..73cc946ca0ce1 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -1954,125 +1954,6 @@ struct TestTryLock {
} // end namespace TrylockTest
-// Regression test for trylock attributes with an enumerator success argument.
-// Prior versions of the analysis silently failed to evaluate success arguments
-// that were neither `CXXBoolLiteralExpr` nor `IntegerLiteral` and assumed the
-// value was false.
-namespace TrylockSuccessEnumFalseNegative {
-
-enum TrylockResult { Failure = 0, Success = 1 };
-
-class LOCKABLE Mutex {
-public:
- TrylockResult TryLock() EXCLUSIVE_TRYLOCK_FUNCTION(Success);
- void Unlock() EXCLUSIVE_UNLOCK_FUNCTION();
-};
-
-class TrylockTest {
- Mutex mu_;
- int a_ GUARDED_BY(mu_) = 0;
-
- void test_bool_expression() {
- if (!mu_.TryLock()) { // expected-note {{mutex acquired here}}
- a_++; // expected-warning {{writing variable 'a_' requires holding mutex 'mu_' exclusively}}
- mu_.Unlock(); // expected-warning {{releasing mutex 'mu_' that was not held}}
- }
- } // expected-warning {{mutex 'mu_' is not held on every path through here}}
-};
-} // namespace TrylockSuccessEnumFalseNegative
-
-// This test demonstrates that the analysis does not distinguish between
-// different non-zero enumerators.
-namespace TrylockFalseNegativeWhenEnumHasMultipleFailures {
-
-enum TrylockResult { Failure1 = 0, Failure2, Success };
-
-class LOCKABLE Mutex {
-public:
- TrylockResult TryLock() EXCLUSIVE_TRYLOCK_FUNCTION(Success);
- void Unlock() EXCLUSIVE_UNLOCK_FUNCTION();
-};
-
-class TrylockTest {
- Mutex mu_;
- int a_ GUARDED_BY(mu_) = 0;
-
- void test_eq_success() {
- if (mu_.TryLock() == Success) {
- a_++;
- mu_.Unlock();
- }
- }
-
- void test_bool_expression() {
- // This branch checks whether the trylock function returned a non-zero
- // value. This satisfies the analysis, but fails to account for `Failure2`.
- // From the analysis's perspective, `Failure2` and `Success` are equivalent!
- if (mu_.TryLock()) {
- a_++;
- mu_.Unlock();
- }
- }
-};
-} // namespace TrylockSuccessEnumMultipleFailureModesFalseNegative
-
-
-// This test demonstrates that the analysis does not detect when all enumerators
-// are positive, and thus incapable of representing a failure.
-namespac...
[truncated]
|
|
@llvm/pr-subscribers-clang-analysis Author: Dan McArdle (dmcardle) ChangesThis PR reverts #95290 and the one-liner followup PR #96494. I received some substantial feedback on #95290, which I plan to address in a future PR. I've also received feedback that because the change emits errors where they were not emitted before, we should at least have a flag to disable the stricter warnings. Patch is 32.75 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/97293.diff 10 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b7a2d97f00087..c7bb25d611971 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -71,11 +71,6 @@ C++ Specific Potentially Breaking Changes
To fix this, update libstdc++ to version 14.1.1 or greater.
-- Clang now emits errors when Thread Safety Analysis trylock attributes are
- applied to functions or methods with incompatible return values, such as
- constructors, destructors, and void-returning functions. This only affects the
- ``TRY_ACQUIRE`` and ``TRY_ACQUIRE_SHARED`` attributes (and any synonyms).
-
ABI Changes in This Version
---------------------------
- Fixed Microsoft name mangling of implicitly defined variables used for thread
@@ -751,11 +746,6 @@ Bug Fixes in This Version
- Fixed `static_cast` to array of unknown bound. Fixes (#GH62863).
-- Clang's Thread Safety Analysis now evaluates trylock success arguments of enum
- types rather than silently defaulting to false. This fixes a class of false
- negatives where the analysis failed to detect unchecked access to guarded
- data.
-
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst
index 1513719caa464..dcde0c706c704 100644
--- a/clang/docs/ThreadSafetyAnalysis.rst
+++ b/clang/docs/ThreadSafetyAnalysis.rst
@@ -420,17 +420,10 @@ TRY_ACQUIRE(<bool>, ...), TRY_ACQUIRE_SHARED(<bool>, ...)
*Previously:* ``EXCLUSIVE_TRYLOCK_FUNCTION``, ``SHARED_TRYLOCK_FUNCTION``
These are attributes on a function or method that tries to acquire the given
-capability, and returns a boolean, integer, or pointer value indicating success
-or failure.
-
-The attribute's first argument defines whether a zero or non-zero return value
-indicates success. Syntactically, it accepts ``NULL`` or ``nullptr``, ``bool``
-and ``int`` literals, as well as enumerator values. *The analysis only cares
-whether this success value is zero or non-zero.* This leads to some subtle
-consequences, discussed in the next section.
-
-The remaining arguments are interpreted in the same way as ``ACQUIRE``. See
-:ref:`mutexheader`, below, for example uses.
+capability, and returns a boolean value indicating success or failure.
+The first argument must be ``true`` or ``false``, to specify which return value
+indicates success, and the remaining arguments are interpreted in the same way
+as ``ACQUIRE``. See :ref:`mutexheader`, below, for example uses.
Because the analysis doesn't support conditional locking, a capability is
treated as acquired after the first branch on the return value of a try-acquire
@@ -452,44 +445,6 @@ function.
}
}
-Subtle Consequences of Non-Boolean Success Values
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-The trylock attributes accept non-boolean expressions for the success value, but
-the analysis only cares whether the value is zero or non-zero.
-
-Suppose you define an enum with two non-zero enumerators: ``LockAcquired = 1``
-and ``LockNotAcquired = 2``. If your trylock function returns ``LockAcquired``
-on success and ``LockNotAcquired`` on failure, the analysis may fail to detect
-access to guarded data without holding the mutex because they are both non-zero.
-
-.. code-block:: c++
-
- // *** Beware: this code demonstrates incorrect usage. ***
-
- enum TrylockResult { LockAcquired = 1, LockNotAcquired = 2 };
-
- class CAPABILITY("mutex") Mutex {
- public:
- TrylockResult TryLock() TRY_ACQUIRE(LockAcquired);
- };
-
- Mutex mu;
- int a GUARDED_BY(mu);
-
- void foo() {
- if (mu.TryLock()) { // This branch satisfies the analysis, but permits
- // unguarded access to `a`!
- a = 0;
- mu.Unlock();
- }
- }
-
-It's also possible to return a pointer from the trylock function. Similarly, all
-that matters is whether the success value is zero or non-zero. For instance, a
-success value of `true` means the function returns a non-null pointer on
-success.
-
ASSERT_CAPABILITY(...) and ASSERT_SHARED_CAPABILITY(...)
--------------------------------------------------------
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 5dc36c594bcb7..ffd3d81e588b4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3282,23 +3282,11 @@ def warn_aligned_attr_underaligned : Warning<err_alignas_underaligned.Summary>,
def err_attribute_sizeless_type : Error<
"%0 attribute cannot be applied to sizeless type %1">;
def err_attribute_argument_n_type : Error<
- "%0 attribute requires parameter %1 to be %select{"
- "int or bool"
- "|an integer constant"
- "|a string"
- "|an identifier"
- "|a constant expression"
- "|a builtin function"
- "|nullptr or a bool, int, or enum literal}2">;
+ "%0 attribute requires parameter %1 to be %select{int or bool|an integer "
+ "constant|a string|an identifier|a constant expression|a builtin function}2">;
def err_attribute_argument_type : Error<
- "%0 attribute requires %select{"
- "int or bool"
- "|an integer constant"
- "|a string"
- "|an identifier"
- "|a constant expression"
- "|a builtin function"
- "|a pointer or a bool, int, or enum literal}1">;
+ "%0 attribute requires %select{int or bool|an integer "
+ "constant|a string|an identifier}1">;
def err_attribute_argument_out_of_range : Error<
"%0 attribute requires integer constant between %1 and %2 inclusive">;
def err_init_priority_object_attr : Error<
@@ -3750,8 +3738,7 @@ def warn_attribute_wrong_decl_type : Warning<
"|types and namespaces"
"|variables, functions and classes"
"|kernel functions"
- "|non-K&R-style functions"
- "|functions that return bool, integer, or a pointer type}2">,
+ "|non-K&R-style functions}2">,
InGroup<IgnoredAttributes>;
def err_attribute_wrong_decl_type : Error<warn_attribute_wrong_decl_type.Summary>;
def warn_type_attribute_wrong_type : Warning<
diff --git a/clang/include/clang/Sema/ParsedAttr.h b/clang/include/clang/Sema/ParsedAttr.h
index 65d73f6cd44f6..22cbd0d90ee43 100644
--- a/clang/include/clang/Sema/ParsedAttr.h
+++ b/clang/include/clang/Sema/ParsedAttr.h
@@ -1083,7 +1083,6 @@ enum AttributeArgumentNType {
AANT_ArgumentIdentifier,
AANT_ArgumentConstantExpr,
AANT_ArgumentBuiltinFunction,
- AANT_ArgumentNullptrOrBoolIntOrEnumLiteral,
};
/// These constants match the enumerated choices of
@@ -1102,7 +1101,6 @@ enum AttributeDeclKind {
ExpectedFunctionVariableOrClass,
ExpectedKernelFunction,
ExpectedFunctionWithProtoType,
- ExpectedFunctionReturningPointerBoolIntOrEnum,
};
inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index 550c2a3ffe522..e25b843c9bf83 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -37,7 +37,6 @@
#include "clang/Basic/OperatorKinds.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/Specifiers.h"
-#include "llvm/ADT/APSInt.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/ImmutableMap.h"
@@ -1035,10 +1034,9 @@ class ThreadSafetyAnalyzer {
template <class AttrType>
void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, const Expr *Exp,
- const NamedDecl *D, const CFGBlock *PredBlock,
- const CFGBlock *CurrBlock,
- const ASTContext &TrylockAttrContext,
- Expr *TrylockAttrSuccessValue, bool Neg);
+ const NamedDecl *D,
+ const CFGBlock *PredBlock, const CFGBlock *CurrBlock,
+ Expr *BrE, bool Neg);
const CallExpr* getTrylockCallExpr(const Stmt *Cond, LocalVarContext C,
bool &Negate);
@@ -1361,18 +1359,17 @@ void ThreadSafetyAnalyzer::getMutexIDs(CapExprSet &Mtxs, AttrType *Attr,
const Expr *Exp, const NamedDecl *D,
const CFGBlock *PredBlock,
const CFGBlock *CurrBlock,
- const ASTContext &TrylockAttrContext,
- Expr *TrylockAttrSuccess,
- bool Neg) {
- // Evaluate the trylock's success value as a boolean.
- bool trylockSuccessValue = false;
- if (!TrylockAttrSuccess->EvaluateAsBooleanCondition(
- trylockSuccessValue, TrylockAttrContext,
- /*InConstantContext=*/true)) {
- llvm_unreachable("Trylock success value could not be evaluated.");
- }
-
- const int branchnum = Neg ? !!trylockSuccessValue : !trylockSuccessValue;
+ Expr *BrE, bool Neg) {
+ // Find out which branch has the lock
+ bool branch = false;
+ if (const auto *BLE = dyn_cast_or_null<CXXBoolLiteralExpr>(BrE))
+ branch = BLE->getValue();
+ else if (const auto *ILE = dyn_cast_or_null<IntegerLiteral>(BrE))
+ branch = ILE->getValue().getBoolValue();
+
+ int branchnum = branch ? 0 : 1;
+ if (Neg)
+ branchnum = !branchnum;
// If we've taken the trylock branch, then add the lock
int i = 0;
@@ -1393,15 +1390,8 @@ static bool getStaticBooleanValue(Expr *E, bool &TCond) {
} else if (const auto *ILE = dyn_cast<IntegerLiteral>(E)) {
TCond = ILE->getValue().getBoolValue();
return true;
- } else if (auto *CE = dyn_cast<ImplicitCastExpr>(E)) {
+ } else if (auto *CE = dyn_cast<ImplicitCastExpr>(E))
return getStaticBooleanValue(CE->getSubExpr(), TCond);
- } else if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
- if (auto *ECD = dyn_cast<EnumConstantDecl>(DRE->getDecl())) {
- llvm::APSInt IV = ECD->getInitVal();
- TCond = IV.getBoolValue();
- return true;
- }
- }
return false;
}
@@ -1507,27 +1497,27 @@ void ThreadSafetyAnalyzer::getEdgeLockset(FactSet& Result,
// If the condition is a call to a Trylock function, then grab the attributes
for (const auto *Attr : FunDecl->attrs()) {
switch (Attr->getKind()) {
- case attr::TryAcquireCapability: {
- auto *A = cast<TryAcquireCapabilityAttr>(Attr);
- getMutexIDs(A->isShared() ? SharedLocksToAdd : ExclusiveLocksToAdd, A,
- Exp, FunDecl, PredBlock, CurrBlock, FunDecl->getASTContext(),
- A->getSuccessValue(), Negate);
- break;
- };
- case attr::ExclusiveTrylockFunction: {
- const auto *A = cast<ExclusiveTrylockFunctionAttr>(Attr);
- getMutexIDs(ExclusiveLocksToAdd, A, Exp, FunDecl, PredBlock, CurrBlock,
- FunDecl->getASTContext(), A->getSuccessValue(), Negate);
- break;
- }
- case attr::SharedTrylockFunction: {
- const auto *A = cast<SharedTrylockFunctionAttr>(Attr);
- getMutexIDs(SharedLocksToAdd, A, Exp, FunDecl, PredBlock, CurrBlock,
- FunDecl->getASTContext(), A->getSuccessValue(), Negate);
- break;
- }
- default:
- break;
+ case attr::TryAcquireCapability: {
+ auto *A = cast<TryAcquireCapabilityAttr>(Attr);
+ getMutexIDs(A->isShared() ? SharedLocksToAdd : ExclusiveLocksToAdd, A,
+ Exp, FunDecl, PredBlock, CurrBlock, A->getSuccessValue(),
+ Negate);
+ break;
+ };
+ case attr::ExclusiveTrylockFunction: {
+ const auto *A = cast<ExclusiveTrylockFunctionAttr>(Attr);
+ getMutexIDs(ExclusiveLocksToAdd, A, Exp, FunDecl, PredBlock, CurrBlock,
+ A->getSuccessValue(), Negate);
+ break;
+ }
+ case attr::SharedTrylockFunction: {
+ const auto *A = cast<SharedTrylockFunctionAttr>(Attr);
+ getMutexIDs(SharedLocksToAdd, A, Exp, FunDecl, PredBlock, CurrBlock,
+ A->getSuccessValue(), Negate);
+ break;
+ }
+ default:
+ break;
}
}
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 41489789919d0..b8842e9003e10 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -14,7 +14,6 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTMutationListener.h"
#include "clang/AST/CXXInheritance.h"
-#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DeclTemplate.h"
@@ -26,7 +25,6 @@
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/Cuda.h"
#include "clang/Basic/DarwinSDKInfo.h"
-#include "clang/Basic/DiagnosticSema.h"
#include "clang/Basic/HLSLRuntime.h"
#include "clang/Basic/IdentifierTable.h"
#include "clang/Basic/LangOptions.h"
@@ -67,13 +65,10 @@
#include "llvm/Demangle/Demangle.h"
#include "llvm/IR/Assumptions.h"
#include "llvm/MC/MCSectionMachO.h"
-#include "llvm/Support/Casting.h"
#include "llvm/Support/Error.h"
-#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/MathExtras.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/TargetParser/Triple.h"
-#include <cassert>
#include <optional>
using namespace clang;
@@ -171,6 +166,13 @@ bool Sema::checkStringLiteralArgumentAttr(const ParsedAttr &AL, unsigned ArgNum,
return checkStringLiteralArgumentAttr(AL, ArgExpr, Str, ArgLocation);
}
+/// 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();
+}
+
+
// Check to see if the type is a smart pointer of some kind. We assume
// it's a smart pointer if it defines both operator-> and operator*.
static bool threadSafetyCheckIsSmartPointer(Sema &S, const RecordType* RT) {
@@ -606,31 +608,15 @@ static bool checkTryLockFunAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL,
if (!AL.checkAtLeastNumArgs(S, 1))
return false;
- // The attribute's first argument defines the success value.
- const Expr *SuccessArg = AL.getArgAsExpr(0);
- if (!isa<CXXNullPtrLiteralExpr>(SuccessArg) &&
- !isa<GNUNullExpr>(SuccessArg) && !isa<CXXBoolLiteralExpr>(SuccessArg) &&
- !isa<IntegerLiteral>(SuccessArg) && !SuccessArg->getEnumConstantDecl()) {
+ if (!isIntOrBool(AL.getArgAsExpr(0))) {
S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type)
- << AL << 1 << AANT_ArgumentNullptrOrBoolIntOrEnumLiteral;
+ << AL << 1 << AANT_ArgumentIntOrBool;
return false;
}
- // All remaining arguments must be lockable objects.
+ // check that all arguments are lockable objects
checkAttrArgsAreCapabilityObjs(S, D, AL, Args, 1);
- // The function must return a pointer, boolean, integer, or enum. We already
- // know that `D` is a function because `ExclusiveTrylockFunction` and friends
- // are defined in Attr.td with subject lists that only include functions.
- QualType ReturnType = D->getAsFunction()->getReturnType();
- if (!ReturnType->isPointerType() && !ReturnType->isBooleanType() &&
- !ReturnType->isIntegerType() && !ReturnType->isEnumeralType()) {
- S.Diag(AL.getLoc(), diag::err_attribute_wrong_decl_type)
- << AL << AL.isRegularKeywordAttribute()
- << ExpectedFunctionReturningPointerBoolIntOrEnum;
- return false;
- }
-
return true;
}
diff --git a/clang/test/Sema/attr-capabilities.c b/clang/test/Sema/attr-capabilities.c
index 91a43c79d5b91..5138803bd5eb7 100644
--- a/clang/test/Sema/attr-capabilities.c
+++ b/clang/test/Sema/attr-capabilities.c
@@ -54,14 +54,14 @@ void Func18(void) __attribute__((release_capability())) {} // expected-warning {
void Func19(void) __attribute__((release_shared_capability())) {} // expected-warning {{'release_shared_capability' attribute without capability arguments can only be applied to non-static methods of a class}}
void Func20(void) __attribute__((release_generic_capability())) {} // expected-warning {{'release_generic_capability' attribute without capability arguments can only be applied to non-static methods of a class}}
-int Func21(void) __attribute__((try_acquire_capability(1))) {} // expected-warning {{'try_acquire_capability' attribute without capability arguments can only be applied to non-static methods of a class}}
-int Func22(void) __attribute__((try_acquire_shared_capability(1))) {} // expected-warning {{'try_acquire_shared_capability' attribute without capability arguments can only be applied to non-static methods of a class}}
+void Func21(void) __attribute__((try_acquire_capability(1))) {} // expected-warning {{'try_acquire_capability' attribute without capability arguments can only be applied to non-static methods of a class}}
+void Func22(void) __attribute__((try_acquire_shared_capability(1))) {} // expected-warning {{'try_acquire_shared_capability' attribute without capability arguments can only be applied to non-static methods of a class}}
-int Func23(void) __attribute__((try_acquire_capability(1, GUI))) {}
-int Func24(void) __attribute__((try_acquire_shared_capability(1, GUI))) {}
+void Func23(void) __attribute__((try_acquire_capability(1, GUI))) {}
+void Func24(void) __attribute__((try_acquire_shared_capability(1, GUI))) {}
-int Func25(void) __attribute__((try_acquire_capability())) {} // expected-error {{'try_acquire_capability' attribute takes at least 1 argument}}
-int Func26(void) __attribute__((try_acquire_shared_capability())) {} // expected-error {{'try_acquire_shared_capability' attribute takes at least 1 argument}}
+void Func25(void) __attribute__((try_acquire_capability())) {} // expected-error {{'try_acquire_capability' attribute takes at least 1 argument}}
+void Func26(void) __attribute__((try_acquire_shared_capability())) {} // expected-error {{'try_acquire_shared_capability' attribute takes at least 1 argument}}
// Test that boolean logic works with capability attributes
void Func27(void) __attribute__((requires_capability(!GUI)));
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index a6ddf028e1fad..73cc946ca0ce1 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -1954,125 +1954,6 @@ struct TestTryLock {
} // end namespace TrylockTest
-// Regression test for trylock attributes with an enumerator success argument.
-// Prior versions of the analysis silently failed to evaluate success arguments
-// that were neither `CXXBoolLiteralExpr` nor `IntegerLiteral` and assumed the
-// value was false.
-namespace TrylockSuccessEnumFalseNegative {
-
-enum TrylockResult { Failure = 0, Success = 1 };
-
-class LOCKABLE Mutex {
-public:
- TrylockResult TryLock() EXCLUSIVE_TRYLOCK_FUNCTION(Success);
- void Unlock() EXCLUSIVE_UNLOCK_FUNCTION();
-};
-
-class TrylockTest {
- Mutex mu_;
- int a_ GUARDED_BY(mu_) = 0;
-
- void test_bool_expression() {
- if (!mu_.TryLock()) { // expected-note {{mutex acquired here}}
- a_++; // expected-warning {{writing variable 'a_' requires holding mutex 'mu_' exclusively}}
- mu_.Unlock(); // expected-warning {{releasing mutex 'mu_' that was not held}}
- }
- } // expected-warning {{mutex 'mu_' is not held on every path through here}}
-};
-} // namespace TrylockSuccessEnumFalseNegative
-
-// This test demonstrates that the analysis does not distinguish between
-// different non-zero enumerators.
-namespace TrylockFalseNegativeWhenEnumHasMultipleFailures {
-
-enum TrylockResult { Failure1 = 0, Failure2, Success };
-
-class LOCKABLE Mutex {
-public:
- TrylockResult TryLock() EXCLUSIVE_TRYLOCK_FUNCTION(Success);
- void Unlock() EXCLUSIVE_UNLOCK_FUNCTION();
-};
-
-class TrylockTest {
- Mutex mu_;
- int a_ GUARDED_BY(mu_) = 0;
-
- void test_eq_success() {
- if (mu_.TryLock() == Success) {
- a_++;
- mu_.Unlock();
- }
- }
-
- void test_bool_expression() {
- // This branch checks whether the trylock function returned a non-zero
- // value. This satisfies the analysis, but fails to account for `Failure2`.
- // From the analysis's perspective, `Failure2` and `Success` are equivalent!
- if (mu_.TryLock()) {
- a_++;
- mu_.Unlock();
- }
- }
-};
-} // namespace TrylockSuccessEnumMultipleFailureModesFalseNegative
-
-
-// This test demonstrates that the analysis does not detect when all enumerators
-// are positive, and thus incapable of representing a failure.
-namespac...
[truncated]
|
You can test this locally with the following command:git-clang-format --diff 0d88f662ff4db7e78a6c48db79ef62c5228d5f2a 783f56be1bacdce768ebc11e8566171a9693becf -- clang/include/clang/Sema/ParsedAttr.h clang/lib/Analysis/ThreadSafety.cpp clang/lib/Sema/SemaDeclAttr.cpp clang/test/Sema/attr-capabilities.c clang/test/SemaCXX/warn-thread-safety-analysis.cpp clang/test/SemaCXX/warn-thread-safety-parsing.cpp clang/unittests/AST/ASTImporterTest.cppView the diff from clang-format here.diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index e25b843c9b..b08fb797e5 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1034,9 +1034,8 @@ public:
template <class AttrType>
void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, const Expr *Exp,
- const NamedDecl *D,
- const CFGBlock *PredBlock, const CFGBlock *CurrBlock,
- Expr *BrE, bool Neg);
+ const NamedDecl *D, const CFGBlock *PredBlock,
+ const CFGBlock *CurrBlock, Expr *BrE, bool Neg);
const CallExpr* getTrylockCallExpr(const Stmt *Cond, LocalVarContext C,
bool &Negate);
@@ -1358,8 +1357,8 @@ template <class AttrType>
void ThreadSafetyAnalyzer::getMutexIDs(CapExprSet &Mtxs, AttrType *Attr,
const Expr *Exp, const NamedDecl *D,
const CFGBlock *PredBlock,
- const CFGBlock *CurrBlock,
- Expr *BrE, bool Neg) {
+ const CFGBlock *CurrBlock, Expr *BrE,
+ bool Neg) {
// Find out which branch has the lock
bool branch = false;
if (const auto *BLE = dyn_cast_or_null<CXXBoolLiteralExpr>(BrE))
@@ -1497,27 +1496,27 @@ void ThreadSafetyAnalyzer::getEdgeLockset(FactSet& Result,
// If the condition is a call to a Trylock function, then grab the attributes
for (const auto *Attr : FunDecl->attrs()) {
switch (Attr->getKind()) {
- case attr::TryAcquireCapability: {
- auto *A = cast<TryAcquireCapabilityAttr>(Attr);
- getMutexIDs(A->isShared() ? SharedLocksToAdd : ExclusiveLocksToAdd, A,
- Exp, FunDecl, PredBlock, CurrBlock, A->getSuccessValue(),
- Negate);
- break;
- };
- case attr::ExclusiveTrylockFunction: {
- const auto *A = cast<ExclusiveTrylockFunctionAttr>(Attr);
- getMutexIDs(ExclusiveLocksToAdd, A, Exp, FunDecl, PredBlock, CurrBlock,
- A->getSuccessValue(), Negate);
- break;
- }
- case attr::SharedTrylockFunction: {
- const auto *A = cast<SharedTrylockFunctionAttr>(Attr);
- getMutexIDs(SharedLocksToAdd, A, Exp, FunDecl, PredBlock, CurrBlock,
- A->getSuccessValue(), Negate);
- break;
- }
- default:
- break;
+ case attr::TryAcquireCapability: {
+ auto *A = cast<TryAcquireCapabilityAttr>(Attr);
+ getMutexIDs(A->isShared() ? SharedLocksToAdd : ExclusiveLocksToAdd, A,
+ Exp, FunDecl, PredBlock, CurrBlock, A->getSuccessValue(),
+ Negate);
+ break;
+ };
+ case attr::ExclusiveTrylockFunction: {
+ const auto *A = cast<ExclusiveTrylockFunctionAttr>(Attr);
+ getMutexIDs(ExclusiveLocksToAdd, A, Exp, FunDecl, PredBlock, CurrBlock,
+ A->getSuccessValue(), Negate);
+ break;
+ }
+ case attr::SharedTrylockFunction: {
+ const auto *A = cast<SharedTrylockFunctionAttr>(Attr);
+ getMutexIDs(SharedLocksToAdd, A, Exp, FunDecl, PredBlock, CurrBlock,
+ A->getSuccessValue(), Negate);
+ break;
+ }
+ default:
+ break;
}
}
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index b8842e9003..b269cf82a9 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -172,7 +172,6 @@ static bool isIntOrBool(Expr *Exp) {
return QT->isBooleanType() || QT->isIntegerType();
}
-
// Check to see if the type is a smart pointer of some kind. We assume
// it's a smart pointer if it defines both operator-> and operator*.
static bool threadSafetyCheckIsSmartPointer(Sema &S, const RecordType* RT) {
|
AaronBallman
approved these changes
Jul 1, 2024
6 tasks
lravenclaw
pushed a commit
to lravenclaw/llvm-project
that referenced
this pull request
Jul 3, 2024
…lvm#97293) This PR reverts llvm#95290 and the one-liner followup PR llvm#96494. I received some substantial feedback on llvm#95290, which I plan to address in a future PR. I've also received feedback that because the change emits errors where they were not emitted before, we should at least have a flag to disable the stricter warnings.
kbluck
pushed a commit
to kbluck/llvm-project
that referenced
this pull request
Jul 6, 2024
…lvm#97293) This PR reverts llvm#95290 and the one-liner followup PR llvm#96494. I received some substantial feedback on llvm#95290, which I plan to address in a future PR. I've also received feedback that because the change emits errors where they were not emitted before, we should at least have a flag to disable the stricter warnings.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR reverts #95290 and the one-liner followup PR #96494.
I received some substantial feedback on #95290, which I plan to address in a future PR.
I've also received feedback that because the change emits errors where they were not emitted before, we should at least have a flag to disable the stricter warnings.