[llvm] 4f4ebee - [msan] Eliminate non-deterministic behavior in the pass (#89831)

[via llvm-commits]

Author: Vitaly Buka
Date: 2024-04-23T16:19:47-07:00
New Revision: 4f4ebee10ec91becb75ed36608ae26a2bd09e3bb

URL: https://github.com/llvm/llvm-project/commit/4f4ebee10ec91becb75ed36608ae26a2bd09e3bb
DIFF: https://github.com/llvm/llvm-project/commit/4f4ebee10ec91becb75ed36608ae26a2bd09e3bb.diff

LOG: [msan] Eliminate non-deterministic behavior in the pass (#89831)

Almost NFC, instrumentation is as correct as it was before.

We need InstrumentationList grouped by origin instruction,
so we used stable_sort. However these objects already grouped
because we never interleave sequences of `insertShadowCheck`
of different instrunction.

Pointer sort has artifact that it was deppendent on allocator behavior,
so we could inserted checks in a different order.

There is no test, as I failed to reproduce this with `opt`. My guess
is that for reproducer we need to increase fragmentation in the
allocator.

Added: 
    

Modified: 
    llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 824cbee4eca578..e5ef0333696d0f 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -152,6 +152,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DepthFirstIterator.h"
 #include "llvm/ADT/SetVector.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
@@ -1464,19 +1465,21 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
   }
 
   void materializeChecks() {
-    llvm::stable_sort(InstrumentationList,
-                      [](const ShadowOriginAndInsertPoint &L,
-                         const ShadowOriginAndInsertPoint &R) {
-                        return L.OrigIns < R.OrigIns;
-                      });
+#ifndef NDEBUG
+    // For assert below.
+    SmallPtrSet<Instruction *, 16> Done;
+#endif
 
     for (auto I = InstrumentationList.begin();
          I != InstrumentationList.end();) {
-      auto J =
-          std::find_if(I + 1, InstrumentationList.end(),
-                       [L = I->OrigIns](const ShadowOriginAndInsertPoint &R) {
-                         return L != R.OrigIns;
-                       });
+      auto OrigIns = I->OrigIns;
+      // Checks are grouped by the original instruction. We call all
+      // `insertShadowCheck` for an instruction at once.
+      assert(Done.insert(OrigIns).second);
+      auto J = std::find_if(I + 1, InstrumentationList.end(),
+                            [OrigIns](const ShadowOriginAndInsertPoint &R) {
+                              return OrigIns != R.OrigIns;
+                            });
       // Process all checks of instruction at once.
       materializeInstructionChecks(ArrayRef<ShadowOriginAndInsertPoint>(I, J));
       I = J;