[llvm] [msan] Add debugging for handleUnknownIntrinsic (PR #123381)

[Thurston Dang via llvm-commits]

https://github.com/thurstond updated https://github.com/llvm/llvm-project/pull/123381

>From f3a6f2f2e8306ce97d836a88e8981eb1c8b46c7c Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Fri, 17 Jan 2025 04:51:07 +0000
Subject: [PATCH 1/2] [msan] Add debugging for handleUnknownIntrinsic

This adds an experimental flag, msan-dump-strict-intrinsics (modeled
after msan-dump-strict-instructions), which prints out any intrinsics
that are heuristically handled. Additionally, MSan will print out heuristically
handled intrinsics when -debug is passed as a flag in debug builds.

MSan's intrinsic handling can be broken down into:
1) special cases (usually highly accurate)
2) heuristic handling (sometimes erroneous)
3) not handled

This patch's -msan-dump-strict-intrinsics can help debug Case 2.
Case 3) (which includes all the heuristics that are not handled by
special cases nor heuristics) can be debugged using the existing -msan-dump-strict-instructions.
---
 .../Instrumentation/MemorySanitizer.cpp       | 36 ++++++++++++-------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 429e323b6b7c24..1564f4c9ebd428 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -318,6 +318,13 @@ static cl::opt<bool> ClDumpStrictInstructions(
     cl::desc("print out instructions with default strict semantics"),
     cl::Hidden, cl::init(false));
 
+static cl::opt<bool> ClDumpStrictIntrinsics(
+    "msan-dump-strict-intrinsics",
+    cl::desc("Prints 'unknown' intrinsics that were handled heuristically. "
+             "Use -msan-dump-strict-instructions to print intrinsics that "
+             "could not be handled exactly nor heuristically."),
+    cl::Hidden, cl::init(false));
+
 static cl::opt<int> ClInstrumentationWithCallThreshold(
     "msan-instrumentation-with-call-threshold",
     cl::desc(
@@ -3016,28 +3023,29 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
   /// handling as an example of that.
   bool handleUnknownIntrinsic(IntrinsicInst &I) {
     unsigned NumArgOperands = I.arg_size();
-    if (NumArgOperands == 0)
-      return false;
 
-    if (NumArgOperands == 2 && I.getArgOperand(0)->getType()->isPointerTy() &&
+    bool success = false;
+    if (NumArgOperands == 0) {
+      // No-op
+    } else if (NumArgOperands == 2 && I.getArgOperand(0)->getType()->isPointerTy() &&
         I.getArgOperand(1)->getType()->isVectorTy() &&
         I.getType()->isVoidTy() && !I.onlyReadsMemory()) {
       // This looks like a vector store.
-      return handleVectorStoreIntrinsic(I);
-    }
-
-    if (NumArgOperands == 1 && I.getArgOperand(0)->getType()->isPointerTy() &&
+      success = handleVectorStoreIntrinsic(I);
+    } else if (NumArgOperands == 1 && I.getArgOperand(0)->getType()->isPointerTy() &&
         I.getType()->isVectorTy() && I.onlyReadsMemory()) {
       // This looks like a vector load.
-      return handleVectorLoadIntrinsic(I);
-    }
+      success = handleVectorLoadIntrinsic(I);
+    } else if (I.doesNotAccessMemory())
+      success = maybeHandleSimpleNomemIntrinsic(I);
+
+    if (success && ClDumpStrictIntrinsics)
+      dumpInst(I);
 
-    if (I.doesNotAccessMemory())
-      if (maybeHandleSimpleNomemIntrinsic(I))
-        return true;
+    LLVM_DEBUG(dbgs() << "UNKNOWN INTRINSIC HANDLED HEURISTICALLY: " << I << "\n");
 
     // FIXME: detect and handle SSE maskstore/maskload
-    return false;
+    return success;
   }
 
   void handleInvariantGroup(IntrinsicInst &I) {
@@ -4033,6 +4041,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
 
   void visitIntrinsicInst(IntrinsicInst &I) {
     switch (I.getIntrinsicID()) {
+#if 0
     case Intrinsic::uadd_with_overflow:
     case Intrinsic::sadd_with_overflow:
     case Intrinsic::usub_with_overflow:
@@ -4445,6 +4454,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
       handleNEONVectorMultiplyIntrinsic(I);
       break;
     }
+#endif
 
     default:
       if (!handleUnknownIntrinsic(I))

>From 44c620cbf6a781b47ae32bca43999bde60564890 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Fri, 17 Jan 2025 18:05:48 +0000
Subject: [PATCH 2/2] Remove testing change + format

---
 .../Instrumentation/MemorySanitizer.cpp         | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 1564f4c9ebd428..ddbe8db408b615 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -3027,13 +3027,15 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
     bool success = false;
     if (NumArgOperands == 0) {
       // No-op
-    } else if (NumArgOperands == 2 && I.getArgOperand(0)->getType()->isPointerTy() &&
-        I.getArgOperand(1)->getType()->isVectorTy() &&
-        I.getType()->isVoidTy() && !I.onlyReadsMemory()) {
+    } else if (NumArgOperands == 2 &&
+               I.getArgOperand(0)->getType()->isPointerTy() &&
+               I.getArgOperand(1)->getType()->isVectorTy() &&
+               I.getType()->isVoidTy() && !I.onlyReadsMemory()) {
       // This looks like a vector store.
       success = handleVectorStoreIntrinsic(I);
-    } else if (NumArgOperands == 1 && I.getArgOperand(0)->getType()->isPointerTy() &&
-        I.getType()->isVectorTy() && I.onlyReadsMemory()) {
+    } else if (NumArgOperands == 1 &&
+               I.getArgOperand(0)->getType()->isPointerTy() &&
+               I.getType()->isVectorTy() && I.onlyReadsMemory()) {
       // This looks like a vector load.
       success = handleVectorLoadIntrinsic(I);
     } else if (I.doesNotAccessMemory())
@@ -3042,7 +3044,8 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
     if (success && ClDumpStrictIntrinsics)
       dumpInst(I);
 
-    LLVM_DEBUG(dbgs() << "UNKNOWN INTRINSIC HANDLED HEURISTICALLY: " << I << "\n");
+    LLVM_DEBUG(dbgs() << "UNKNOWN INTRINSIC HANDLED HEURISTICALLY: " << I
+                      << "\n");
 
     // FIXME: detect and handle SSE maskstore/maskload
     return success;
@@ -4041,7 +4044,6 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
 
   void visitIntrinsicInst(IntrinsicInst &I) {
     switch (I.getIntrinsicID()) {
-#if 0
     case Intrinsic::uadd_with_overflow:
     case Intrinsic::sadd_with_overflow:
     case Intrinsic::usub_with_overflow:
@@ -4454,7 +4456,6 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
       handleNEONVectorMultiplyIntrinsic(I);
       break;
     }
-#endif
 
     default:
       if (!handleUnknownIntrinsic(I))