[PATCH] D101755: Thread safety analysis: Eliminate parameter from intersectAndWarn (NFC)

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert created this revision.
aaronpuchert added reviewers: aaron.ballman, delesley.
aaronpuchert requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We were modifying precisely when intersecting the lock sets of multiple
predecessors without back edge. That's no coincidence: we can't modify
on back edges, it doesn't make sense to modify at the end of a function,
and otherwise we always want to intersect on forward edges, because we
can build a new lock set for those.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101755

Files:
  clang/lib/Analysis/ThreadSafety.cpp


Index: clang/lib/Analysis/ThreadSafety.cpp
===================================================================
--- clang/lib/Analysis/ThreadSafety.cpp
+++ clang/lib/Analysis/ThreadSafety.cpp
@@ -1051,14 +1051,12 @@
                       const CFGBlock *CurrBlock);
 
   void intersectAndWarn(FactSet &FSet1, const FactSet &FSet2,
-                        SourceLocation JoinLoc,
-                        LockErrorKind LEK1, LockErrorKind LEK2,
-                        bool Modify=true);
+                        SourceLocation JoinLoc, LockErrorKind LEK1,
+                        LockErrorKind LEK2);
 
   void intersectAndWarn(FactSet &FSet1, const FactSet &FSet2,
-                        SourceLocation JoinLoc, LockErrorKind LEK1,
-                        bool Modify=true) {
-    intersectAndWarn(FSet1, FSet2, JoinLoc, LEK1, LEK1, Modify);
+                        SourceLocation JoinLoc, LockErrorKind LEK1) {
+    intersectAndWarn(FSet1, FSet2, JoinLoc, LEK1, LEK1);
   }
 
   void runAnalysis(AnalysisDeclContext &AC);
@@ -2206,8 +2204,7 @@
                                             const FactSet &FSet2,
                                             SourceLocation JoinLoc,
                                             LockErrorKind LEK1,
-                                            LockErrorKind LEK2,
-                                            bool Modify) {
+                                            LockErrorKind LEK2) {
   FactSet FSet1Orig = FSet1;
 
   // Find locks in FSet2 that conflict or are not in FSet1, and warn.
@@ -2220,11 +2217,13 @@
       if (LDat1.kind() != LDat2.kind()) {
         Handler.handleExclusiveAndShared("mutex", LDat2.toString(), LDat2.loc(),
                                          LDat1.loc());
-        if (Modify && LDat1.kind() != LK_Exclusive) {
+        if (LEK1 == LEK_LockedSomePredecessors &&
+            LDat1.kind() != LK_Exclusive) {
           // Take the exclusive lock, which is the one in FSet2.
           *Iter1 = Fact;
         }
-      } else if (Modify && LDat1.asserted() && !LDat2.asserted()) {
+      } else if (LEK1 == LEK_LockedSomePredecessors && LDat1.asserted() &&
+                 !LDat2.asserted()) {
         // The non-asserted lock in FSet2 is the one we want to track.
         *Iter1 = Fact;
       }
@@ -2242,7 +2241,7 @@
     if (!LDat2) {
       LDat1->handleRemovalFromIntersection(FSet1Orig, FactMan, JoinLoc, LEK2,
                                            Handler);
-      if (Modify)
+      if (LEK2 == LEK_LockedSomePredecessors)
         FSet1.removeLock(FactMan, *LDat1);
     }
   }
@@ -2469,8 +2468,7 @@
         // Do not update EntrySet.
         intersectAndWarn(
             CurrBlockInfo->EntrySet, PrevLockset, PrevBlockInfo->ExitLoc,
-            IsLoop ? LEK_LockedSomeLoopIterations : LEK_LockedSomePredecessors,
-            !IsLoop);
+            IsLoop ? LEK_LockedSomeLoopIterations : LEK_LockedSomePredecessors);
       }
     }
 
@@ -2518,10 +2516,8 @@
       CFGBlock *FirstLoopBlock = *SI;
       CFGBlockInfo *PreLoop = &BlockInfo[FirstLoopBlock->getBlockID()];
       CFGBlockInfo *LoopEnd = &BlockInfo[CurrBlockID];
-      intersectAndWarn(LoopEnd->ExitSet, PreLoop->EntrySet,
-                       PreLoop->EntryLoc,
-                       LEK_LockedSomeLoopIterations,
-                       false);
+      intersectAndWarn(LoopEnd->ExitSet, PreLoop->EntrySet, PreLoop->EntryLoc,
+                       LEK_LockedSomeLoopIterations);
     }
   }
 
@@ -2549,11 +2545,8 @@
     ExpectedExitSet.removeLock(FactMan, Lock);
 
   // FIXME: Should we call this function for all blocks which exit the function?
-  intersectAndWarn(ExpectedExitSet, Final->ExitSet,
-                   Final->ExitLoc,
-                   LEK_LockedAtEndOfFunction,
-                   LEK_NotLockedAtEndOfFunction,
-                   false);
+  intersectAndWarn(ExpectedExitSet, Final->ExitSet, Final->ExitLoc,
+                   LEK_LockedAtEndOfFunction, LEK_NotLockedAtEndOfFunction);
 
   Handler.leaveFunction(CurrentFunction);
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D101755.342365.patch
Type: text/x-patch
Size: 4051 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210503/88412a17/attachment.bin>