forked from OSchip/llvm-project
Revert "Allow relockable scopes with thread safety attributes."
This reverts commit r339456.
The change introduces a new crash, see
class SCOPED_LOCKABLE FileLock {
public:
explicit FileLock()
EXCLUSIVE_LOCK_FUNCTION(file_);
~FileLock() UNLOCK_FUNCTION(file_);
void Lock() EXCLUSIVE_LOCK_FUNCTION(file_);
Mutex file_;
};
void relockShared2() {
FileLock file_lock;
file_lock.Lock();
}
llvm-svn: 339558
This commit is contained in:
Haojian Wu
parent
7b31fae983
commit
74e0f40d98
|
|
@ -86,8 +86,8 @@ static void warnInvalidLock(ThreadSafetyHandler &Handler,
|
|||
|
||||
namespace {
|
||||
|
||||
/// A set of CapabilityExpr objects, which are compiled from thread safety
|
||||
/// attributes on a function.
|
||||
/// A set of CapabilityInfo objects, which are compiled from the
|
||||
/// requires attributes on a function.
|
||||
class CapExprSet : public SmallVector<CapabilityExpr, 4> {
|
||||
public:
|
||||
/// Push M onto list, but discard duplicates.
|
||||
|
|
@ -944,23 +944,6 @@ public:
|
|||
if (FullyRemove)
|
||||
FSet.removeLock(FactMan, Cp);
|
||||
}
|
||||
|
||||
void Relock(FactSet &FSet, FactManager &FactMan, LockKind LK,
|
||||
SourceLocation RelockLoc, ThreadSafetyHandler &Handler,
|
||||
StringRef DiagKind) const {
|
||||
for (const auto *UnderlyingMutex : UnderlyingMutexes) {
|
||||
CapabilityExpr UnderCp(UnderlyingMutex, false);
|
||||
|
||||
// We're relocking the underlying mutexes. Warn on double locking.
|
||||
if (FSet.findLock(FactMan, UnderCp))
|
||||
Handler.handleDoubleLock(DiagKind, UnderCp.toString(), RelockLoc);
|
||||
else {
|
||||
FSet.removeLock(FactMan, !UnderCp);
|
||||
FSet.addLock(FactMan, llvm::make_unique<LockableFactEntry>(UnderCp, LK,
|
||||
RelockLoc));
|
||||
}
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
/// Class which implements the core thread safety analysis routines.
|
||||
|
|
@ -991,9 +974,6 @@ public:
|
|||
void removeLock(FactSet &FSet, const CapabilityExpr &CapE,
|
||||
SourceLocation UnlockLoc, bool FullyRemove, LockKind Kind,
|
||||
StringRef DiagKind);
|
||||
void relockScopedLock(FactSet &FSet, const CapabilityExpr &CapE,
|
||||
SourceLocation RelockLoc, LockKind Kind,
|
||||
StringRef DiagKind);
|
||||
|
||||
template <typename AttrType>
|
||||
void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, Expr *Exp,
|
||||
|
|
@ -1305,27 +1285,6 @@ void ThreadSafetyAnalyzer::removeLock(FactSet &FSet, const CapabilityExpr &Cp,
|
|||
DiagKind);
|
||||
}
|
||||
|
||||
void ThreadSafetyAnalyzer::relockScopedLock(FactSet &FSet,
|
||||
const CapabilityExpr &Cp,
|
||||
SourceLocation RelockLoc,
|
||||
LockKind Kind, StringRef DiagKind) {
|
||||
if (Cp.shouldIgnore())
|
||||
return;
|
||||
|
||||
const FactEntry *LDat = FSet.findLock(FactMan, Cp);
|
||||
if (!LDat) {
|
||||
// FIXME: It's possible to manually destruct the scope and then relock it.
|
||||
// Should that be a separate warning? For now we pretend nothing happened.
|
||||
// It's undefined behavior, so not related to thread safety.
|
||||
return;
|
||||
}
|
||||
|
||||
// We should only land here if Cp is a scoped capability, so if we have any
|
||||
// fact, it must be a ScopedLockableFactEntry.
|
||||
const auto *SLDat = static_cast<const ScopedLockableFactEntry *>(LDat);
|
||||
SLDat->Relock(FSet, FactMan, Kind, RelockLoc, Handler, DiagKind);
|
||||
}
|
||||
|
||||
/// Extract the list of mutexIDs from the attribute on an expression,
|
||||
/// and push them onto Mtxs, discarding any duplicates.
|
||||
template <typename AttrType>
|
||||
|
|
@ -1767,19 +1726,14 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) {
|
|||
StringRef CapDiagKind = "mutex";
|
||||
|
||||
// Figure out if we're constructing an object of scoped lockable class
|
||||
bool isScopedConstructor = false, isScopedMemberCall = false;
|
||||
bool isScopedVar = false;
|
||||
if (VD) {
|
||||
if (const auto *CD = dyn_cast<const CXXConstructorDecl>(D)) {
|
||||
const CXXRecordDecl* PD = CD->getParent();
|
||||
if (PD && PD->hasAttr<ScopedLockableAttr>())
|
||||
isScopedConstructor = true;
|
||||
isScopedVar = true;
|
||||
}
|
||||
}
|
||||
if (const auto *MCD = dyn_cast<const CXXMemberCallExpr>(Exp)) {
|
||||
const CXXRecordDecl *PD = MCD->getRecordDecl();
|
||||
if (PD && PD->hasAttr<ScopedLockableAttr>())
|
||||
isScopedMemberCall = true;
|
||||
}
|
||||
|
||||
for(const Attr *At : D->attrs()) {
|
||||
switch (At->getKind()) {
|
||||
|
|
@ -1859,7 +1813,7 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) {
|
|||
POK_FunctionCall, ClassifyDiagnostic(A),
|
||||
Exp->getExprLoc());
|
||||
// use for adopting a lock
|
||||
if (isScopedConstructor) {
|
||||
if (isScopedVar) {
|
||||
Analyzer->getMutexIDs(A->isShared() ? ScopedSharedReqs
|
||||
: ScopedExclusiveReqs,
|
||||
A, Exp, D, VD);
|
||||
|
|
@ -1892,24 +1846,16 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) {
|
|||
Analyzer->removeLock(FSet, M, Loc, Dtor, LK_Generic, CapDiagKind);
|
||||
|
||||
// Add locks.
|
||||
if (isScopedMemberCall) {
|
||||
// If the call is on a scoped capability, we need to relock instead.
|
||||
for (const auto &M : ExclusiveLocksToAdd)
|
||||
Analyzer->relockScopedLock(FSet, M, Loc, LK_Exclusive, CapDiagKind);
|
||||
for (const auto &M : SharedLocksToAdd)
|
||||
Analyzer->relockScopedLock(FSet, M, Loc, LK_Shared, CapDiagKind);
|
||||
} else {
|
||||
for (const auto &M : ExclusiveLocksToAdd)
|
||||
Analyzer->addLock(FSet, llvm::make_unique<LockableFactEntry>(
|
||||
M, LK_Exclusive, Loc, isScopedConstructor),
|
||||
CapDiagKind);
|
||||
for (const auto &M : SharedLocksToAdd)
|
||||
Analyzer->addLock(FSet, llvm::make_unique<LockableFactEntry>(
|
||||
M, LK_Shared, Loc, isScopedConstructor),
|
||||
CapDiagKind);
|
||||
}
|
||||
for (const auto &M : ExclusiveLocksToAdd)
|
||||
Analyzer->addLock(FSet, llvm::make_unique<LockableFactEntry>(
|
||||
M, LK_Exclusive, Loc, isScopedVar),
|
||||
CapDiagKind);
|
||||
for (const auto &M : SharedLocksToAdd)
|
||||
Analyzer->addLock(FSet, llvm::make_unique<LockableFactEntry>(
|
||||
M, LK_Shared, Loc, isScopedVar),
|
||||
CapDiagKind);
|
||||
|
||||
if (isScopedConstructor) {
|
||||
if (isScopedVar) {
|
||||
// Add the managing object as a dummy mutex, mapped to the underlying mutex.
|
||||
SourceLocation MLoc = VD->getLocation();
|
||||
DeclRefExpr DRE(VD, false, VD->getType(), VK_LValue, VD->getLocation());
|
||||
|
|
|
|||
|
|
@ -2621,154 +2621,6 @@ void Foo::test5() {
|
|||
} // end namespace ReleasableScopedLock
|
||||
|
||||
|
||||
namespace RelockableScopedLock {
|
||||
|
||||
class SCOPED_LOCKABLE RelockableExclusiveMutexLock {
|
||||
public:
|
||||
RelockableExclusiveMutexLock(Mutex *mu) EXCLUSIVE_LOCK_FUNCTION(mu);
|
||||
~RelockableExclusiveMutexLock() EXCLUSIVE_UNLOCK_FUNCTION();
|
||||
|
||||
void Lock() EXCLUSIVE_LOCK_FUNCTION();
|
||||
void Unlock() UNLOCK_FUNCTION();
|
||||
};
|
||||
|
||||
class SCOPED_LOCKABLE RelockableSharedMutexLock {
|
||||
public:
|
||||
RelockableSharedMutexLock(Mutex *mu) SHARED_LOCK_FUNCTION(mu);
|
||||
~RelockableSharedMutexLock() UNLOCK_FUNCTION();
|
||||
|
||||
void Lock() SHARED_LOCK_FUNCTION();
|
||||
void Unlock() UNLOCK_FUNCTION();
|
||||
};
|
||||
|
||||
class SharedTraits {};
|
||||
class ExclusiveTraits {};
|
||||
|
||||
class SCOPED_LOCKABLE RelockableMutexLock {
|
||||
public:
|
||||
RelockableMutexLock(Mutex *mu, SharedTraits) SHARED_LOCK_FUNCTION(mu);
|
||||
RelockableMutexLock(Mutex *mu, ExclusiveTraits) EXCLUSIVE_LOCK_FUNCTION(mu);
|
||||
~RelockableMutexLock() UNLOCK_FUNCTION();
|
||||
|
||||
void Lock() EXCLUSIVE_LOCK_FUNCTION();
|
||||
void Unlock() UNLOCK_FUNCTION();
|
||||
|
||||
void ReaderLock() SHARED_LOCK_FUNCTION();
|
||||
void ReaderUnlock() UNLOCK_FUNCTION();
|
||||
|
||||
void PromoteShared() UNLOCK_FUNCTION() EXCLUSIVE_LOCK_FUNCTION();
|
||||
void DemoteExclusive() UNLOCK_FUNCTION() SHARED_LOCK_FUNCTION();
|
||||
};
|
||||
|
||||
Mutex mu;
|
||||
int x GUARDED_BY(mu);
|
||||
|
||||
void print(int);
|
||||
|
||||
void write() {
|
||||
RelockableExclusiveMutexLock scope(&mu);
|
||||
x = 2;
|
||||
scope.Unlock();
|
||||
|
||||
x = 3; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
|
||||
|
||||
scope.Lock();
|
||||
x = 4;
|
||||
}
|
||||
|
||||
void read() {
|
||||
RelockableSharedMutexLock scope(&mu);
|
||||
print(x);
|
||||
scope.Unlock();
|
||||
|
||||
print(x); // expected-warning {{reading variable 'x' requires holding mutex 'mu'}}
|
||||
x = 3; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
|
||||
|
||||
scope.Lock();
|
||||
print(x);
|
||||
x = 4; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
|
||||
}
|
||||
|
||||
void relockExclusive() {
|
||||
RelockableMutexLock scope(&mu, SharedTraits{});
|
||||
print(x);
|
||||
x = 2; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
|
||||
scope.ReaderUnlock();
|
||||
|
||||
print(x); // expected-warning {{reading variable 'x' requires holding mutex 'mu'}}
|
||||
|
||||
scope.Lock();
|
||||
print(x);
|
||||
x = 4;
|
||||
|
||||
scope.DemoteExclusive();
|
||||
print(x);
|
||||
x = 5; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
|
||||
}
|
||||
|
||||
void relockShared() {
|
||||
RelockableMutexLock scope(&mu, ExclusiveTraits{});
|
||||
print(x);
|
||||
x = 2;
|
||||
scope.Unlock();
|
||||
|
||||
print(x); // expected-warning {{reading variable 'x' requires holding mutex 'mu'}}
|
||||
|
||||
scope.ReaderLock();
|
||||
print(x);
|
||||
x = 4; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}}
|
||||
|
||||
scope.PromoteShared();
|
||||
print(x);
|
||||
x = 5;
|
||||
}
|
||||
|
||||
void doubleUnlock1() {
|
||||
RelockableExclusiveMutexLock scope(&mu);
|
||||
scope.Unlock();
|
||||
scope.Unlock(); // expected-warning {{releasing mutex 'mu' that was not held}}
|
||||
}
|
||||
|
||||
void doubleUnlock2() {
|
||||
RelockableSharedMutexLock scope(&mu);
|
||||
scope.Unlock();
|
||||
scope.Unlock(); // expected-warning {{releasing mutex 'mu' that was not held}}
|
||||
}
|
||||
|
||||
void doubleLock1() {
|
||||
RelockableExclusiveMutexLock scope(&mu);
|
||||
scope.Lock(); // expected-warning {{acquiring mutex 'mu' that is already held}}
|
||||
}
|
||||
|
||||
void doubleLock2() {
|
||||
RelockableSharedMutexLock scope(&mu);
|
||||
scope.Lock(); // expected-warning {{acquiring mutex 'mu' that is already held}}
|
||||
}
|
||||
|
||||
void doubleLock3() {
|
||||
RelockableExclusiveMutexLock scope(&mu);
|
||||
scope.Unlock();
|
||||
scope.Lock();
|
||||
scope.Lock(); // expected-warning {{acquiring mutex 'mu' that is already held}}
|
||||
}
|
||||
|
||||
void doubleLock4() {
|
||||
RelockableSharedMutexLock scope(&mu);
|
||||
scope.Unlock();
|
||||
scope.Lock();
|
||||
scope.Lock(); // expected-warning {{acquiring mutex 'mu' that is already held}}
|
||||
}
|
||||
|
||||
// Doesn't make a lot of sense, just making sure there is no crash.
|
||||
void destructLock() {
|
||||
RelockableExclusiveMutexLock scope(&mu);
|
||||
scope.~RelockableExclusiveMutexLock();
|
||||
scope.Lock(); // Maybe this should warn.
|
||||
} // expected-warning {{releasing mutex 'scope' that was not held}}
|
||||
|
||||
} // end namespace RelockableScopedLock
|
||||
|
||||
|
||||
namespace TrylockFunctionTest {
|
||||
|
||||
class Foo {
|
||||
|
|
|
|||
Loading…
Reference in New Issue