Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[msan] Generalize handleIntrinsicByApplyingToShadow by adding bitcasting #123474

Merged
merged 2 commits into from
Jan 23, 2025

Conversation

thurstond
Copy link
Contributor

@thurstond thurstond commented Jan 18, 2025

handleIntrinsicByApplyingToShadow (introduced in
#114490) requires that the intrinsic supports integer-ish operands; this is not the case for all intrinsics. This patch generalizes the function to bitcast the shadow arguments to be the same type as the original intrinsic, thus guaranteeing that the intrinsic exists. Additionally, it casts the computed shadow to be an appropriate shadow type.

This function assumes that the intrinsic will handle arbitrary bit-patterns (for example, if the intrinsic accepts floats for var1, we assume that it works normally even if inputs are NaNs etc.).

handleIntrinsicByApplyingToShadow (introduced in
llvm#114490) requires that the
intrinsic supports integer-ish operands; this is not the case for all
intrinsics. This patch generalizes the function to bitcast the shadow
arguments to be the same type as the original intrinsic, thus
guaranteeing that the intrinsic exists. Additionally, it casts the
computed shadow to be an appropriate shadow type.

This function assumes that the intrinsic will handle arbitrary bit-patterns (for example, if the intrinsic only accepts floats
for var1, we require that it doesn't care if inputs are NaNs).
@llvmbot
Copy link
Member

llvmbot commented Jan 18, 2025

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Thurston Dang (thurstond)

Changes

handleIntrinsicByApplyingToShadow (introduced in
#114490) requires that the intrinsic supports integer-ish operands; this is not the case for all intrinsics. This patch generalizes the function to bitcast the shadow arguments to be the same type as the original intrinsic, thus guaranteeing that the intrinsic exists. Additionally, it casts the computed shadow to be an appropriate shadow type.

This function assumes that the intrinsic will handle arbitrary bit-patterns (for example, if the intrinsic only accepts floats for var1, we require that it doesn't care if inputs are NaNs).


Full diff: https://github.com/llvm/llvm-project/pull/123474.diff

1 Files Affected:

  • (modified) llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp (+10-2)
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 6daee7a3b6e815..7b5a3fa4066a70 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -4008,6 +4008,10 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
   ///     shadow[out] =
   ///         intrinsic(shadow[var1], shadow[var2], opType) | shadow[opType]
   ///
+  /// CAUTION: this assumes that the intrinsic will handle arbitrary
+  ///          bit-patterns (for example, if the intrinsic only accepts floats
+  ///          for var1, we require that it doesn't care if inputs are NaNs).
+  ///
   /// For example, this can be applied to the Arm NEON vector table intrinsics
   /// (tbl{1,2,3,4}).
   ///
@@ -4022,7 +4026,11 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
     // Don't use getNumOperands() because it includes the callee
     for (unsigned int i = 0; i < I.arg_size() - trailingVerbatimArgs; i++) {
       Value *Shadow = getShadow(&I, i);
-      ShadowArgs.push_back(Shadow);
+
+      // Shadows are integer-ish types but some intrinsics require a
+      // different (e.g., floating-point) type.
+      ShadowArgs.push_back(
+          IRB.CreateBitCast(Shadow, I.getArgOperand(i)->getType()));
     }
 
     for (unsigned int i = I.arg_size() - trailingVerbatimArgs; i < I.arg_size();
@@ -4043,7 +4051,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
       CombinedShadow = IRB.CreateOr(Shadow, CombinedShadow, "_msprop");
     }
 
-    setShadow(&I, CombinedShadow);
+    setShadow(&I, IRB.CreateBitCast(CombinedShadow, getShadowTy(&I)));
 
     setOriginForNaryOp(I);
   }

@thurstond thurstond merged commit 9cefa3e into llvm:main Jan 23, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants