Skip to content

Commit d65c922

Browse files
committed
Thread safety analysis: Store CapabilityExprs in ScopedLockableFactEntry (NFC)
For now this doesn't make a whole lot of sense, but it will allow us to store the capability kind in a CapabilityExpr and make sure it doesn't get lost. The capabilities managed by a scoped lockable can of course be of different kind, so we'll need to store that per entry. Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D124128
1 parent dd1790c commit d65c922

File tree

1 file changed

+23
-25
lines changed

1 file changed

+23
-25
lines changed

clang/lib/Analysis/ThreadSafety.cpp

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
#include "llvm/ADT/DenseMap.h"
4242
#include "llvm/ADT/ImmutableMap.h"
4343
#include "llvm/ADT/Optional.h"
44-
#include "llvm/ADT/PointerIntPair.h"
4544
#include "llvm/ADT/STLExtras.h"
4645
#include "llvm/ADT/SmallVector.h"
4746
#include "llvm/ADT/StringRef.h"
@@ -897,41 +896,41 @@ class ScopedLockableFactEntry : public FactEntry {
897896
UCK_ReleasedExclusive, ///< Exclusive capability that was released.
898897
};
899898

900-
using UnderlyingCapability =
901-
llvm::PointerIntPair<const til::SExpr *, 2, UnderlyingCapabilityKind>;
899+
struct UnderlyingCapability {
900+
CapabilityExpr Cap;
901+
UnderlyingCapabilityKind Kind;
902+
};
902903

903-
SmallVector<UnderlyingCapability, 4> UnderlyingMutexes;
904+
SmallVector<UnderlyingCapability, 2> UnderlyingMutexes;
904905

905906
public:
906907
ScopedLockableFactEntry(const CapabilityExpr &CE, SourceLocation Loc)
907908
: FactEntry(CE, LK_Exclusive, Loc, Acquired) {}
908909

909910
void addLock(const CapabilityExpr &M) {
910-
UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
911+
UnderlyingMutexes.push_back(UnderlyingCapability{M, UCK_Acquired});
911912
}
912913

913914
void addExclusiveUnlock(const CapabilityExpr &M) {
914-
UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedExclusive);
915+
UnderlyingMutexes.push_back(UnderlyingCapability{M, UCK_ReleasedExclusive});
915916
}
916917

917918
void addSharedUnlock(const CapabilityExpr &M) {
918-
UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedShared);
919+
UnderlyingMutexes.push_back(UnderlyingCapability{M, UCK_ReleasedShared});
919920
}
920921

921922
void
922923
handleRemovalFromIntersection(const FactSet &FSet, FactManager &FactMan,
923924
SourceLocation JoinLoc, LockErrorKind LEK,
924925
ThreadSafetyHandler &Handler) const override {
925926
for (const auto &UnderlyingMutex : UnderlyingMutexes) {
926-
const auto *Entry = FSet.findLock(
927-
FactMan, CapabilityExpr(UnderlyingMutex.getPointer(), false));
928-
if ((UnderlyingMutex.getInt() == UCK_Acquired && Entry) ||
929-
(UnderlyingMutex.getInt() != UCK_Acquired && !Entry)) {
927+
const auto *Entry = FSet.findLock(FactMan, UnderlyingMutex.Cap);
928+
if ((UnderlyingMutex.Kind == UCK_Acquired && Entry) ||
929+
(UnderlyingMutex.Kind != UCK_Acquired && !Entry)) {
930930
// If this scoped lock manages another mutex, and if the underlying
931931
// mutex is still/not held, then warn about the underlying mutex.
932932
Handler.handleMutexHeldEndOfScope(
933-
"mutex", sx::toString(UnderlyingMutex.getPointer()), loc(), JoinLoc,
934-
LEK);
933+
"mutex", UnderlyingMutex.Cap.toString(), loc(), JoinLoc, LEK);
935934
}
936935
}
937936
}
@@ -940,13 +939,12 @@ class ScopedLockableFactEntry : public FactEntry {
940939
ThreadSafetyHandler &Handler,
941940
StringRef DiagKind) const override {
942941
for (const auto &UnderlyingMutex : UnderlyingMutexes) {
943-
CapabilityExpr UnderCp(UnderlyingMutex.getPointer(), false);
944-
945-
if (UnderlyingMutex.getInt() == UCK_Acquired)
946-
lock(FSet, FactMan, UnderCp, entry.kind(), entry.loc(), &Handler,
947-
DiagKind);
942+
if (UnderlyingMutex.Kind == UCK_Acquired)
943+
lock(FSet, FactMan, UnderlyingMutex.Cap, entry.kind(), entry.loc(),
944+
&Handler, DiagKind);
948945
else
949-
unlock(FSet, FactMan, UnderCp, entry.loc(), &Handler, DiagKind);
946+
unlock(FSet, FactMan, UnderlyingMutex.Cap, entry.loc(), &Handler,
947+
DiagKind);
950948
}
951949
}
952950

@@ -956,18 +954,18 @@ class ScopedLockableFactEntry : public FactEntry {
956954
StringRef DiagKind) const override {
957955
assert(!Cp.negative() && "Managing object cannot be negative.");
958956
for (const auto &UnderlyingMutex : UnderlyingMutexes) {
959-
CapabilityExpr UnderCp(UnderlyingMutex.getPointer(), false);
960-
961957
// Remove/lock the underlying mutex if it exists/is still unlocked; warn
962958
// on double unlocking/locking if we're not destroying the scoped object.
963959
ThreadSafetyHandler *TSHandler = FullyRemove ? nullptr : &Handler;
964-
if (UnderlyingMutex.getInt() == UCK_Acquired) {
965-
unlock(FSet, FactMan, UnderCp, UnlockLoc, TSHandler, DiagKind);
960+
if (UnderlyingMutex.Kind == UCK_Acquired) {
961+
unlock(FSet, FactMan, UnderlyingMutex.Cap, UnlockLoc, TSHandler,
962+
DiagKind);
966963
} else {
967-
LockKind kind = UnderlyingMutex.getInt() == UCK_ReleasedShared
964+
LockKind kind = UnderlyingMutex.Kind == UCK_ReleasedShared
968965
? LK_Shared
969966
: LK_Exclusive;
970-
lock(FSet, FactMan, UnderCp, kind, UnlockLoc, TSHandler, DiagKind);
967+
lock(FSet, FactMan, UnderlyingMutex.Cap, kind, UnlockLoc, TSHandler,
968+
DiagKind);
971969
}
972970
}
973971
if (FullyRemove)

0 commit comments

Comments
 (0)