[compiler-rt] [llvm] [msan] Add experimental '-mllvm -msan-embed-faulting-instruction' and MSAN_OPTIONS=print_faulting_instruction (PR #136539)
Thurston Dang via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 22 22:11:23 PDT 2025
https://github.com/thurstond updated https://github.com/llvm/llvm-project/pull/136539
>From cb98d98d8050b8fb170f31b6f659d3a023af9ac0 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 18 Mar 2025 16:20:10 +0000
Subject: [PATCH 01/26] [msan] Add experimental '-mllvm
-msan-print-faulting-instruction'
This adds an experimental flag, -mllvm -msan-print-faulting-instruction,
which will print the LLVM instruction that resulted in an MSan UUM
report.
Although MSan UUM reports will print out the line and column number
(assuming symbolization is available), inlining, macros and LLVM
"auto-upgraded" intrinsics can obscure the root cause.
This patch adds a test case,
compiler-rt/test/msan/print_faulting_inst.cpp, which illustrates that
-msan-print-faulting-instruction can provide more information than line
and column number.
---
compiler-rt/lib/msan/msan.cpp | 74 +++++++++
.../lib/msan/msan_interface_internal.h | 20 +++
compiler-rt/test/msan/print_faulting_inst.cpp | 83 ++++++++++
.../Instrumentation/MemorySanitizer.cpp | 145 ++++++++++++++----
4 files changed, 293 insertions(+), 29 deletions(-)
create mode 100644 compiler-rt/test/msan/print_faulting_inst.cpp
diff --git a/compiler-rt/lib/msan/msan.cpp b/compiler-rt/lib/msan/msan.cpp
index a3c0c2e485af3..a22bfe0d7ab32 100644
--- a/compiler-rt/lib/msan/msan.cpp
+++ b/compiler-rt/lib/msan/msan.cpp
@@ -352,6 +352,29 @@ void __sanitizer::BufferedStackTrace::UnwindImpl(
using namespace __msan;
+#define PRINT_FAULTING_INSTRUCTION(instname) \
+ Printf("Instruction that failed the shadow check: %s\n", instname); \
+ Printf("\n");
+
+#define MSAN_MAYBE_WARNING_INSTNAME(type, size, instname) \
+ void __msan_maybe_warning_instname_##size(type s, u32 o, char *instname) { \
+ GET_CALLER_PC_BP; \
+ if (UNLIKELY(s)) { \
+ if (instname) \
+ PRINT_FAULTING_INSTRUCTION(instname); \
+ PrintWarningWithOrigin(pc, bp, o); \
+ if (__msan::flags()->halt_on_error) { \
+ Printf("Exiting\n"); \
+ Die(); \
+ } \
+ } \
+ }
+
+MSAN_MAYBE_WARNING_INSTNAME(u8, 1, instname)
+MSAN_MAYBE_WARNING_INSTNAME(u16, 2, instname)
+MSAN_MAYBE_WARNING_INSTNAME(u32, 4, instname)
+MSAN_MAYBE_WARNING_INSTNAME(u64, 8, instname)
+
#define MSAN_MAYBE_WARNING(type, size) \
void __msan_maybe_warning_##size(type s, u32 o) { \
GET_CALLER_PC_BP; \
@@ -426,6 +449,57 @@ void __msan_warning_with_origin_noreturn(u32 origin) {
Die();
}
+// We duplicate the non _instname function's body because we don't want to
+// pollute the stack traces with an additional function call.
+//
+// We can't use a simple macro wrapper, because the instrumentation pass
+// expects function symbols.
+// We don't add instname as a parameter everywhere (with a check whether the
+// value is null) to avoid polluting the fastpath.
+void __msan_warning_instname(char *instname) {
+ PRINT_FAULTING_INSTRUCTION(instname);
+ GET_CALLER_PC_BP;
+ PrintWarningWithOrigin(pc, bp, 0);
+ if (__msan::flags()->halt_on_error) {
+ if (__msan::flags()->print_stats)
+ ReportStats();
+ Printf("Exiting\n");
+ Die();
+ }
+}
+
+void __msan_warning_noreturn_instname(char *instname) {
+ PRINT_FAULTING_INSTRUCTION(instname);
+ GET_CALLER_PC_BP;
+ PrintWarningWithOrigin(pc, bp, 0);
+ if (__msan::flags()->print_stats)
+ ReportStats();
+ Printf("Exiting\n");
+ Die();
+}
+
+void __msan_warning_with_origin_instname(u32 origin, char *instname) {
+ PRINT_FAULTING_INSTRUCTION(instname);
+ GET_CALLER_PC_BP;
+ PrintWarningWithOrigin(pc, bp, origin);
+ if (__msan::flags()->halt_on_error) {
+ if (__msan::flags()->print_stats)
+ ReportStats();
+ Printf("Exiting\n");
+ Die();
+ }
+}
+
+void __msan_warning_with_origin_noreturn_instname(u32 origin, char *instname) {
+ PRINT_FAULTING_INSTRUCTION(instname);
+ GET_CALLER_PC_BP;
+ PrintWarningWithOrigin(pc, bp, origin);
+ if (__msan::flags()->print_stats)
+ ReportStats();
+ Printf("Exiting\n");
+ Die();
+}
+
static void OnStackUnwind(const SignalContext &sig, const void *,
BufferedStackTrace *stack) {
stack->Unwind(StackTrace::GetNextInstructionPc(sig.pc), sig.bp, sig.context,
diff --git a/compiler-rt/lib/msan/msan_interface_internal.h b/compiler-rt/lib/msan/msan_interface_internal.h
index c2eead13c20cf..5607d08e66d86 100644
--- a/compiler-rt/lib/msan/msan_interface_internal.h
+++ b/compiler-rt/lib/msan/msan_interface_internal.h
@@ -30,12 +30,18 @@ void __msan_init();
SANITIZER_INTERFACE_ATTRIBUTE
void __msan_warning();
+SANITIZER_INTERFACE_ATTRIBUTE
+void __msan_warning_instname(char *instname);
+
// Print a warning and die.
// Instrumentation inserts calls to this function when building in "fast" mode
// (i.e. -mllvm -msan-keep-going)
SANITIZER_INTERFACE_ATTRIBUTE __attribute__((noreturn))
void __msan_warning_noreturn();
+SANITIZER_INTERFACE_ATTRIBUTE __attribute__((noreturn)) void
+__msan_warning_noreturn_instname(char *instname);
+
using __sanitizer::uptr;
using __sanitizer::sptr;
using __sanitizer::uu64;
@@ -49,8 +55,13 @@ using __sanitizer::u8;
// Versions of the above which take Origin as a parameter
SANITIZER_INTERFACE_ATTRIBUTE
void __msan_warning_with_origin(u32 origin);
+SANITIZER_INTERFACE_ATTRIBUTE
+void __msan_warning_with_origin_instname(u32 origin, char *instname);
+
SANITIZER_INTERFACE_ATTRIBUTE __attribute__((noreturn)) void
__msan_warning_with_origin_noreturn(u32 origin);
+SANITIZER_INTERFACE_ATTRIBUTE __attribute__((noreturn)) void
+__msan_warning_with_origin_noreturn_instname(u32 origin, char *instname);
SANITIZER_INTERFACE_ATTRIBUTE
void __msan_maybe_warning_1(u8 s, u32 o);
@@ -61,6 +72,15 @@ void __msan_maybe_warning_4(u32 s, u32 o);
SANITIZER_INTERFACE_ATTRIBUTE
void __msan_maybe_warning_8(u64 s, u32 o);
+SANITIZER_INTERFACE_ATTRIBUTE
+void __msan_maybe_warning_1_instname(u8 s, u32 o, char *instname);
+SANITIZER_INTERFACE_ATTRIBUTE
+void __msan_maybe_warning_2_instname(u16 s, u32 o, char *instname);
+SANITIZER_INTERFACE_ATTRIBUTE
+void __msan_maybe_warning_4_instname(u32 s, u32 o, char *instname);
+SANITIZER_INTERFACE_ATTRIBUTE
+void __msan_maybe_warning_8_instname(u64 s, u32 o, char *instname);
+
SANITIZER_INTERFACE_ATTRIBUTE
void __msan_maybe_store_origin_1(u8 s, void *p, u32 o);
SANITIZER_INTERFACE_ATTRIBUTE
diff --git a/compiler-rt/test/msan/print_faulting_inst.cpp b/compiler-rt/test/msan/print_faulting_inst.cpp
new file mode 100644
index 0000000000000..09c5f22ada132
--- /dev/null
+++ b/compiler-rt/test/msan/print_faulting_inst.cpp
@@ -0,0 +1,83 @@
+// Try parameter '0' (program runs cleanly)
+// -------------------------------------------------------
+// RUN: %clangxx_msan -g %s -o %t && %run %t 0
+
+// Try parameter '1'
+// -------------------------------------------------------
+// RUN: %clangxx_msan -g %s -o %t && not %run %t 1 >%t.out 2>&1
+// RUN: FileCheck --check-prefix STORE-CHECK %s < %t.out
+
+// RUN: %clangxx_msan -mllvm -msan-print-faulting-instruction=1 -g %s -o %t && not %run %t 1 >%t.out 2>&1
+// RUN: FileCheck --check-prefixes VERBOSE-STORE-CHECK,STORE-CHECK %s < %t.out
+
+// RUN: %clangxx_msan -mllvm -msan-print-faulting-instruction=2 -g %s -o %t && not %run %t 1 >%t.out 2>&1
+// RUN: FileCheck --check-prefixes VERY-VERBOSE-STORE-CHECK,STORE-CHECK %s < %t.out
+
+// Try parameter '2', with -fsanitize-memory-param-retval
+// -------------------------------------------------------
+// RUN: %clangxx_msan -fsanitize-memory-param-retval -g %s -o %t && not %run %t 2 >%t.out 2>&1
+// RUN: FileCheck --check-prefix PARAM-CHECK %s < %t.out
+
+// RUN: %clangxx_msan -fsanitize-memory-param-retval -mllvm -msan-print-faulting-instruction=1 -g %s -o %t && not %run %t 2 >%t.out 2>&1
+// RUN: FileCheck --check-prefixes VERBOSE-PARAM-CHECK,PARAM-CHECK %s < %t.out
+
+// RUN: %clangxx_msan -fsanitize-memory-param-retval -mllvm -msan-print-faulting-instruction=2 -g %s -o %t && not %run %t 2 >%t.out 2>&1
+// RUN: FileCheck --check-prefixes VERY-VERBOSE-PARAM-CHECK,PARAM-CHECK %s < %t.out
+
+// Try parameter '2', with -fno-sanitize-memory-param-retval
+// -------------------------------------------------------
+// RUN: %clangxx_msan -fno-sanitize-memory-param-retval -g %s -o %t && not %run %t 2 >%t.out 2>&1
+// RUN: FileCheck --check-prefix NO-PARAM-CHECK %s < %t.out
+
+// RUN: %clangxx_msan -fno-sanitize-memory-param-retval -mllvm -msan-print-faulting-instruction=1 -g %s -o %t && not %run %t 2 >%t.out 2>&1
+// RUN: FileCheck --check-prefixes VERBOSE-NO-PARAM-CHECK,NO-PARAM-CHECK %s < %t.out
+
+// RUN: %clangxx_msan -fno-sanitize-memory-param-retval -mllvm -msan-print-faulting-instruction=2 -g %s -o %t && not %run %t 2 >%t.out 2>&1
+// RUN: FileCheck --check-prefixes VERY-VERBOSE-NO-PARAM-CHECK,NO-PARAM-CHECK %s < %t.out
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#define THRICE(o,t) twice(o,t)
+
+__attribute__((noinline)) extern "C" int twice(int o, int t) {
+ return o + t < 3;
+}
+
+int main(int argc, char *argv[]) {
+ int buf[100];
+ buf[0] = 42;
+ buf[1] = 43;
+
+ if (argc != 2) {
+ printf("Usage: %s index\n", argv[0]);
+ return 1;
+ }
+
+ int index = atoi(argv[1]);
+ int val = buf[index];
+
+ printf("index %d, abs(val) %d, THRICE(val,5) %d\n", index, abs(val), THRICE(val,5));
+ // VERY-VERBOSE-PARAM-CHECK: Instruction that failed the shadow check: %{{.*}} = call noundef i32 @twice(i32 noundef %{{.*}}, i32 noundef 5)
+ // VERBOSE-PARAM-CHECK: Instruction that failed the shadow check: call twice
+ // PARAM-CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
+ // PARAM-CHECK: {{#0 0x.* in main .*print_faulting_inst.cpp:}}[[@LINE-4]]
+
+ if (val)
+ // VERY-VERBOSE-NO-PARAM-CHECK: Instruction that failed the shadow check: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}
+ // VERBOSE-NO-PARAM-CHECK: Instruction that failed the shadow check: br
+ // NO-PARAM-CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
+ // NO-PARAM-CHECK: {{#0 0x.* in main .*print_faulting_inst.cpp:}}[[@LINE-4]]
+ printf("Variable is non-zero\n");
+ else
+ printf("Variable is zero\n");
+
+ int nextval = buf[index + 1];
+ buf[nextval + abs(index)] = twice(index,6);
+ // VERY-VERBOSE-STORE-CHECK: Instruction that failed the shadow check: store i32 %{{.*}}, ptr %{{.*}}
+ // VERBOSE-STORE-CHECK: Instruction that failed the shadow check: store
+ // STORE-CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
+ // STORE-CHECK: {{#0 0x.* in main .*print_faulting_inst.cpp:}}[[@LINE-4]]
+
+ return 0;
+}
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 8e31e8d2a4fbd..c98097c135b40 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -274,6 +274,13 @@ static cl::opt<bool>
cl::desc("propagate shadow through ICmpEQ and ICmpNE"),
cl::Hidden, cl::init(true));
+static cl::opt<uint> ClPrintFaultingInst(
+ "msan-print-faulting-instruction",
+ cl::desc("If set to 1, print the name of the LLVM IR instruction that "
+ "failed the shadow check."
+ "If set to 2, print the full LLVM IR instruction."),
+ cl::Hidden, cl::init(0));
+
static cl::opt<bool>
ClHandleICmpExact("msan-handle-icmp-exact",
cl::desc("exact handling of relational integer ICmp"),
@@ -816,9 +823,14 @@ void MemorySanitizer::createKernelApi(Module &M, const TargetLibraryInfo &TLI) {
VAArgOriginTLS = nullptr;
VAArgOverflowSizeTLS = nullptr;
- WarningFn = M.getOrInsertFunction("__msan_warning",
- TLI.getAttrList(C, {0}, /*Signed=*/false),
- IRB.getVoidTy(), IRB.getInt32Ty());
+ if (ClPrintFaultingInst)
+ WarningFn = M.getOrInsertFunction(
+ "__msan_warning_instname", TLI.getAttrList(C, {0}, /*Signed=*/false),
+ IRB.getVoidTy(), IRB.getInt32Ty(), IRB.getPtrTy());
+ else
+ WarningFn = M.getOrInsertFunction("__msan_warning",
+ TLI.getAttrList(C, {0}, /*Signed=*/false),
+ IRB.getVoidTy(), IRB.getInt32Ty());
// Requests the per-task context state (kmsan_context_state*) from the
// runtime library.
@@ -870,19 +882,37 @@ void MemorySanitizer::createUserspaceApi(Module &M,
const TargetLibraryInfo &TLI) {
IRBuilder<> IRB(*C);
+ Type *Int8Ptr = PointerType::getUnqual(*C);
+
// Create the callback.
// FIXME: this function should have "Cold" calling conv,
// which is not yet implemented.
if (TrackOrigins) {
- StringRef WarningFnName = Recover ? "__msan_warning_with_origin"
- : "__msan_warning_with_origin_noreturn";
- WarningFn = M.getOrInsertFunction(WarningFnName,
- TLI.getAttrList(C, {0}, /*Signed=*/false),
- IRB.getVoidTy(), IRB.getInt32Ty());
+ if (ClPrintFaultingInst) {
+ StringRef WarningFnName =
+ Recover ? "__msan_warning_with_origin_instname"
+ : "__msan_warning_with_origin_noreturn_instname";
+ WarningFn = M.getOrInsertFunction(
+ WarningFnName, TLI.getAttrList(C, {0}, /*Signed=*/false),
+ IRB.getVoidTy(), IRB.getInt32Ty(), Int8Ptr);
+ } else {
+ StringRef WarningFnName = Recover ? "__msan_warning_with_origin"
+ : "__msan_warning_with_origin_noreturn";
+ WarningFn = M.getOrInsertFunction(
+ WarningFnName, TLI.getAttrList(C, {0}, /*Signed=*/false),
+ IRB.getVoidTy(), IRB.getInt32Ty());
+ }
} else {
- StringRef WarningFnName =
- Recover ? "__msan_warning" : "__msan_warning_noreturn";
- WarningFn = M.getOrInsertFunction(WarningFnName, IRB.getVoidTy());
+ if (ClPrintFaultingInst) {
+ StringRef WarningFnName = Recover ? "__msan_warning_instname"
+ : "__msan_warning_noreturn_instname";
+ WarningFn =
+ M.getOrInsertFunction(WarningFnName, IRB.getVoidTy(), Int8Ptr);
+ } else {
+ StringRef WarningFnName =
+ Recover ? "__msan_warning" : "__msan_warning_noreturn";
+ WarningFn = M.getOrInsertFunction(WarningFnName, IRB.getVoidTy());
+ }
}
// Create the global TLS variables.
@@ -914,10 +944,19 @@ void MemorySanitizer::createUserspaceApi(Module &M,
for (size_t AccessSizeIndex = 0; AccessSizeIndex < kNumberOfAccessSizes;
AccessSizeIndex++) {
unsigned AccessSize = 1 << AccessSizeIndex;
- std::string FunctionName = "__msan_maybe_warning_" + itostr(AccessSize);
- MaybeWarningFn[AccessSizeIndex] = M.getOrInsertFunction(
- FunctionName, TLI.getAttrList(C, {0, 1}, /*Signed=*/false),
- IRB.getVoidTy(), IRB.getIntNTy(AccessSize * 8), IRB.getInt32Ty());
+ std::string FunctionName;
+ if (ClPrintFaultingInst) {
+ FunctionName = "__msan_maybe_warning_instname_" + itostr(AccessSize);
+ MaybeWarningFn[AccessSizeIndex] = M.getOrInsertFunction(
+ FunctionName, TLI.getAttrList(C, {0, 1}, /*Signed=*/false),
+ IRB.getVoidTy(), IRB.getIntNTy(AccessSize * 8), IRB.getInt32Ty(),
+ IRB.getPtrTy());
+ } else {
+ FunctionName = "__msan_maybe_warning_" + itostr(AccessSize);
+ MaybeWarningFn[AccessSizeIndex] = M.getOrInsertFunction(
+ FunctionName, TLI.getAttrList(C, {0, 1}, /*Signed=*/false),
+ IRB.getVoidTy(), IRB.getIntNTy(AccessSize * 8), IRB.getInt32Ty());
+ }
FunctionName = "__msan_maybe_store_origin_" + itostr(AccessSize);
MaybeStoreOriginFn[AccessSizeIndex] = M.getOrInsertFunction(
@@ -1387,7 +1426,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
}
/// Helper function to insert a warning at IRB's current insert point.
- void insertWarningFn(IRBuilder<> &IRB, Value *Origin) {
+ void insertWarningFn(IRBuilder<> &IRB, Value *Origin, Value *InstName) {
if (!Origin)
Origin = (Value *)IRB.getInt32(0);
assert(Origin->getType()->isIntegerTy());
@@ -1410,17 +1449,24 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
}
}
- if (MS.CompileKernel || MS.TrackOrigins)
- IRB.CreateCall(MS.WarningFn, Origin)->setCannotMerge();
- else
- IRB.CreateCall(MS.WarningFn)->setCannotMerge();
+ if (ClPrintFaultingInst) {
+ if (MS.CompileKernel || MS.TrackOrigins)
+ IRB.CreateCall(MS.WarningFn, {Origin, InstName})->setCannotMerge();
+ else
+ IRB.CreateCall(MS.WarningFn, {InstName})->setCannotMerge();
+ } else {
+ if (MS.CompileKernel || MS.TrackOrigins)
+ IRB.CreateCall(MS.WarningFn, Origin)->setCannotMerge();
+ else
+ IRB.CreateCall(MS.WarningFn)->setCannotMerge();
+ }
// FIXME: Insert UnreachableInst if !MS.Recover?
// This may invalidate some of the following checks and needs to be done
// at the very end.
}
void materializeOneCheck(IRBuilder<> &IRB, Value *ConvertedShadow,
- Value *Origin) {
+ Value *Origin, Value *InstName) {
const DataLayout &DL = F.getDataLayout();
TypeSize TypeSizeInBits = DL.getTypeSizeInBits(ConvertedShadow->getType());
unsigned SizeIndex = TypeSizeToSizeIndex(TypeSizeInBits);
@@ -1431,9 +1477,18 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
ConvertedShadow = convertShadowToScalar(ConvertedShadow, IRB);
Value *ConvertedShadow2 =
IRB.CreateZExt(ConvertedShadow, IRB.getIntNTy(8 * (1 << SizeIndex)));
- CallBase *CB = IRB.CreateCall(
- Fn, {ConvertedShadow2,
- MS.TrackOrigins && Origin ? Origin : (Value *)IRB.getInt32(0)});
+ CallBase *CB;
+ if (ClPrintFaultingInst)
+ CB = IRB.CreateCall(
+ Fn, {ConvertedShadow2,
+ MS.TrackOrigins && Origin ? Origin : (Value *)IRB.getInt32(0),
+ InstName});
+ else
+ CB = IRB.CreateCall(Fn,
+ {ConvertedShadow2, MS.TrackOrigins && Origin
+ ? Origin
+ : (Value *)IRB.getInt32(0)});
+
CB->addParamAttr(0, Attribute::ZExt);
CB->addParamAttr(1, Attribute::ZExt);
} else {
@@ -1443,7 +1498,9 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
/* Unreachable */ !MS.Recover, MS.ColdCallWeights);
IRB.SetInsertPoint(CheckTerm);
- insertWarningFn(IRB, Origin);
+ // InstName will be ignored by insertWarningFn if ClPrintFaultingInst is
+ // false
+ insertWarningFn(IRB, Origin, InstName);
LLVM_DEBUG(dbgs() << " CHECK: " << *Cmp << "\n");
}
}
@@ -1455,13 +1512,43 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
// correct origin.
bool Combine = !MS.TrackOrigins;
Instruction *Instruction = InstructionChecks.front().OrigIns;
+
+ Value *InstName = nullptr;
+ if (ClPrintFaultingInst >= 1) {
+ IRBuilder<> IRB0(Instruction);
+ std::string str;
+ StringRef InstNameStrRef;
+
+ // Dumping the full instruction is expensive because the operands etc.
+ // likely make the string unique per instruction instance, hence we
+ // offer a choice whether to only print the instruction name.
+ if (ClPrintFaultingInst >= 2) {
+ llvm::raw_string_ostream buf(str);
+ Instruction->print(buf);
+ InstNameStrRef = StringRef(str);
+ } else if (ClPrintFaultingInst >= 1) {
+ if (CallInst *CI = dyn_cast<CallInst>(Instruction)) {
+ if (CI->getCalledFunction()) {
+ Twine description = "call " + CI->getCalledFunction()->getName();
+ SmallVector<char> maybeBuf;
+ InstNameStrRef = description.toStringRef(maybeBuf);
+ } else {
+ InstNameStrRef = StringRef("Unknown call");
+ }
+ } else {
+ InstNameStrRef = StringRef(Instruction->getOpcodeName());
+ }
+ }
+
+ InstName = IRB0.CreateGlobalString(InstNameStrRef);
+ }
+
Value *Shadow = nullptr;
for (const auto &ShadowData : InstructionChecks) {
assert(ShadowData.OrigIns == Instruction);
IRBuilder<> IRB(Instruction);
Value *ConvertedShadow = ShadowData.Shadow;
-
if (auto *ConstantShadow = dyn_cast<Constant>(ConvertedShadow)) {
if (!ClCheckConstantShadow || ConstantShadow->isZeroValue()) {
// Skip, value is initialized or const shadow is ignored.
@@ -1469,7 +1556,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
}
if (llvm::isKnownNonZero(ConvertedShadow, DL)) {
// Report as the value is definitely uninitialized.
- insertWarningFn(IRB, ShadowData.Origin);
+ insertWarningFn(IRB, ShadowData.Origin, InstName);
if (!MS.Recover)
return; // Always fail and stop here, not need to check the rest.
// Skip entire instruction,
@@ -1479,7 +1566,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
}
if (!Combine) {
- materializeOneCheck(IRB, ConvertedShadow, ShadowData.Origin);
+ materializeOneCheck(IRB, ConvertedShadow, ShadowData.Origin, InstName);
continue;
}
@@ -1496,7 +1583,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
if (Shadow) {
assert(Combine);
IRBuilder<> IRB(Instruction);
- materializeOneCheck(IRB, Shadow, nullptr);
+ materializeOneCheck(IRB, Shadow, nullptr, InstName);
}
}
>From 39a5ab94584f292c4f68f82763a50c43a4e01ada Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Mon, 21 Apr 2025 05:56:12 +0000
Subject: [PATCH 02/26] Separate out embedding the instruction vs. printing the
faulting instruction
---
compiler-rt/lib/msan/msan.cpp | 21 ++++++++++++--
compiler-rt/lib/msan/msan_flags.inc | 4 +++
compiler-rt/test/msan/print_faulting_inst.cpp | 20 ++++++-------
.../Instrumentation/MemorySanitizer.cpp | 29 ++++++++++---------
4 files changed, 47 insertions(+), 27 deletions(-)
diff --git a/compiler-rt/lib/msan/msan.cpp b/compiler-rt/lib/msan/msan.cpp
index a22bfe0d7ab32..676b9908bfd43 100644
--- a/compiler-rt/lib/msan/msan.cpp
+++ b/compiler-rt/lib/msan/msan.cpp
@@ -352,9 +352,19 @@ void __sanitizer::BufferedStackTrace::UnwindImpl(
using namespace __msan;
-#define PRINT_FAULTING_INSTRUCTION(instname) \
- Printf("Instruction that failed the shadow check: %s\n", instname); \
- Printf("\n");
+#define PRINT_FAULTING_INSTRUCTION(instname) \
+ if (__msan::flags()->print_faulting_instruction) { \
+ Printf("Instruction that failed the shadow check: %s\n", instname); \
+ Printf("\n"); \
+ }
+
+#define CANNOT_PRINT_FAULTING_INSTRUCTION \
+ if (__msan::flags()->print_faulting_instruction) { \
+ Printf( \
+ "Error: print_faulting_instruction requested but code was not " \
+ "instrumented with -mllvm -embed-faulting-instruction.\n"); \
+ Printf("\n"); \
+ }
#define MSAN_MAYBE_WARNING_INSTNAME(type, size, instname) \
void __msan_maybe_warning_instname_##size(type s, u32 o, char *instname) { \
@@ -379,6 +389,7 @@ MSAN_MAYBE_WARNING_INSTNAME(u64, 8, instname)
void __msan_maybe_warning_##size(type s, u32 o) { \
GET_CALLER_PC_BP; \
if (UNLIKELY(s)) { \
+ CANNOT_PRINT_FAULTING_INSTRUCTION; \
PrintWarningWithOrigin(pc, bp, o); \
if (__msan::flags()->halt_on_error) { \
Printf("Exiting\n"); \
@@ -410,6 +421,7 @@ MSAN_MAYBE_STORE_ORIGIN(u32, 4)
MSAN_MAYBE_STORE_ORIGIN(u64, 8)
void __msan_warning() {
+ CANNOT_PRINT_FAULTING_INSTRUCTION;
GET_CALLER_PC_BP;
PrintWarningWithOrigin(pc, bp, 0);
if (__msan::flags()->halt_on_error) {
@@ -421,6 +433,7 @@ void __msan_warning() {
}
void __msan_warning_noreturn() {
+ CANNOT_PRINT_FAULTING_INSTRUCTION;
GET_CALLER_PC_BP;
PrintWarningWithOrigin(pc, bp, 0);
if (__msan::flags()->print_stats)
@@ -430,6 +443,7 @@ void __msan_warning_noreturn() {
}
void __msan_warning_with_origin(u32 origin) {
+ CANNOT_PRINT_FAULTING_INSTRUCTION;
GET_CALLER_PC_BP;
PrintWarningWithOrigin(pc, bp, origin);
if (__msan::flags()->halt_on_error) {
@@ -441,6 +455,7 @@ void __msan_warning_with_origin(u32 origin) {
}
void __msan_warning_with_origin_noreturn(u32 origin) {
+ CANNOT_PRINT_FAULTING_INSTRUCTION;
GET_CALLER_PC_BP;
PrintWarningWithOrigin(pc, bp, origin);
if (__msan::flags()->print_stats)
diff --git a/compiler-rt/lib/msan/msan_flags.inc b/compiler-rt/lib/msan/msan_flags.inc
index 16db26bd42ed9..30a17fb3db9a8 100644
--- a/compiler-rt/lib/msan/msan_flags.inc
+++ b/compiler-rt/lib/msan/msan_flags.inc
@@ -25,6 +25,10 @@ MSAN_FLAG(bool, poison_stack_with_zeroes, false, "")
MSAN_FLAG(bool, poison_in_malloc, true, "")
MSAN_FLAG(bool, poison_in_free, true, "")
MSAN_FLAG(bool, poison_in_dtor, true, "")
+MSAN_FLAG(bool, print_faulting_instruction, false,
+ "When reporting a UUM, print the name of the faulting instruction. "
+ "Note: requires code to be instrumented with -mllvm "
+ "-msan-embed-faulting-instruction.")
MSAN_FLAG(bool, report_umrs, true, "")
MSAN_FLAG(bool, wrap_signals, true, "")
MSAN_FLAG(bool, print_stats, false, "")
diff --git a/compiler-rt/test/msan/print_faulting_inst.cpp b/compiler-rt/test/msan/print_faulting_inst.cpp
index 09c5f22ada132..1cbea9eb035e0 100644
--- a/compiler-rt/test/msan/print_faulting_inst.cpp
+++ b/compiler-rt/test/msan/print_faulting_inst.cpp
@@ -1,38 +1,38 @@
// Try parameter '0' (program runs cleanly)
// -------------------------------------------------------
-// RUN: %clangxx_msan -g %s -o %t && %run %t 0
+// RUN: env MSAN_OPTIONS=print_faulting_instruction=true %clangxx_msan -g %s -o %t && env MSAN_OPTIONS=print_faulting_instruction=true %run %t 0
// Try parameter '1'
// -------------------------------------------------------
-// RUN: %clangxx_msan -g %s -o %t && not %run %t 1 >%t.out 2>&1
+// RUN: %clangxx_msan -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 1 >%t.out 2>&1
// RUN: FileCheck --check-prefix STORE-CHECK %s < %t.out
-// RUN: %clangxx_msan -mllvm -msan-print-faulting-instruction=1 -g %s -o %t && not %run %t 1 >%t.out 2>&1
+// RUN: %clangxx_msan -mllvm -msan-embed-faulting-instruction=1 -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 1 >%t.out 2>&1
// RUN: FileCheck --check-prefixes VERBOSE-STORE-CHECK,STORE-CHECK %s < %t.out
-// RUN: %clangxx_msan -mllvm -msan-print-faulting-instruction=2 -g %s -o %t && not %run %t 1 >%t.out 2>&1
+// RUN: %clangxx_msan -mllvm -msan-embed-faulting-instruction=2 -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 1 >%t.out 2>&1
// RUN: FileCheck --check-prefixes VERY-VERBOSE-STORE-CHECK,STORE-CHECK %s < %t.out
// Try parameter '2', with -fsanitize-memory-param-retval
// -------------------------------------------------------
-// RUN: %clangxx_msan -fsanitize-memory-param-retval -g %s -o %t && not %run %t 2 >%t.out 2>&1
+// RUN: %clangxx_msan -fsanitize-memory-param-retval -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 2 >%t.out 2>&1
// RUN: FileCheck --check-prefix PARAM-CHECK %s < %t.out
-// RUN: %clangxx_msan -fsanitize-memory-param-retval -mllvm -msan-print-faulting-instruction=1 -g %s -o %t && not %run %t 2 >%t.out 2>&1
+// RUN: %clangxx_msan -fsanitize-memory-param-retval -mllvm -msan-embed-faulting-instruction=1 -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 2 >%t.out 2>&1
// RUN: FileCheck --check-prefixes VERBOSE-PARAM-CHECK,PARAM-CHECK %s < %t.out
-// RUN: %clangxx_msan -fsanitize-memory-param-retval -mllvm -msan-print-faulting-instruction=2 -g %s -o %t && not %run %t 2 >%t.out 2>&1
+// RUN: %clangxx_msan -fsanitize-memory-param-retval -mllvm -msan-embed-faulting-instruction=2 -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 2 >%t.out 2>&1
// RUN: FileCheck --check-prefixes VERY-VERBOSE-PARAM-CHECK,PARAM-CHECK %s < %t.out
// Try parameter '2', with -fno-sanitize-memory-param-retval
// -------------------------------------------------------
-// RUN: %clangxx_msan -fno-sanitize-memory-param-retval -g %s -o %t && not %run %t 2 >%t.out 2>&1
+// RUN: %clangxx_msan -fno-sanitize-memory-param-retval -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 2 >%t.out 2>&1
// RUN: FileCheck --check-prefix NO-PARAM-CHECK %s < %t.out
-// RUN: %clangxx_msan -fno-sanitize-memory-param-retval -mllvm -msan-print-faulting-instruction=1 -g %s -o %t && not %run %t 2 >%t.out 2>&1
+// RUN: %clangxx_msan -fno-sanitize-memory-param-retval -mllvm -msan-embed-faulting-instruction=1 -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 2 >%t.out 2>&1
// RUN: FileCheck --check-prefixes VERBOSE-NO-PARAM-CHECK,NO-PARAM-CHECK %s < %t.out
-// RUN: %clangxx_msan -fno-sanitize-memory-param-retval -mllvm -msan-print-faulting-instruction=2 -g %s -o %t && not %run %t 2 >%t.out 2>&1
+// RUN: %clangxx_msan -fno-sanitize-memory-param-retval -mllvm -msan-embed-faulting-instruction=2 -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 2 >%t.out 2>&1
// RUN: FileCheck --check-prefixes VERY-VERBOSE-NO-PARAM-CHECK,NO-PARAM-CHECK %s < %t.out
#include <stdio.h>
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index c98097c135b40..047cf4312cbe3 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -274,11 +274,12 @@ static cl::opt<bool>
cl::desc("propagate shadow through ICmpEQ and ICmpNE"),
cl::Hidden, cl::init(true));
-static cl::opt<uint> ClPrintFaultingInst(
- "msan-print-faulting-instruction",
- cl::desc("If set to 1, print the name of the LLVM IR instruction that "
+static cl::opt<uint> ClEmbedFaultingInst(
+ "msan-embed-faulting-instruction",
+ cl::desc("If set to 1, embed the name of the LLVM IR instruction that "
"failed the shadow check."
- "If set to 2, print the full LLVM IR instruction."),
+ "If set to 2, embed the full LLVM IR instruction. "
+ "The runtime can print the embedded instruction."),
cl::Hidden, cl::init(0));
static cl::opt<bool>
@@ -823,7 +824,7 @@ void MemorySanitizer::createKernelApi(Module &M, const TargetLibraryInfo &TLI) {
VAArgOriginTLS = nullptr;
VAArgOverflowSizeTLS = nullptr;
- if (ClPrintFaultingInst)
+ if (ClEmbedFaultingInst)
WarningFn = M.getOrInsertFunction(
"__msan_warning_instname", TLI.getAttrList(C, {0}, /*Signed=*/false),
IRB.getVoidTy(), IRB.getInt32Ty(), IRB.getPtrTy());
@@ -888,7 +889,7 @@ void MemorySanitizer::createUserspaceApi(Module &M,
// FIXME: this function should have "Cold" calling conv,
// which is not yet implemented.
if (TrackOrigins) {
- if (ClPrintFaultingInst) {
+ if (ClEmbedFaultingInst) {
StringRef WarningFnName =
Recover ? "__msan_warning_with_origin_instname"
: "__msan_warning_with_origin_noreturn_instname";
@@ -903,7 +904,7 @@ void MemorySanitizer::createUserspaceApi(Module &M,
IRB.getVoidTy(), IRB.getInt32Ty());
}
} else {
- if (ClPrintFaultingInst) {
+ if (ClEmbedFaultingInst) {
StringRef WarningFnName = Recover ? "__msan_warning_instname"
: "__msan_warning_noreturn_instname";
WarningFn =
@@ -945,7 +946,7 @@ void MemorySanitizer::createUserspaceApi(Module &M,
AccessSizeIndex++) {
unsigned AccessSize = 1 << AccessSizeIndex;
std::string FunctionName;
- if (ClPrintFaultingInst) {
+ if (ClEmbedFaultingInst) {
FunctionName = "__msan_maybe_warning_instname_" + itostr(AccessSize);
MaybeWarningFn[AccessSizeIndex] = M.getOrInsertFunction(
FunctionName, TLI.getAttrList(C, {0, 1}, /*Signed=*/false),
@@ -1449,7 +1450,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
}
}
- if (ClPrintFaultingInst) {
+ if (ClEmbedFaultingInst) {
if (MS.CompileKernel || MS.TrackOrigins)
IRB.CreateCall(MS.WarningFn, {Origin, InstName})->setCannotMerge();
else
@@ -1478,7 +1479,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
Value *ConvertedShadow2 =
IRB.CreateZExt(ConvertedShadow, IRB.getIntNTy(8 * (1 << SizeIndex)));
CallBase *CB;
- if (ClPrintFaultingInst)
+ if (ClEmbedFaultingInst)
CB = IRB.CreateCall(
Fn, {ConvertedShadow2,
MS.TrackOrigins && Origin ? Origin : (Value *)IRB.getInt32(0),
@@ -1498,7 +1499,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
/* Unreachable */ !MS.Recover, MS.ColdCallWeights);
IRB.SetInsertPoint(CheckTerm);
- // InstName will be ignored by insertWarningFn if ClPrintFaultingInst is
+ // InstName will be ignored by insertWarningFn if ClEmbedFaultingInst is
// false
insertWarningFn(IRB, Origin, InstName);
LLVM_DEBUG(dbgs() << " CHECK: " << *Cmp << "\n");
@@ -1514,7 +1515,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
Instruction *Instruction = InstructionChecks.front().OrigIns;
Value *InstName = nullptr;
- if (ClPrintFaultingInst >= 1) {
+ if (ClEmbedFaultingInst >= 1) {
IRBuilder<> IRB0(Instruction);
std::string str;
StringRef InstNameStrRef;
@@ -1522,11 +1523,11 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
// Dumping the full instruction is expensive because the operands etc.
// likely make the string unique per instruction instance, hence we
// offer a choice whether to only print the instruction name.
- if (ClPrintFaultingInst >= 2) {
+ if (ClEmbedFaultingInst >= 2) {
llvm::raw_string_ostream buf(str);
Instruction->print(buf);
InstNameStrRef = StringRef(str);
- } else if (ClPrintFaultingInst >= 1) {
+ } else if (ClEmbedFaultingInst >= 1) {
if (CallInst *CI = dyn_cast<CallInst>(Instruction)) {
if (CI->getCalledFunction()) {
Twine description = "call " + CI->getCalledFunction()->getName();
>From cacf9f23439eef2340cc98abab3b64e54c2f2699 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Mon, 21 Apr 2025 15:43:05 +0000
Subject: [PATCH 03/26] clang-format
---
compiler-rt/test/msan/print_faulting_inst.cpp | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/compiler-rt/test/msan/print_faulting_inst.cpp b/compiler-rt/test/msan/print_faulting_inst.cpp
index 1cbea9eb035e0..7274f4d14de0f 100644
--- a/compiler-rt/test/msan/print_faulting_inst.cpp
+++ b/compiler-rt/test/msan/print_faulting_inst.cpp
@@ -38,7 +38,7 @@
#include <stdio.h>
#include <stdlib.h>
-#define THRICE(o,t) twice(o,t)
+#define THRICE(o, t) twice(o, t)
__attribute__((noinline)) extern "C" int twice(int o, int t) {
return o + t < 3;
@@ -57,7 +57,8 @@ int main(int argc, char *argv[]) {
int index = atoi(argv[1]);
int val = buf[index];
- printf("index %d, abs(val) %d, THRICE(val,5) %d\n", index, abs(val), THRICE(val,5));
+ printf("index %d, abs(val) %d, THRICE(val,5) %d\n", index, abs(val),
+ THRICE(val, 5));
// VERY-VERBOSE-PARAM-CHECK: Instruction that failed the shadow check: %{{.*}} = call noundef i32 @twice(i32 noundef %{{.*}}, i32 noundef 5)
// VERBOSE-PARAM-CHECK: Instruction that failed the shadow check: call twice
// PARAM-CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
@@ -73,7 +74,7 @@ int main(int argc, char *argv[]) {
printf("Variable is zero\n");
int nextval = buf[index + 1];
- buf[nextval + abs(index)] = twice(index,6);
+ buf[nextval + abs(index)] = twice(index, 6);
// VERY-VERBOSE-STORE-CHECK: Instruction that failed the shadow check: store i32 %{{.*}}, ptr %{{.*}}
// VERBOSE-STORE-CHECK: Instruction that failed the shadow check: store
// STORE-CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
>From e539a52b4e4072ee4097b06b49a50fac0a0cf03e Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Mon, 21 Apr 2025 23:19:26 +0000
Subject: [PATCH 04/26] Address Florian's feedback Also add "[EXPERIMENTAL]"
disclaimer
---
compiler-rt/lib/msan/msan_flags.inc | 3 +-
compiler-rt/test/msan/print_faulting_inst.cpp | 23 ++++---
.../Instrumentation/MemorySanitizer.h | 7 +++
.../Instrumentation/MemorySanitizer.cpp | 62 ++++++++++++-------
4 files changed, 63 insertions(+), 32 deletions(-)
diff --git a/compiler-rt/lib/msan/msan_flags.inc b/compiler-rt/lib/msan/msan_flags.inc
index 30a17fb3db9a8..969a1cc8cc448 100644
--- a/compiler-rt/lib/msan/msan_flags.inc
+++ b/compiler-rt/lib/msan/msan_flags.inc
@@ -26,7 +26,8 @@ MSAN_FLAG(bool, poison_in_malloc, true, "")
MSAN_FLAG(bool, poison_in_free, true, "")
MSAN_FLAG(bool, poison_in_dtor, true, "")
MSAN_FLAG(bool, print_faulting_instruction, false,
- "When reporting a UUM, print the name of the faulting instruction. "
+ "[EXPERIMENTAL] When reporting a UUM, print the name of the faulting "
+ "instruction. "
"Note: requires code to be instrumented with -mllvm "
"-msan-embed-faulting-instruction.")
MSAN_FLAG(bool, report_umrs, true, "")
diff --git a/compiler-rt/test/msan/print_faulting_inst.cpp b/compiler-rt/test/msan/print_faulting_inst.cpp
index 7274f4d14de0f..bf39ce2ce79db 100644
--- a/compiler-rt/test/msan/print_faulting_inst.cpp
+++ b/compiler-rt/test/msan/print_faulting_inst.cpp
@@ -1,16 +1,19 @@
// Try parameter '0' (program runs cleanly)
// -------------------------------------------------------
-// RUN: env MSAN_OPTIONS=print_faulting_instruction=true %clangxx_msan -g %s -o %t && env MSAN_OPTIONS=print_faulting_instruction=true %run %t 0
+// RUN: %clangxx_msan -g %s -o %t && env MSAN_OPTIONS=print_faulting_instruction=true %run %t 0
// Try parameter '1'
// -------------------------------------------------------
// RUN: %clangxx_msan -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 1 >%t.out 2>&1
// RUN: FileCheck --check-prefix STORE-CHECK %s < %t.out
-// RUN: %clangxx_msan -mllvm -msan-embed-faulting-instruction=1 -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 1 >%t.out 2>&1
+// RUN: %clangxx_msan -mllvm -msan-embed-faulting-instruction=none -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 1 >%t.out 2>&1
+// RUN: FileCheck --check-prefix STORE-CHECK %s < %t.out
+
+// RUN: %clangxx_msan -mllvm -msan-embed-faulting-instruction=name -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 1 >%t.out 2>&1
// RUN: FileCheck --check-prefixes VERBOSE-STORE-CHECK,STORE-CHECK %s < %t.out
-// RUN: %clangxx_msan -mllvm -msan-embed-faulting-instruction=2 -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 1 >%t.out 2>&1
+// RUN: %clangxx_msan -mllvm -msan-embed-faulting-instruction=full -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 1 >%t.out 2>&1
// RUN: FileCheck --check-prefixes VERY-VERBOSE-STORE-CHECK,STORE-CHECK %s < %t.out
// Try parameter '2', with -fsanitize-memory-param-retval
@@ -18,10 +21,13 @@
// RUN: %clangxx_msan -fsanitize-memory-param-retval -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 2 >%t.out 2>&1
// RUN: FileCheck --check-prefix PARAM-CHECK %s < %t.out
-// RUN: %clangxx_msan -fsanitize-memory-param-retval -mllvm -msan-embed-faulting-instruction=1 -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 2 >%t.out 2>&1
+// RUN: %clangxx_msan -fsanitize-memory-param-retval -mllvm -msan-embed-faulting-instruction=none -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 2 >%t.out 2>&1
+// RUN: FileCheck --check-prefix PARAM-CHECK %s < %t.out
+
+// RUN: %clangxx_msan -fsanitize-memory-param-retval -mllvm -msan-embed-faulting-instruction=name -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 2 >%t.out 2>&1
// RUN: FileCheck --check-prefixes VERBOSE-PARAM-CHECK,PARAM-CHECK %s < %t.out
-// RUN: %clangxx_msan -fsanitize-memory-param-retval -mllvm -msan-embed-faulting-instruction=2 -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 2 >%t.out 2>&1
+// RUN: %clangxx_msan -fsanitize-memory-param-retval -mllvm -msan-embed-faulting-instruction=full -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 2 >%t.out 2>&1
// RUN: FileCheck --check-prefixes VERY-VERBOSE-PARAM-CHECK,PARAM-CHECK %s < %t.out
// Try parameter '2', with -fno-sanitize-memory-param-retval
@@ -29,10 +35,13 @@
// RUN: %clangxx_msan -fno-sanitize-memory-param-retval -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 2 >%t.out 2>&1
// RUN: FileCheck --check-prefix NO-PARAM-CHECK %s < %t.out
-// RUN: %clangxx_msan -fno-sanitize-memory-param-retval -mllvm -msan-embed-faulting-instruction=1 -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 2 >%t.out 2>&1
+// RUN: %clangxx_msan -fno-sanitize-memory-param-retval -mllvm -msan-embed-faulting-instruction=none -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 2 >%t.out 2>&1
+// RUN: FileCheck --check-prefix NO-PARAM-CHECK %s < %t.out
+
+// RUN: %clangxx_msan -fno-sanitize-memory-param-retval -mllvm -msan-embed-faulting-instruction=name -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 2 >%t.out 2>&1
// RUN: FileCheck --check-prefixes VERBOSE-NO-PARAM-CHECK,NO-PARAM-CHECK %s < %t.out
-// RUN: %clangxx_msan -fno-sanitize-memory-param-retval -mllvm -msan-embed-faulting-instruction=2 -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 2 >%t.out 2>&1
+// RUN: %clangxx_msan -fno-sanitize-memory-param-retval -mllvm -msan-embed-faulting-instruction=full -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 2 >%t.out 2>&1
// RUN: FileCheck --check-prefixes VERY-VERBOSE-NO-PARAM-CHECK,NO-PARAM-CHECK %s < %t.out
#include <stdio.h>
diff --git a/llvm/include/llvm/Transforms/Instrumentation/MemorySanitizer.h b/llvm/include/llvm/Transforms/Instrumentation/MemorySanitizer.h
index f88d832351118..dffa32fab366f 100644
--- a/llvm/include/llvm/Transforms/Instrumentation/MemorySanitizer.h
+++ b/llvm/include/llvm/Transforms/Instrumentation/MemorySanitizer.h
@@ -21,6 +21,13 @@ class Module;
class StringRef;
class raw_ostream;
+/// Mode of MSan -embed-faulting-instruction
+enum class MSanEmbedFaultingInstructionMode {
+ None, ///< Do not embed the faulting instruction information
+ Name, ///< Embed the LLVM IR instruction name (excluding operands)
+ Full, ///< Embed the complete LLVM IR instruction (including operands)
+};
+
struct MemorySanitizerOptions {
MemorySanitizerOptions() : MemorySanitizerOptions(0, false, false, false){};
MemorySanitizerOptions(int TrackOrigins, bool Recover, bool Kernel)
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 047cf4312cbe3..5ff7af7c39bdc 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -274,13 +274,19 @@ static cl::opt<bool>
cl::desc("propagate shadow through ICmpEQ and ICmpNE"),
cl::Hidden, cl::init(true));
-static cl::opt<uint> ClEmbedFaultingInst(
+static cl::opt<MSanEmbedFaultingInstructionMode> ClEmbedFaultingInst(
"msan-embed-faulting-instruction",
- cl::desc("If set to 1, embed the name of the LLVM IR instruction that "
- "failed the shadow check."
- "If set to 2, embed the full LLVM IR instruction. "
- "The runtime can print the embedded instruction."),
- cl::Hidden, cl::init(0));
+ cl::desc("[EXPERIMENTAL] Sets whether to embed the LLVM IR instruction "
+ "info for each UUM check."),
+ cl::values(
+ clEnumValN(MSanEmbedFaultingInstructionMode::None, "none",
+ "Do not embed the faulting instruction information."),
+ clEnumValN(MSanEmbedFaultingInstructionMode::Name, "name",
+ "Embed the LLVM IR instruction name (excluding operands)."),
+ clEnumValN(
+ MSanEmbedFaultingInstructionMode::Full, "full",
+ "Embed the complete LLVM IR instruction (including operands).")),
+ cl::Hidden, cl::init(MSanEmbedFaultingInstructionMode::None));
static cl::opt<bool>
ClHandleICmpExact("msan-handle-icmp-exact",
@@ -824,7 +830,7 @@ void MemorySanitizer::createKernelApi(Module &M, const TargetLibraryInfo &TLI) {
VAArgOriginTLS = nullptr;
VAArgOverflowSizeTLS = nullptr;
- if (ClEmbedFaultingInst)
+ if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None)
WarningFn = M.getOrInsertFunction(
"__msan_warning_instname", TLI.getAttrList(C, {0}, /*Signed=*/false),
IRB.getVoidTy(), IRB.getInt32Ty(), IRB.getPtrTy());
@@ -889,7 +895,7 @@ void MemorySanitizer::createUserspaceApi(Module &M,
// FIXME: this function should have "Cold" calling conv,
// which is not yet implemented.
if (TrackOrigins) {
- if (ClEmbedFaultingInst) {
+ if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
StringRef WarningFnName =
Recover ? "__msan_warning_with_origin_instname"
: "__msan_warning_with_origin_noreturn_instname";
@@ -904,7 +910,7 @@ void MemorySanitizer::createUserspaceApi(Module &M,
IRB.getVoidTy(), IRB.getInt32Ty());
}
} else {
- if (ClEmbedFaultingInst) {
+ if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
StringRef WarningFnName = Recover ? "__msan_warning_instname"
: "__msan_warning_noreturn_instname";
WarningFn =
@@ -946,7 +952,7 @@ void MemorySanitizer::createUserspaceApi(Module &M,
AccessSizeIndex++) {
unsigned AccessSize = 1 << AccessSizeIndex;
std::string FunctionName;
- if (ClEmbedFaultingInst) {
+ if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
FunctionName = "__msan_maybe_warning_instname_" + itostr(AccessSize);
MaybeWarningFn[AccessSizeIndex] = M.getOrInsertFunction(
FunctionName, TLI.getAttrList(C, {0, 1}, /*Signed=*/false),
@@ -1450,7 +1456,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
}
}
- if (ClEmbedFaultingInst) {
+ if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
if (MS.CompileKernel || MS.TrackOrigins)
IRB.CreateCall(MS.WarningFn, {Origin, InstName})->setCannotMerge();
else
@@ -1479,7 +1485,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
Value *ConvertedShadow2 =
IRB.CreateZExt(ConvertedShadow, IRB.getIntNTy(8 * (1 << SizeIndex)));
CallBase *CB;
- if (ClEmbedFaultingInst)
+ if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None)
CB = IRB.CreateCall(
Fn, {ConvertedShadow2,
MS.TrackOrigins && Origin ? Origin : (Value *)IRB.getInt32(0),
@@ -1500,22 +1506,15 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
IRB.SetInsertPoint(CheckTerm);
// InstName will be ignored by insertWarningFn if ClEmbedFaultingInst is
- // false
+ // MSanEmbedFaultingInstructionMode::None
insertWarningFn(IRB, Origin, InstName);
LLVM_DEBUG(dbgs() << " CHECK: " << *Cmp << "\n");
}
}
- void materializeInstructionChecks(
- ArrayRef<ShadowOriginAndInsertPoint> InstructionChecks) {
- const DataLayout &DL = F.getDataLayout();
- // Disable combining in some cases. TrackOrigins checks each shadow to pick
- // correct origin.
- bool Combine = !MS.TrackOrigins;
- Instruction *Instruction = InstructionChecks.front().OrigIns;
-
+ Value *getInstName(Instruction *Instruction) {
Value *InstName = nullptr;
- if (ClEmbedFaultingInst >= 1) {
+ if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
IRBuilder<> IRB0(Instruction);
std::string str;
StringRef InstNameStrRef;
@@ -1523,11 +1522,12 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
// Dumping the full instruction is expensive because the operands etc.
// likely make the string unique per instruction instance, hence we
// offer a choice whether to only print the instruction name.
- if (ClEmbedFaultingInst >= 2) {
+ if (ClEmbedFaultingInst == MSanEmbedFaultingInstructionMode::Full) {
llvm::raw_string_ostream buf(str);
Instruction->print(buf);
InstNameStrRef = StringRef(str);
- } else if (ClEmbedFaultingInst >= 1) {
+ } else if (ClEmbedFaultingInst ==
+ MSanEmbedFaultingInstructionMode::Name) {
if (CallInst *CI = dyn_cast<CallInst>(Instruction)) {
if (CI->getCalledFunction()) {
Twine description = "call " + CI->getCalledFunction()->getName();
@@ -1544,12 +1544,26 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
InstName = IRB0.CreateGlobalString(InstNameStrRef);
}
+ return InstName;
+ }
+
+ void materializeInstructionChecks(
+ ArrayRef<ShadowOriginAndInsertPoint> InstructionChecks) {
+ const DataLayout &DL = F.getDataLayout();
+ // Disable combining in some cases. TrackOrigins checks each shadow to pick
+ // correct origin.
+ bool Combine = !MS.TrackOrigins;
+ Instruction *Instruction = InstructionChecks.front().OrigIns;
+
+ Value *InstName = getInstName(Instruction);
+
Value *Shadow = nullptr;
for (const auto &ShadowData : InstructionChecks) {
assert(ShadowData.OrigIns == Instruction);
IRBuilder<> IRB(Instruction);
Value *ConvertedShadow = ShadowData.Shadow;
+
if (auto *ConstantShadow = dyn_cast<Constant>(ConvertedShadow)) {
if (!ClCheckConstantShadow || ConstantShadow->isZeroValue()) {
// Skip, value is initialized or const shadow is ignored.
>From 2f2f2484c197c7db47997f334cabbd829d6aad00 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Mon, 21 Apr 2025 23:22:07 +0000
Subject: [PATCH 05/26] Address one other piece of feedback
---
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 5ff7af7c39bdc..f7252d2b75b6e 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -1516,13 +1516,13 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
Value *InstName = nullptr;
if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
IRBuilder<> IRB0(Instruction);
- std::string str;
StringRef InstNameStrRef;
// Dumping the full instruction is expensive because the operands etc.
// likely make the string unique per instruction instance, hence we
// offer a choice whether to only print the instruction name.
if (ClEmbedFaultingInst == MSanEmbedFaultingInstructionMode::Full) {
+ std::string str;
llvm::raw_string_ostream buf(str);
Instruction->print(buf);
InstNameStrRef = StringRef(str);
>From 5ac8339524a1e71956c8a0423af85fe0aea37c66 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Mon, 21 Apr 2025 23:30:41 +0000
Subject: [PATCH 06/26] Reword comment
---
compiler-rt/lib/msan/msan.cpp | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/compiler-rt/lib/msan/msan.cpp b/compiler-rt/lib/msan/msan.cpp
index 676b9908bfd43..6d4566e0582b0 100644
--- a/compiler-rt/lib/msan/msan.cpp
+++ b/compiler-rt/lib/msan/msan.cpp
@@ -464,13 +464,13 @@ void __msan_warning_with_origin_noreturn(u32 origin) {
Die();
}
-// We duplicate the non _instname function's body because we don't want to
-// pollute the stack traces with an additional function call.
-//
-// We can't use a simple macro wrapper, because the instrumentation pass
-// expects function symbols.
-// We don't add instname as a parameter everywhere (with a check whether the
-// value is null) to avoid polluting the fastpath.
+// We duplicate the non _instname function's body because:
+// - we don't want to pollute the stack traces with an additional function
+// call.
+// - we can't use a simple macro wrapper, because the instrumentation pass
+// expects function symbols.
+// - we don't add instname as a parameter everywhere (with a check whether the
+// value is null) to avoid polluting the fastpath.
void __msan_warning_instname(char *instname) {
PRINT_FAULTING_INSTRUCTION(instname);
GET_CALLER_PC_BP;
>From 3dec04fb0d8d1a3dee31f1f4bfa9c20535a26e54 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 22 Apr 2025 01:48:26 +0000
Subject: [PATCH 07/26] Rename CANT_PRINT_FAULTING_INSTRUCTION to
WARN_IF_PRINT_FAULTING_INSTRUCTION_REQUESTED
---
compiler-rt/lib/msan/msan.cpp | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/compiler-rt/lib/msan/msan.cpp b/compiler-rt/lib/msan/msan.cpp
index 6d4566e0582b0..01cf0d5bddffe 100644
--- a/compiler-rt/lib/msan/msan.cpp
+++ b/compiler-rt/lib/msan/msan.cpp
@@ -358,7 +358,7 @@ using namespace __msan;
Printf("\n"); \
}
-#define CANNOT_PRINT_FAULTING_INSTRUCTION \
+#define WARN_IF_PRINT_FAULTING_INSTRUCTION_REQUESTED \
if (__msan::flags()->print_faulting_instruction) { \
Printf( \
"Error: print_faulting_instruction requested but code was not " \
@@ -421,7 +421,7 @@ MSAN_MAYBE_STORE_ORIGIN(u32, 4)
MSAN_MAYBE_STORE_ORIGIN(u64, 8)
void __msan_warning() {
- CANNOT_PRINT_FAULTING_INSTRUCTION;
+ WARN_IF_PRINT_FAULTING_INSTRUCTION_REQUESTED;
GET_CALLER_PC_BP;
PrintWarningWithOrigin(pc, bp, 0);
if (__msan::flags()->halt_on_error) {
@@ -433,7 +433,7 @@ void __msan_warning() {
}
void __msan_warning_noreturn() {
- CANNOT_PRINT_FAULTING_INSTRUCTION;
+ WARN_IF_PRINT_FAULTING_INSTRUCTION_REQUESTED;
GET_CALLER_PC_BP;
PrintWarningWithOrigin(pc, bp, 0);
if (__msan::flags()->print_stats)
@@ -443,7 +443,7 @@ void __msan_warning_noreturn() {
}
void __msan_warning_with_origin(u32 origin) {
- CANNOT_PRINT_FAULTING_INSTRUCTION;
+ WARN_IF_PRINT_FAULTING_INSTRUCTION_REQUESTED;
GET_CALLER_PC_BP;
PrintWarningWithOrigin(pc, bp, origin);
if (__msan::flags()->halt_on_error) {
@@ -455,7 +455,7 @@ void __msan_warning_with_origin(u32 origin) {
}
void __msan_warning_with_origin_noreturn(u32 origin) {
- CANNOT_PRINT_FAULTING_INSTRUCTION;
+ WARN_IF_PRINT_FAULTING_INSTRUCTION_REQUESTED;
GET_CALLER_PC_BP;
PrintWarningWithOrigin(pc, bp, origin);
if (__msan::flags()->print_stats)
>From 97f827b659b4e0aafb94cfbe8f7b7cbbfa2ca3f8 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 22 Apr 2025 01:55:22 +0000
Subject: [PATCH 08/26] Revert scope level change because it causes
use-after-scope
---
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index f7252d2b75b6e..e34a4206a3203 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -1517,12 +1517,14 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
IRBuilder<> IRB0(Instruction);
StringRef InstNameStrRef;
+ // Keep str at this scope level because it is indirectly needed by
+ // CreateGlobalString
+ std::string str;
// Dumping the full instruction is expensive because the operands etc.
// likely make the string unique per instruction instance, hence we
// offer a choice whether to only print the instruction name.
if (ClEmbedFaultingInst == MSanEmbedFaultingInstructionMode::Full) {
- std::string str;
llvm::raw_string_ostream buf(str);
Instruction->print(buf);
InstNameStrRef = StringRef(str);
>From f41479cbc55459086fbc0a48588b52d4fc467cde Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 22 Apr 2025 02:04:55 +0000
Subject: [PATCH 09/26] Move maybeBuf to higher scope too
---
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index e34a4206a3203..8316fd6491834 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -1517,9 +1517,10 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
IRBuilder<> IRB0(Instruction);
StringRef InstNameStrRef;
- // Keep str at this scope level because it is indirectly needed by
- // CreateGlobalString
+ // Keep str and maybeBuf at this scope level because they may be
+ // indirectly needed by CreateGlobalString
std::string str;
+ SmallVector<char> maybeBuf;
// Dumping the full instruction is expensive because the operands etc.
// likely make the string unique per instruction instance, hence we
@@ -1533,7 +1534,6 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
if (CallInst *CI = dyn_cast<CallInst>(Instruction)) {
if (CI->getCalledFunction()) {
Twine description = "call " + CI->getCalledFunction()->getName();
- SmallVector<char> maybeBuf;
InstNameStrRef = description.toStringRef(maybeBuf);
} else {
InstNameStrRef = StringRef("Unknown call");
>From 19fd6df6c459323e48a3d8dc40835a5c5e609437 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 22 Apr 2025 02:10:36 +0000
Subject: [PATCH 10/26] Rename IRB0 -> IRB since there is no longer name
collision
---
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 8316fd6491834..86661d81ba852 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -1515,7 +1515,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
Value *getInstName(Instruction *Instruction) {
Value *InstName = nullptr;
if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
- IRBuilder<> IRB0(Instruction);
+ IRBuilder<> IRB(Instruction);
StringRef InstNameStrRef;
// Keep str and maybeBuf at this scope level because they may be
// indirectly needed by CreateGlobalString
@@ -1543,7 +1543,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
}
}
- InstName = IRB0.CreateGlobalString(InstNameStrRef);
+ InstName = IRB.CreateGlobalString(InstNameStrRef);
}
return InstName;
>From 97bece224d5cd085cd2fd5f42383e3a7b2600bf1 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 22 Apr 2025 02:48:58 +0000
Subject: [PATCH 11/26] Add newline (and restart CI)
---
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 86661d81ba852..7d001cd83ff26 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -1514,6 +1514,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
Value *getInstName(Instruction *Instruction) {
Value *InstName = nullptr;
+
if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
IRBuilder<> IRB(Instruction);
StringRef InstNameStrRef;
>From 069356dbd7109024f99d463c240fb1c02d854419 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 22 Apr 2025 04:02:18 +0000
Subject: [PATCH 12/26] Rerun CI
>From e244b025a3c0f7600ff773a79d47bf144aafd43f Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 22 Apr 2025 05:14:53 +0000
Subject: [PATCH 13/26] Fix rename
---
compiler-rt/lib/msan/msan.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compiler-rt/lib/msan/msan.cpp b/compiler-rt/lib/msan/msan.cpp
index 01cf0d5bddffe..f35cd0fefc874 100644
--- a/compiler-rt/lib/msan/msan.cpp
+++ b/compiler-rt/lib/msan/msan.cpp
@@ -389,7 +389,7 @@ MSAN_MAYBE_WARNING_INSTNAME(u64, 8, instname)
void __msan_maybe_warning_##size(type s, u32 o) { \
GET_CALLER_PC_BP; \
if (UNLIKELY(s)) { \
- CANNOT_PRINT_FAULTING_INSTRUCTION; \
+ WARN_IF_PRINT_FAULTING_INSTRUCTION_REQUESTED; \
PrintWarningWithOrigin(pc, bp, o); \
if (__msan::flags()->halt_on_error) { \
Printf("Exiting\n"); \
>From 84728f136fe0932bf1fb5c0c5e318fc2b5e44e76 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 22 Apr 2025 05:51:47 +0000
Subject: [PATCH 14/26] Refactor to minimize code duplication between x() and
x_instname() variants
---
compiler-rt/lib/msan/msan.cpp | 131 ++++++++++++++++------------------
1 file changed, 60 insertions(+), 71 deletions(-)
diff --git a/compiler-rt/lib/msan/msan.cpp b/compiler-rt/lib/msan/msan.cpp
index f35cd0fefc874..7f30919869d89 100644
--- a/compiler-rt/lib/msan/msan.cpp
+++ b/compiler-rt/lib/msan/msan.cpp
@@ -420,99 +420,88 @@ MSAN_MAYBE_STORE_ORIGIN(u16, 2)
MSAN_MAYBE_STORE_ORIGIN(u32, 4)
MSAN_MAYBE_STORE_ORIGIN(u64, 8)
+// These macros to reuse the function body are kludgy, but are the better than
+// the alternatives:
+// - call a common function: this pollutes the stack traces
+// - have x_instname() be a simple macro wrapper around x(): the
+// instrumentation pass expects function symbols
+// - add instname as a parameter everywhere (with a check whether instname is
+// null): this pollutes the fastpath
+// - duplicate the function body: redundancy is redundant
+#define __MSAN_WARNING_BODY \
+ GET_CALLER_PC_BP; \
+ PrintWarningWithOrigin(pc, bp, 0); \
+ if (__msan::flags()->halt_on_error) { \
+ if (__msan::flags()->print_stats) \
+ ReportStats(); \
+ Printf("Exiting\n"); \
+ Die(); \
+ }
+
void __msan_warning() {
WARN_IF_PRINT_FAULTING_INSTRUCTION_REQUESTED;
- GET_CALLER_PC_BP;
- PrintWarningWithOrigin(pc, bp, 0);
- if (__msan::flags()->halt_on_error) {
- if (__msan::flags()->print_stats)
- ReportStats();
- Printf("Exiting\n");
- Die();
- }
+ __MSAN_WARNING_BODY
}
+void __msan_warning_instname(char *instname) {
+ PRINT_FAULTING_INSTRUCTION(instname);
+ __MSAN_WARNING_BODY
+}
+
+#define __MSAN_WARNING_NORETURN_BODY \
+ GET_CALLER_PC_BP; \
+ PrintWarningWithOrigin(pc, bp, 0); \
+ if (__msan::flags()->print_stats) \
+ ReportStats(); \
+ Printf("Exiting\n"); \
+ Die();
+
void __msan_warning_noreturn() {
WARN_IF_PRINT_FAULTING_INSTRUCTION_REQUESTED;
- GET_CALLER_PC_BP;
- PrintWarningWithOrigin(pc, bp, 0);
- if (__msan::flags()->print_stats)
- ReportStats();
- Printf("Exiting\n");
- Die();
+ __MSAN_WARNING_NORETURN_BODY
}
-void __msan_warning_with_origin(u32 origin) {
- WARN_IF_PRINT_FAULTING_INSTRUCTION_REQUESTED;
- GET_CALLER_PC_BP;
- PrintWarningWithOrigin(pc, bp, origin);
- if (__msan::flags()->halt_on_error) {
- if (__msan::flags()->print_stats)
- ReportStats();
- Printf("Exiting\n");
- Die();
- }
+void __msan_warning_noreturn_instname(char *instname) {
+ PRINT_FAULTING_INSTRUCTION(instname);
+ __MSAN_WARNING_NORETURN_BODY
}
-void __msan_warning_with_origin_noreturn(u32 origin) {
+#define __MSAN_WARNING_WITH_ORIGIN_BODY(origin) \
+ GET_CALLER_PC_BP; \
+ PrintWarningWithOrigin(pc, bp, origin); \
+ if (__msan::flags()->halt_on_error) { \
+ if (__msan::flags()->print_stats) \
+ ReportStats(); \
+ Printf("Exiting\n"); \
+ Die(); \
+ }
+
+void __msan_warning_with_origin(u32 origin) {
WARN_IF_PRINT_FAULTING_INSTRUCTION_REQUESTED;
- GET_CALLER_PC_BP;
- PrintWarningWithOrigin(pc, bp, origin);
- if (__msan::flags()->print_stats)
- ReportStats();
- Printf("Exiting\n");
- Die();
+ __MSAN_WARNING_WITH_ORIGIN_BODY(origin)
}
-// We duplicate the non _instname function's body because:
-// - we don't want to pollute the stack traces with an additional function
-// call.
-// - we can't use a simple macro wrapper, because the instrumentation pass
-// expects function symbols.
-// - we don't add instname as a parameter everywhere (with a check whether the
-// value is null) to avoid polluting the fastpath.
-void __msan_warning_instname(char *instname) {
+void __msan_warning_with_origin_instname(u32 origin, char *instname) {
PRINT_FAULTING_INSTRUCTION(instname);
- GET_CALLER_PC_BP;
- PrintWarningWithOrigin(pc, bp, 0);
- if (__msan::flags()->halt_on_error) {
- if (__msan::flags()->print_stats)
- ReportStats();
- Printf("Exiting\n");
- Die();
- }
+ __MSAN_WARNING_WITH_ORIGIN_BODY(origin)
}
-void __msan_warning_noreturn_instname(char *instname) {
- PRINT_FAULTING_INSTRUCTION(instname);
- GET_CALLER_PC_BP;
- PrintWarningWithOrigin(pc, bp, 0);
- if (__msan::flags()->print_stats)
- ReportStats();
- Printf("Exiting\n");
+#define __MSAN_WARNING_WITH_ORIGIN_NORETURN_BODY(origin) \
+ GET_CALLER_PC_BP; \
+ PrintWarningWithOrigin(pc, bp, origin); \
+ if (__msan::flags()->print_stats) \
+ ReportStats(); \
+ Printf("Exiting\n"); \
Die();
-}
-void __msan_warning_with_origin_instname(u32 origin, char *instname) {
- PRINT_FAULTING_INSTRUCTION(instname);
- GET_CALLER_PC_BP;
- PrintWarningWithOrigin(pc, bp, origin);
- if (__msan::flags()->halt_on_error) {
- if (__msan::flags()->print_stats)
- ReportStats();
- Printf("Exiting\n");
- Die();
- }
+void __msan_warning_with_origin_noreturn(u32 origin) {
+ WARN_IF_PRINT_FAULTING_INSTRUCTION_REQUESTED;
+ __MSAN_WARNING_WITH_ORIGIN_NORETURN_BODY(origin)
}
void __msan_warning_with_origin_noreturn_instname(u32 origin, char *instname) {
PRINT_FAULTING_INSTRUCTION(instname);
- GET_CALLER_PC_BP;
- PrintWarningWithOrigin(pc, bp, origin);
- if (__msan::flags()->print_stats)
- ReportStats();
- Printf("Exiting\n");
- Die();
+ __MSAN_WARNING_WITH_ORIGIN_NORETURN_BODY(origin)
}
static void OnStackUnwind(const SignalContext &sig, const void *,
>From f2ac5c7208e1473e92fe27f7c2f4bd08acb3be1d Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 22 Apr 2025 05:52:46 +0000
Subject: [PATCH 15/26] clang-format
---
compiler-rt/lib/msan/msan.cpp | 46 +++++++++++++++++------------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/compiler-rt/lib/msan/msan.cpp b/compiler-rt/lib/msan/msan.cpp
index 7f30919869d89..6d8b2cfcb7787 100644
--- a/compiler-rt/lib/msan/msan.cpp
+++ b/compiler-rt/lib/msan/msan.cpp
@@ -428,14 +428,14 @@ MSAN_MAYBE_STORE_ORIGIN(u64, 8)
// - add instname as a parameter everywhere (with a check whether instname is
// null): this pollutes the fastpath
// - duplicate the function body: redundancy is redundant
-#define __MSAN_WARNING_BODY \
- GET_CALLER_PC_BP; \
- PrintWarningWithOrigin(pc, bp, 0); \
+#define __MSAN_WARNING_BODY \
+ GET_CALLER_PC_BP; \
+ PrintWarningWithOrigin(pc, bp, 0); \
if (__msan::flags()->halt_on_error) { \
- if (__msan::flags()->print_stats) \
- ReportStats(); \
- Printf("Exiting\n"); \
- Die(); \
+ if (__msan::flags()->print_stats) \
+ ReportStats(); \
+ Printf("Exiting\n"); \
+ Die(); \
}
void __msan_warning() {
@@ -449,11 +449,11 @@ void __msan_warning_instname(char *instname) {
}
#define __MSAN_WARNING_NORETURN_BODY \
- GET_CALLER_PC_BP; \
+ GET_CALLER_PC_BP; \
PrintWarningWithOrigin(pc, bp, 0); \
- if (__msan::flags()->print_stats) \
- ReportStats(); \
- Printf("Exiting\n"); \
+ if (__msan::flags()->print_stats) \
+ ReportStats(); \
+ Printf("Exiting\n"); \
Die();
void __msan_warning_noreturn() {
@@ -467,13 +467,13 @@ void __msan_warning_noreturn_instname(char *instname) {
}
#define __MSAN_WARNING_WITH_ORIGIN_BODY(origin) \
- GET_CALLER_PC_BP; \
- PrintWarningWithOrigin(pc, bp, origin); \
- if (__msan::flags()->halt_on_error) { \
- if (__msan::flags()->print_stats) \
- ReportStats(); \
- Printf("Exiting\n"); \
- Die(); \
+ GET_CALLER_PC_BP; \
+ PrintWarningWithOrigin(pc, bp, origin); \
+ if (__msan::flags()->halt_on_error) { \
+ if (__msan::flags()->print_stats) \
+ ReportStats(); \
+ Printf("Exiting\n"); \
+ Die(); \
}
void __msan_warning_with_origin(u32 origin) {
@@ -487,11 +487,11 @@ void __msan_warning_with_origin_instname(u32 origin, char *instname) {
}
#define __MSAN_WARNING_WITH_ORIGIN_NORETURN_BODY(origin) \
- GET_CALLER_PC_BP; \
- PrintWarningWithOrigin(pc, bp, origin); \
- if (__msan::flags()->print_stats) \
- ReportStats(); \
- Printf("Exiting\n"); \
+ GET_CALLER_PC_BP; \
+ PrintWarningWithOrigin(pc, bp, origin); \
+ if (__msan::flags()->print_stats) \
+ ReportStats(); \
+ Printf("Exiting\n"); \
Die();
void __msan_warning_with_origin_noreturn(u32 origin) {
>From 12f1a08d16ce4c0741b81cf601d2ea6c63741a50 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 22 Apr 2025 05:53:30 +0000
Subject: [PATCH 16/26] Typo
---
compiler-rt/lib/msan/msan.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/compiler-rt/lib/msan/msan.cpp b/compiler-rt/lib/msan/msan.cpp
index 6d8b2cfcb7787..42684d14edfdb 100644
--- a/compiler-rt/lib/msan/msan.cpp
+++ b/compiler-rt/lib/msan/msan.cpp
@@ -420,8 +420,8 @@ MSAN_MAYBE_STORE_ORIGIN(u16, 2)
MSAN_MAYBE_STORE_ORIGIN(u32, 4)
MSAN_MAYBE_STORE_ORIGIN(u64, 8)
-// These macros to reuse the function body are kludgy, but are the better than
-// the alternatives:
+// These macros to reuse the function body are kludgy, but are better than the
+// alternatives:
// - call a common function: this pollutes the stack traces
// - have x_instname() be a simple macro wrapper around x(): the
// instrumentation pass expects function symbols
>From c639c69dac92a2399726ff07e4d5d5d9bd406edc Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 22 Apr 2025 15:06:00 +0000
Subject: [PATCH 17/26] Rerun CI
>From 44ca7d03ec5f0c43d8b28d88490742b2604cd69e Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 22 Apr 2025 17:13:00 +0000
Subject: [PATCH 18/26] Use FunctionType::get to reduce duplication for
creating _instname variants
---
.../Instrumentation/MemorySanitizer.cpp | 56 +++++++++----------
1 file changed, 26 insertions(+), 30 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 7d001cd83ff26..7ec6791665de9 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -830,14 +830,15 @@ void MemorySanitizer::createKernelApi(Module &M, const TargetLibraryInfo &TLI) {
VAArgOriginTLS = nullptr;
VAArgOverflowSizeTLS = nullptr;
- if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None)
- WarningFn = M.getOrInsertFunction(
- "__msan_warning_instname", TLI.getAttrList(C, {0}, /*Signed=*/false),
- IRB.getVoidTy(), IRB.getInt32Ty(), IRB.getPtrTy());
- else
- WarningFn = M.getOrInsertFunction("__msan_warning",
- TLI.getAttrList(C, {0}, /*Signed=*/false),
- IRB.getVoidTy(), IRB.getInt32Ty());
+ SmallVector<Type *, 4> ArgsTy = {IRB.getInt32Ty()};
+ StringRef FnName = "__msan_warning";
+ if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
+ ArgsTy.push_back(IRB.getPtrTy());
+ FnName = "__msan_warning_instname";
+ }
+ WarningFn = M.getOrInsertFunction(
+ FnName, FunctionType::get(IRB.getVoidTy(), ArgsTy, false),
+ TLI.getAttrList(C, {0}, /*Signed=*/false));
// Requests the per-task context state (kmsan_context_state*) from the
// runtime library.
@@ -889,37 +890,32 @@ void MemorySanitizer::createUserspaceApi(Module &M,
const TargetLibraryInfo &TLI) {
IRBuilder<> IRB(*C);
- Type *Int8Ptr = PointerType::getUnqual(*C);
-
// Create the callback.
// FIXME: this function should have "Cold" calling conv,
// which is not yet implemented.
if (TrackOrigins) {
+ SmallVector<Type *, 4> ArgsTy = {IRB.getInt32Ty()};
+ StringRef WarningFnName = Recover ? "__msan_warning_with_origin"
+ : "__msan_warning_with_origin_noreturn";
if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
- StringRef WarningFnName =
- Recover ? "__msan_warning_with_origin_instname"
- : "__msan_warning_with_origin_noreturn_instname";
- WarningFn = M.getOrInsertFunction(
- WarningFnName, TLI.getAttrList(C, {0}, /*Signed=*/false),
- IRB.getVoidTy(), IRB.getInt32Ty(), Int8Ptr);
- } else {
- StringRef WarningFnName = Recover ? "__msan_warning_with_origin"
- : "__msan_warning_with_origin_noreturn";
- WarningFn = M.getOrInsertFunction(
- WarningFnName, TLI.getAttrList(C, {0}, /*Signed=*/false),
- IRB.getVoidTy(), IRB.getInt32Ty());
+ ArgsTy.push_back(IRB.getPtrTy());
+ WarningFnName = Recover ? "__msan_warning_with_origin_instname"
+ : "__msan_warning_with_origin_noreturn_instname";
}
+ WarningFn = M.getOrInsertFunction(
+ WarningFnName, FunctionType::get(IRB.getVoidTy(), ArgsTy, false),
+ TLI.getAttrList(C, {0}, /*Signed=*/false));
} else {
+ SmallVector<Type *, 4> ArgsTy = {};
+ StringRef WarningFnName =
+ Recover ? "__msan_warning" : "__msan_warning_noreturn";
if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
- StringRef WarningFnName = Recover ? "__msan_warning_instname"
- : "__msan_warning_noreturn_instname";
- WarningFn =
- M.getOrInsertFunction(WarningFnName, IRB.getVoidTy(), Int8Ptr);
- } else {
- StringRef WarningFnName =
- Recover ? "__msan_warning" : "__msan_warning_noreturn";
- WarningFn = M.getOrInsertFunction(WarningFnName, IRB.getVoidTy());
+ ArgsTy.push_back(IRB.getPtrTy());
+ WarningFnName = Recover ? "__msan_warning_instname"
+ : "__msan_warning_noreturn_instname";
}
+ WarningFn = M.getOrInsertFunction(
+ WarningFnName, FunctionType::get(IRB.getVoidTy(), ArgsTy, false));
}
// Create the global TLS variables.
>From cdf5671370d3cbada64ebeac72f7a15d759b9da8 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 22 Apr 2025 18:14:01 +0000
Subject: [PATCH 19/26] More simplification with FunctionType::get
---
.../Instrumentation/MemorySanitizer.cpp | 52 ++++++++-----------
1 file changed, 21 insertions(+), 31 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 7ec6791665de9..70107460c2017 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -947,19 +947,16 @@ void MemorySanitizer::createUserspaceApi(Module &M,
for (size_t AccessSizeIndex = 0; AccessSizeIndex < kNumberOfAccessSizes;
AccessSizeIndex++) {
unsigned AccessSize = 1 << AccessSizeIndex;
- std::string FunctionName;
+ std::string FunctionName = "__msan_maybe_warning_" + itostr(AccessSize);
+ SmallVector<Type *, 4> ArgsTy = {IRB.getIntNTy(AccessSize * 8),
+ IRB.getInt32Ty()};
if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
FunctionName = "__msan_maybe_warning_instname_" + itostr(AccessSize);
- MaybeWarningFn[AccessSizeIndex] = M.getOrInsertFunction(
- FunctionName, TLI.getAttrList(C, {0, 1}, /*Signed=*/false),
- IRB.getVoidTy(), IRB.getIntNTy(AccessSize * 8), IRB.getInt32Ty(),
- IRB.getPtrTy());
- } else {
- FunctionName = "__msan_maybe_warning_" + itostr(AccessSize);
- MaybeWarningFn[AccessSizeIndex] = M.getOrInsertFunction(
- FunctionName, TLI.getAttrList(C, {0, 1}, /*Signed=*/false),
- IRB.getVoidTy(), IRB.getIntNTy(AccessSize * 8), IRB.getInt32Ty());
+ ArgsTy.push_back(IRB.getPtrTy());
}
+ MaybeWarningFn[AccessSizeIndex] = M.getOrInsertFunction(
+ FunctionName, FunctionType::get(IRB.getVoidTy(), ArgsTy, false),
+ TLI.getAttrList(C, {0, 1}, /*Signed=*/false));
FunctionName = "__msan_maybe_store_origin_" + itostr(AccessSize);
MaybeStoreOriginFn[AccessSizeIndex] = M.getOrInsertFunction(
@@ -1452,17 +1449,13 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
}
}
- if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
- if (MS.CompileKernel || MS.TrackOrigins)
- IRB.CreateCall(MS.WarningFn, {Origin, InstName})->setCannotMerge();
- else
- IRB.CreateCall(MS.WarningFn, {InstName})->setCannotMerge();
- } else {
- if (MS.CompileKernel || MS.TrackOrigins)
- IRB.CreateCall(MS.WarningFn, Origin)->setCannotMerge();
- else
- IRB.CreateCall(MS.WarningFn)->setCannotMerge();
- }
+ SmallVector<Value *, 4> Args;
+ if (MS.CompileKernel || MS.TrackOrigins)
+ Args.push_back(Origin);
+ if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None)
+ Args.push_back(InstName);
+ IRB.CreateCall(MS.WarningFn, Args)->setCannotMerge();
+
// FIXME: Insert UnreachableInst if !MS.Recover?
// This may invalidate some of the following checks and needs to be done
// at the very end.
@@ -1480,17 +1473,14 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
ConvertedShadow = convertShadowToScalar(ConvertedShadow, IRB);
Value *ConvertedShadow2 =
IRB.CreateZExt(ConvertedShadow, IRB.getIntNTy(8 * (1 << SizeIndex)));
- CallBase *CB;
+
+ SmallVector<Value *, 4> Args = {
+ ConvertedShadow2,
+ MS.TrackOrigins && Origin ? Origin : (Value *)IRB.getInt32(0)};
if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None)
- CB = IRB.CreateCall(
- Fn, {ConvertedShadow2,
- MS.TrackOrigins && Origin ? Origin : (Value *)IRB.getInt32(0),
- InstName});
- else
- CB = IRB.CreateCall(Fn,
- {ConvertedShadow2, MS.TrackOrigins && Origin
- ? Origin
- : (Value *)IRB.getInt32(0)});
+ Args.push_back(InstName);
+
+ CallBase *CB = IRB.CreateCall(Fn, Args);
CB->addParamAttr(0, Attribute::ZExt);
CB->addParamAttr(1, Attribute::ZExt);
>From 1ef2ac5b7df6c8d074ccecf1550b9ddad1ea65cd Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 22 Apr 2025 19:52:57 +0000
Subject: [PATCH 20/26] Refactor to use getWarningFnName
---
.../Instrumentation/MemorySanitizer.cpp | 41 +++++++++++--------
1 file changed, 23 insertions(+), 18 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 70107460c2017..e6f0f1e86675d 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -817,6 +817,22 @@ MemorySanitizer::getOrInsertMsanMetadataFunction(Module &M, StringRef Name,
std::forward<ArgsTy>(Args)...);
}
+StringRef getWarningFnName(bool TrackOrigins, bool Recover, bool EmbedFaultingInst) {
+ StringRef warningFnName [2][2][2]
+ = {
+ {
+ {"__msan_warning_noreturn", "__msan_warning_noreturn_instname"},
+ {"__msan_warning", "__msan_warning_instname"}
+ },
+ {
+ {"__msan_warning_with_origin_noreturn", "__msan_warning_with_origin_noreturn_instname"},
+ {"__msan_warning_with_origin", "__msan_warning_with_origin_instname"},
+ }
+ };
+
+ return warningFnName[TrackOrigins][Recover][EmbedFaultingInst];
+}
+
/// Create KMSAN API callbacks.
void MemorySanitizer::createKernelApi(Module &M, const TargetLibraryInfo &TLI) {
IRBuilder<> IRB(*C);
@@ -831,11 +847,9 @@ void MemorySanitizer::createKernelApi(Module &M, const TargetLibraryInfo &TLI) {
VAArgOverflowSizeTLS = nullptr;
SmallVector<Type *, 4> ArgsTy = {IRB.getInt32Ty()};
- StringRef FnName = "__msan_warning";
- if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
+ StringRef FnName = getWarningFnName(/*TrackOrigins=*/ false, /*Recover=*/ true, ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None);
+ if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None)
ArgsTy.push_back(IRB.getPtrTy());
- FnName = "__msan_warning_instname";
- }
WarningFn = M.getOrInsertFunction(
FnName, FunctionType::get(IRB.getVoidTy(), ArgsTy, false),
TLI.getAttrList(C, {0}, /*Signed=*/false));
@@ -893,27 +907,18 @@ void MemorySanitizer::createUserspaceApi(Module &M,
// Create the callback.
// FIXME: this function should have "Cold" calling conv,
// which is not yet implemented.
+ StringRef WarningFnName = getWarningFnName(TrackOrigins, Recover, ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None);
+ SmallVector<Type *, 4> ArgsTy = {};
if (TrackOrigins) {
- SmallVector<Type *, 4> ArgsTy = {IRB.getInt32Ty()};
- StringRef WarningFnName = Recover ? "__msan_warning_with_origin"
- : "__msan_warning_with_origin_noreturn";
- if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
+ ArgsTy.push_back (IRB.getInt32Ty());
+ if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None)
ArgsTy.push_back(IRB.getPtrTy());
- WarningFnName = Recover ? "__msan_warning_with_origin_instname"
- : "__msan_warning_with_origin_noreturn_instname";
- }
WarningFn = M.getOrInsertFunction(
WarningFnName, FunctionType::get(IRB.getVoidTy(), ArgsTy, false),
TLI.getAttrList(C, {0}, /*Signed=*/false));
} else {
- SmallVector<Type *, 4> ArgsTy = {};
- StringRef WarningFnName =
- Recover ? "__msan_warning" : "__msan_warning_noreturn";
- if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
+ if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None)
ArgsTy.push_back(IRB.getPtrTy());
- WarningFnName = Recover ? "__msan_warning_instname"
- : "__msan_warning_noreturn_instname";
- }
WarningFn = M.getOrInsertFunction(
WarningFnName, FunctionType::get(IRB.getVoidTy(), ArgsTy, false));
}
>From 91bd6bd1561c3229be9a21c9300e0b3a4be14279 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 22 Apr 2025 19:59:56 +0000
Subject: [PATCH 21/26] clang-format
---
.../Instrumentation/MemorySanitizer.cpp | 32 ++++++++++---------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index e6f0f1e86675d..34135db34ed85 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -817,18 +817,16 @@ MemorySanitizer::getOrInsertMsanMetadataFunction(Module &M, StringRef Name,
std::forward<ArgsTy>(Args)...);
}
-StringRef getWarningFnName(bool TrackOrigins, bool Recover, bool EmbedFaultingInst) {
- StringRef warningFnName [2][2][2]
- = {
- {
- {"__msan_warning_noreturn", "__msan_warning_noreturn_instname"},
- {"__msan_warning", "__msan_warning_instname"}
- },
- {
- {"__msan_warning_with_origin_noreturn", "__msan_warning_with_origin_noreturn_instname"},
- {"__msan_warning_with_origin", "__msan_warning_with_origin_instname"},
- }
- };
+StringRef getWarningFnName(bool TrackOrigins, bool Recover,
+ bool EmbedFaultingInst) {
+ StringRef warningFnName[2][2][2] = {
+ {{"__msan_warning_noreturn", "__msan_warning_noreturn_instname"},
+ {"__msan_warning", "__msan_warning_instname"}},
+ {
+ {"__msan_warning_with_origin_noreturn",
+ "__msan_warning_with_origin_noreturn_instname"},
+ {"__msan_warning_with_origin", "__msan_warning_with_origin_instname"},
+ }};
return warningFnName[TrackOrigins][Recover][EmbedFaultingInst];
}
@@ -847,7 +845,9 @@ void MemorySanitizer::createKernelApi(Module &M, const TargetLibraryInfo &TLI) {
VAArgOverflowSizeTLS = nullptr;
SmallVector<Type *, 4> ArgsTy = {IRB.getInt32Ty()};
- StringRef FnName = getWarningFnName(/*TrackOrigins=*/ false, /*Recover=*/ true, ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None);
+ StringRef FnName = getWarningFnName(
+ /*TrackOrigins=*/false, /*Recover=*/true,
+ ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None);
if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None)
ArgsTy.push_back(IRB.getPtrTy());
WarningFn = M.getOrInsertFunction(
@@ -907,10 +907,12 @@ void MemorySanitizer::createUserspaceApi(Module &M,
// Create the callback.
// FIXME: this function should have "Cold" calling conv,
// which is not yet implemented.
- StringRef WarningFnName = getWarningFnName(TrackOrigins, Recover, ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None);
+ StringRef WarningFnName = getWarningFnName(
+ TrackOrigins, Recover,
+ ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None);
SmallVector<Type *, 4> ArgsTy = {};
if (TrackOrigins) {
- ArgsTy.push_back (IRB.getInt32Ty());
+ ArgsTy.push_back(IRB.getInt32Ty());
if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None)
ArgsTy.push_back(IRB.getPtrTy());
WarningFn = M.getOrInsertFunction(
>From c8c0762ddd3f372b52ca0854c0b0781acaa41087 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 22 Apr 2025 20:46:37 +0000
Subject: [PATCH 22/26] Format getWarningFnName
---
.../Instrumentation/MemorySanitizer.cpp | 29 +++++++++++++++----
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 34135db34ed85..48baec3116501 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -820,12 +820,31 @@ MemorySanitizer::getOrInsertMsanMetadataFunction(Module &M, StringRef Name,
StringRef getWarningFnName(bool TrackOrigins, bool Recover,
bool EmbedFaultingInst) {
StringRef warningFnName[2][2][2] = {
- {{"__msan_warning_noreturn", "__msan_warning_noreturn_instname"},
- {"__msan_warning", "__msan_warning_instname"}},
{
- {"__msan_warning_with_origin_noreturn",
- "__msan_warning_with_origin_noreturn_instname"},
- {"__msan_warning_with_origin", "__msan_warning_with_origin_instname"},
+ // TrackOrigins=false
+ {
+ // Recover=false
+ "__msan_warning_noreturn", // EmbedFaultingInst=false
+ "__msan_warning_noreturn_instname" // EmbedFaultingInst=true
+ },
+ {
+ // Recover=true
+ "__msan_warning", // EmbedFaultingInst=false
+ "__msan_warning_instname" // EmbedFaultingInst=true
+ },
+ },
+ {
+ // TrackOrigins=true
+ {
+ // Recover=false
+ "__msan_warning_with_origin_noreturn", // EmbedFaultingInst=false
+ "__msan_warning_with_origin_noreturn_instname" // EmbedFaultingInst=true
+ },
+ {
+ // Recover=true
+ "__msan_warning_with_origin", // EmbedFaultingInst=false
+ "__msan_warning_with_origin_instname" // EmbedFaultingInst=true
+ },
}};
return warningFnName[TrackOrigins][Recover][EmbedFaultingInst];
>From 7b9cebbbf66eaa4dfae6ce6fcd9855dcc1259151 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 22 Apr 2025 22:03:10 +0000
Subject: [PATCH 23/26] Change macro to function
---
compiler-rt/lib/msan/msan.cpp | 42 ++++++++++++++++++-----------------
1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/compiler-rt/lib/msan/msan.cpp b/compiler-rt/lib/msan/msan.cpp
index 42684d14edfdb..00d7972d3fa20 100644
--- a/compiler-rt/lib/msan/msan.cpp
+++ b/compiler-rt/lib/msan/msan.cpp
@@ -352,26 +352,28 @@ void __sanitizer::BufferedStackTrace::UnwindImpl(
using namespace __msan;
-#define PRINT_FAULTING_INSTRUCTION(instname) \
- if (__msan::flags()->print_faulting_instruction) { \
- Printf("Instruction that failed the shadow check: %s\n", instname); \
- Printf("\n"); \
+static inline void PrintFaultingInstruction(char *instname) {
+ if (__msan::flags()->print_faulting_instruction) {
+ Printf("Instruction that failed the shadow check: %s\n", instname);
+ Printf("\n");
}
+}
-#define WARN_IF_PRINT_FAULTING_INSTRUCTION_REQUESTED \
- if (__msan::flags()->print_faulting_instruction) { \
- Printf( \
- "Error: print_faulting_instruction requested but code was not " \
- "instrumented with -mllvm -embed-faulting-instruction.\n"); \
- Printf("\n"); \
+static inline void WarnIfPrintFaultingInstructionRequested() {
+ if (__msan::flags()->print_faulting_instruction) {
+ Printf(
+ "Error: print_faulting_instruction requested but code was not "
+ "instrumented with -mllvm -embed-faulting-instruction.\n");
+ Printf("\n");
}
+}
#define MSAN_MAYBE_WARNING_INSTNAME(type, size, instname) \
void __msan_maybe_warning_instname_##size(type s, u32 o, char *instname) { \
GET_CALLER_PC_BP; \
if (UNLIKELY(s)) { \
if (instname) \
- PRINT_FAULTING_INSTRUCTION(instname); \
+ PrintFaultingInstruction(instname); \
PrintWarningWithOrigin(pc, bp, o); \
if (__msan::flags()->halt_on_error) { \
Printf("Exiting\n"); \
@@ -389,7 +391,7 @@ MSAN_MAYBE_WARNING_INSTNAME(u64, 8, instname)
void __msan_maybe_warning_##size(type s, u32 o) { \
GET_CALLER_PC_BP; \
if (UNLIKELY(s)) { \
- WARN_IF_PRINT_FAULTING_INSTRUCTION_REQUESTED; \
+ WarnIfPrintFaultingInstructionRequested(); \
PrintWarningWithOrigin(pc, bp, o); \
if (__msan::flags()->halt_on_error) { \
Printf("Exiting\n"); \
@@ -439,12 +441,12 @@ MSAN_MAYBE_STORE_ORIGIN(u64, 8)
}
void __msan_warning() {
- WARN_IF_PRINT_FAULTING_INSTRUCTION_REQUESTED;
+ WarnIfPrintFaultingInstructionRequested();
__MSAN_WARNING_BODY
}
void __msan_warning_instname(char *instname) {
- PRINT_FAULTING_INSTRUCTION(instname);
+ PrintFaultingInstruction(instname);
__MSAN_WARNING_BODY
}
@@ -457,12 +459,12 @@ void __msan_warning_instname(char *instname) {
Die();
void __msan_warning_noreturn() {
- WARN_IF_PRINT_FAULTING_INSTRUCTION_REQUESTED;
+ WarnIfPrintFaultingInstructionRequested();
__MSAN_WARNING_NORETURN_BODY
}
void __msan_warning_noreturn_instname(char *instname) {
- PRINT_FAULTING_INSTRUCTION(instname);
+ PrintFaultingInstruction(instname);
__MSAN_WARNING_NORETURN_BODY
}
@@ -477,12 +479,12 @@ void __msan_warning_noreturn_instname(char *instname) {
}
void __msan_warning_with_origin(u32 origin) {
- WARN_IF_PRINT_FAULTING_INSTRUCTION_REQUESTED;
+ WarnIfPrintFaultingInstructionRequested();
__MSAN_WARNING_WITH_ORIGIN_BODY(origin)
}
void __msan_warning_with_origin_instname(u32 origin, char *instname) {
- PRINT_FAULTING_INSTRUCTION(instname);
+ PrintFaultingInstruction(instname);
__MSAN_WARNING_WITH_ORIGIN_BODY(origin)
}
@@ -495,12 +497,12 @@ void __msan_warning_with_origin_instname(u32 origin, char *instname) {
Die();
void __msan_warning_with_origin_noreturn(u32 origin) {
- WARN_IF_PRINT_FAULTING_INSTRUCTION_REQUESTED;
+ WarnIfPrintFaultingInstructionRequested();
__MSAN_WARNING_WITH_ORIGIN_NORETURN_BODY(origin)
}
void __msan_warning_with_origin_noreturn_instname(u32 origin, char *instname) {
- PRINT_FAULTING_INSTRUCTION(instname);
+ PrintFaultingInstruction(instname);
__MSAN_WARNING_WITH_ORIGIN_NORETURN_BODY(origin)
}
>From 277467f5201c2f79c2d5feb4c37a24186eb71a1d Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 22 Apr 2025 22:59:45 +0000
Subject: [PATCH 24/26] PrintFaultingInstruction ->
PrintFaultingInstructionIfRequested
---
compiler-rt/lib/msan/msan.cpp | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/compiler-rt/lib/msan/msan.cpp b/compiler-rt/lib/msan/msan.cpp
index 00d7972d3fa20..8cc38d98e4dc5 100644
--- a/compiler-rt/lib/msan/msan.cpp
+++ b/compiler-rt/lib/msan/msan.cpp
@@ -352,7 +352,7 @@ void __sanitizer::BufferedStackTrace::UnwindImpl(
using namespace __msan;
-static inline void PrintFaultingInstruction(char *instname) {
+static inline void PrintFaultingInstructionIfRequested(char *instname) {
if (__msan::flags()->print_faulting_instruction) {
Printf("Instruction that failed the shadow check: %s\n", instname);
Printf("\n");
@@ -373,7 +373,7 @@ static inline void WarnIfPrintFaultingInstructionRequested() {
GET_CALLER_PC_BP; \
if (UNLIKELY(s)) { \
if (instname) \
- PrintFaultingInstruction(instname); \
+ PrintFaultingInstructionIfRequested(instname); \
PrintWarningWithOrigin(pc, bp, o); \
if (__msan::flags()->halt_on_error) { \
Printf("Exiting\n"); \
@@ -446,7 +446,7 @@ void __msan_warning() {
}
void __msan_warning_instname(char *instname) {
- PrintFaultingInstruction(instname);
+ PrintFaultingInstructionIfRequested(instname);
__MSAN_WARNING_BODY
}
@@ -464,7 +464,7 @@ void __msan_warning_noreturn() {
}
void __msan_warning_noreturn_instname(char *instname) {
- PrintFaultingInstruction(instname);
+ PrintFaultingInstructionIfRequested(instname);
__MSAN_WARNING_NORETURN_BODY
}
@@ -484,7 +484,7 @@ void __msan_warning_with_origin(u32 origin) {
}
void __msan_warning_with_origin_instname(u32 origin, char *instname) {
- PrintFaultingInstruction(instname);
+ PrintFaultingInstructionIfRequested(instname);
__MSAN_WARNING_WITH_ORIGIN_BODY(origin)
}
@@ -502,7 +502,7 @@ void __msan_warning_with_origin_noreturn(u32 origin) {
}
void __msan_warning_with_origin_noreturn_instname(u32 origin, char *instname) {
- PrintFaultingInstruction(instname);
+ PrintFaultingInstructionIfRequested(instname);
__MSAN_WARNING_WITH_ORIGIN_NORETURN_BODY(origin)
}
>From 973c7c29beecd8b4f1357cfdb45b956cf44864e6 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Tue, 22 Apr 2025 23:58:57 +0000
Subject: [PATCH 25/26] Fix function declaration
---
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 48baec3116501..2784c844d40f9 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -286,7 +286,7 @@ static cl::opt<MSanEmbedFaultingInstructionMode> ClEmbedFaultingInst(
clEnumValN(
MSanEmbedFaultingInstructionMode::Full, "full",
"Embed the complete LLVM IR instruction (including operands).")),
- cl::Hidden, cl::init(MSanEmbedFaultingInstructionMode::None));
+ cl::Hidden, cl::init(MSanEmbedFaultingInstructionMode::Name));
static cl::opt<bool>
ClHandleICmpExact("msan-handle-icmp-exact",
>From 99076180628e5f95b59a259799c06d25b076c196 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Wed, 23 Apr 2025 01:55:20 +0000
Subject: [PATCH 26/26] Smaller SmallVectors
---
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 2784c844d40f9..451d1588849bd 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -286,7 +286,7 @@ static cl::opt<MSanEmbedFaultingInstructionMode> ClEmbedFaultingInst(
clEnumValN(
MSanEmbedFaultingInstructionMode::Full, "full",
"Embed the complete LLVM IR instruction (including operands).")),
- cl::Hidden, cl::init(MSanEmbedFaultingInstructionMode::Name));
+ cl::Hidden, cl::init(MSanEmbedFaultingInstructionMode::None));
static cl::opt<bool>
ClHandleICmpExact("msan-handle-icmp-exact",
@@ -929,7 +929,7 @@ void MemorySanitizer::createUserspaceApi(Module &M,
StringRef WarningFnName = getWarningFnName(
TrackOrigins, Recover,
ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None);
- SmallVector<Type *, 4> ArgsTy = {};
+ SmallVector<Type *, 2> ArgsTy = {};
if (TrackOrigins) {
ArgsTy.push_back(IRB.getInt32Ty());
if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None)
@@ -974,7 +974,7 @@ void MemorySanitizer::createUserspaceApi(Module &M,
AccessSizeIndex++) {
unsigned AccessSize = 1 << AccessSizeIndex;
std::string FunctionName = "__msan_maybe_warning_" + itostr(AccessSize);
- SmallVector<Type *, 4> ArgsTy = {IRB.getIntNTy(AccessSize * 8),
+ SmallVector<Type *, 3> ArgsTy = {IRB.getIntNTy(AccessSize * 8),
IRB.getInt32Ty()};
if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
FunctionName = "__msan_maybe_warning_instname_" + itostr(AccessSize);
@@ -1500,7 +1500,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
Value *ConvertedShadow2 =
IRB.CreateZExt(ConvertedShadow, IRB.getIntNTy(8 * (1 << SizeIndex)));
- SmallVector<Value *, 4> Args = {
+ SmallVector<Value *, 3> Args = {
ConvertedShadow2,
MS.TrackOrigins && Origin ? Origin : (Value *)IRB.getInt32(0)};
if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None)
More information about the llvm-commits
mailing list