Skip to content

Commit e539a52

Browse files
thurstondthurstond
committed
Address Florian's feedback
Also add "[EXPERIMENTAL]" disclaimer
1 parent cacf9f2 commit e539a52

File tree

4 files changed

+63
-32
lines changed

4 files changed

+63
-32
lines changed

‎compiler-rt/lib/msan/msan_flags.inc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ MSAN_FLAG(bool, poison_in_malloc, true, "")
2626
MSAN_FLAG(bool, poison_in_free, true, "")
2727
MSAN_FLAG(bool, poison_in_dtor, true, "")
2828
MSAN_FLAG(bool, print_faulting_instruction, false,
29-
"When reporting a UUM, print the name of the faulting instruction. "
29+
"[EXPERIMENTAL] When reporting a UUM, print the name of the faulting "
30+
"instruction. "
3031
"Note: requires code to be instrumented with -mllvm "
3132
"-msan-embed-faulting-instruction.")
3233
MSAN_FLAG(bool, report_umrs, true, "")

‎compiler-rt/test/msan/print_faulting_inst.cpp

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,47 @@
11
// Try parameter '0' (program runs cleanly)
22
// -------------------------------------------------------
3-
// RUN: env MSAN_OPTIONS=print_faulting_instruction=true %clangxx_msan -g %s -o %t && env MSAN_OPTIONS=print_faulting_instruction=true %run %t 0
3+
// RUN: %clangxx_msan -g %s -o %t && env MSAN_OPTIONS=print_faulting_instruction=true %run %t 0
44

55
// Try parameter '1'
66
// -------------------------------------------------------
77
// RUN: %clangxx_msan -g %s -o %t && not env MSAN_OPTIONS=print_faulting_instruction=true %run %t 1 >%t.out 2>&1
88
// RUN: FileCheck --check-prefix STORE-CHECK %s < %t.out
99

10-
// 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
10+
// 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
11+
// RUN: FileCheck --check-prefix STORE-CHECK %s < %t.out
12+
13+
// 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
1114
// RUN: FileCheck --check-prefixes VERBOSE-STORE-CHECK,STORE-CHECK %s < %t.out
1215

13-
// 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
16+
// 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
1417
// RUN: FileCheck --check-prefixes VERY-VERBOSE-STORE-CHECK,STORE-CHECK %s < %t.out
1518

1619
// Try parameter '2', with -fsanitize-memory-param-retval
1720
// -------------------------------------------------------
1821
// 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
1922
// RUN: FileCheck --check-prefix PARAM-CHECK %s < %t.out
2023

21-
// 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
24+
// 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
25+
// RUN: FileCheck --check-prefix PARAM-CHECK %s < %t.out
26+
27+
// 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
2228
// RUN: FileCheck --check-prefixes VERBOSE-PARAM-CHECK,PARAM-CHECK %s < %t.out
2329

24-
// 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
30+
// 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
2531
// RUN: FileCheck --check-prefixes VERY-VERBOSE-PARAM-CHECK,PARAM-CHECK %s < %t.out
2632

2733
// Try parameter '2', with -fno-sanitize-memory-param-retval
2834
// -------------------------------------------------------
2935
// 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
3036
// RUN: FileCheck --check-prefix NO-PARAM-CHECK %s < %t.out
3137

32-
// 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
38+
// 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
39+
// RUN: FileCheck --check-prefix NO-PARAM-CHECK %s < %t.out
40+
41+
// 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
3342
// RUN: FileCheck --check-prefixes VERBOSE-NO-PARAM-CHECK,NO-PARAM-CHECK %s < %t.out
3443

35-
// 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
44+
// 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
3645
// RUN: FileCheck --check-prefixes VERY-VERBOSE-NO-PARAM-CHECK,NO-PARAM-CHECK %s < %t.out
3746

3847
#include <stdio.h>

‎llvm/include/llvm/Transforms/Instrumentation/MemorySanitizer.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,13 @@ class Module;
2121
class StringRef;
2222
class raw_ostream;
2323

24+
/// Mode of MSan -embed-faulting-instruction
25+
enum class MSanEmbedFaultingInstructionMode {
26+
None, ///< Do not embed the faulting instruction information
27+
Name, ///< Embed the LLVM IR instruction name (excluding operands)
28+
Full, ///< Embed the complete LLVM IR instruction (including operands)
29+
};
30+
2431
struct MemorySanitizerOptions {
2532
MemorySanitizerOptions() : MemorySanitizerOptions(0, false, false, false){};
2633
MemorySanitizerOptions(int TrackOrigins, bool Recover, bool Kernel)

‎llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

Lines changed: 38 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -274,13 +274,19 @@ static cl::opt<bool>
274274
cl::desc("propagate shadow through ICmpEQ and ICmpNE"),
275275
cl::Hidden, cl::init(true));
276276

277-
static cl::opt<uint> ClEmbedFaultingInst(
277+
static cl::opt<MSanEmbedFaultingInstructionMode> ClEmbedFaultingInst(
278278
"msan-embed-faulting-instruction",
279-
cl::desc("If set to 1, embed the name of the LLVM IR instruction that "
280-
"failed the shadow check."
281-
"If set to 2, embed the full LLVM IR instruction. "
282-
"The runtime can print the embedded instruction."),
283-
cl::Hidden, cl::init(0));
279+
cl::desc("[EXPERIMENTAL] Sets whether to embed the LLVM IR instruction "
280+
"info for each UUM check."),
281+
cl::values(
282+
clEnumValN(MSanEmbedFaultingInstructionMode::None, "none",
283+
"Do not embed the faulting instruction information."),
284+
clEnumValN(MSanEmbedFaultingInstructionMode::Name, "name",
285+
"Embed the LLVM IR instruction name (excluding operands)."),
286+
clEnumValN(
287+
MSanEmbedFaultingInstructionMode::Full, "full",
288+
"Embed the complete LLVM IR instruction (including operands).")),
289+
cl::Hidden, cl::init(MSanEmbedFaultingInstructionMode::None));
284290

285291
static cl::opt<bool>
286292
ClHandleICmpExact("msan-handle-icmp-exact",
@@ -824,7 +830,7 @@ void MemorySanitizer::createKernelApi(Module &M, const TargetLibraryInfo &TLI) {
824830
VAArgOriginTLS = nullptr;
825831
VAArgOverflowSizeTLS = nullptr;
826832

827-
if (ClEmbedFaultingInst)
833+
if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None)
828834
WarningFn = M.getOrInsertFunction(
829835
"__msan_warning_instname", TLI.getAttrList(C, {0}, /*Signed=*/false),
830836
IRB.getVoidTy(), IRB.getInt32Ty(), IRB.getPtrTy());
@@ -889,7 +895,7 @@ void MemorySanitizer::createUserspaceApi(Module &M,
889895
// FIXME: this function should have "Cold" calling conv,
890896
// which is not yet implemented.
891897
if (TrackOrigins) {
892-
if (ClEmbedFaultingInst) {
898+
if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
893899
StringRef WarningFnName =
894900
Recover ? "__msan_warning_with_origin_instname"
895901
: "__msan_warning_with_origin_noreturn_instname";
@@ -904,7 +910,7 @@ void MemorySanitizer::createUserspaceApi(Module &M,
904910
IRB.getVoidTy(), IRB.getInt32Ty());
905911
}
906912
} else {
907-
if (ClEmbedFaultingInst) {
913+
if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
908914
StringRef WarningFnName = Recover ? "__msan_warning_instname"
909915
: "__msan_warning_noreturn_instname";
910916
WarningFn =
@@ -946,7 +952,7 @@ void MemorySanitizer::createUserspaceApi(Module &M,
946952
AccessSizeIndex++) {
947953
unsigned AccessSize = 1 << AccessSizeIndex;
948954
std::string FunctionName;
949-
if (ClEmbedFaultingInst) {
955+
if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
950956
FunctionName = "__msan_maybe_warning_instname_" + itostr(AccessSize);
951957
MaybeWarningFn[AccessSizeIndex] = M.getOrInsertFunction(
952958
FunctionName, TLI.getAttrList(C, {0, 1}, /*Signed=*/false),
@@ -1450,7 +1456,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
14501456
}
14511457
}
14521458

1453-
if (ClEmbedFaultingInst) {
1459+
if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
14541460
if (MS.CompileKernel || MS.TrackOrigins)
14551461
IRB.CreateCall(MS.WarningFn, {Origin, InstName})->setCannotMerge();
14561462
else
@@ -1479,7 +1485,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
14791485
Value *ConvertedShadow2 =
14801486
IRB.CreateZExt(ConvertedShadow, IRB.getIntNTy(8 * (1 << SizeIndex)));
14811487
CallBase *CB;
1482-
if (ClEmbedFaultingInst)
1488+
if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None)
14831489
CB = IRB.CreateCall(
14841490
Fn, {ConvertedShadow2,
14851491
MS.TrackOrigins && Origin ? Origin : (Value *)IRB.getInt32(0),
@@ -1500,34 +1506,28 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
15001506

15011507
IRB.SetInsertPoint(CheckTerm);
15021508
// InstName will be ignored by insertWarningFn if ClEmbedFaultingInst is
1503-
// false
1509+
// MSanEmbedFaultingInstructionMode::None
15041510
insertWarningFn(IRB, Origin, InstName);
15051511
LLVM_DEBUG(dbgs() << " CHECK: " << *Cmp << "\n");
15061512
}
15071513
}
15081514

1509-
void materializeInstructionChecks(
1510-
ArrayRef<ShadowOriginAndInsertPoint> InstructionChecks) {
1511-
const DataLayout &DL = F.getDataLayout();
1512-
// Disable combining in some cases. TrackOrigins checks each shadow to pick
1513-
// correct origin.
1514-
bool Combine = !MS.TrackOrigins;
1515-
Instruction *Instruction = InstructionChecks.front().OrigIns;
1516-
1515+
Value *getInstName(Instruction *Instruction) {
15171516
Value *InstName = nullptr;
1518-
if (ClEmbedFaultingInst >= 1) {
1517+
if (ClEmbedFaultingInst != MSanEmbedFaultingInstructionMode::None) {
15191518
IRBuilder<> IRB0(Instruction);
15201519
std::string str;
15211520
StringRef InstNameStrRef;
15221521

15231522
// Dumping the full instruction is expensive because the operands etc.
15241523
// likely make the string unique per instruction instance, hence we
15251524
// offer a choice whether to only print the instruction name.
1526-
if (ClEmbedFaultingInst >= 2) {
1525+
if (ClEmbedFaultingInst == MSanEmbedFaultingInstructionMode::Full) {
15271526
llvm::raw_string_ostream buf(str);
15281527
Instruction->print(buf);
15291528
InstNameStrRef = StringRef(str);
1530-
} else if (ClEmbedFaultingInst >= 1) {
1529+
} else if (ClEmbedFaultingInst ==
1530+
MSanEmbedFaultingInstructionMode::Name) {
15311531
if (CallInst *CI = dyn_cast<CallInst>(Instruction)) {
15321532
if (CI->getCalledFunction()) {
15331533
Twine description = "call " + CI->getCalledFunction()->getName();
@@ -1544,12 +1544,26 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
15441544
InstName = IRB0.CreateGlobalString(InstNameStrRef);
15451545
}
15461546

1547+
return InstName;
1548+
}
1549+
1550+
void materializeInstructionChecks(
1551+
ArrayRef<ShadowOriginAndInsertPoint> InstructionChecks) {
1552+
const DataLayout &DL = F.getDataLayout();
1553+
// Disable combining in some cases. TrackOrigins checks each shadow to pick
1554+
// correct origin.
1555+
bool Combine = !MS.TrackOrigins;
1556+
Instruction *Instruction = InstructionChecks.front().OrigIns;
1557+
1558+
Value *InstName = getInstName(Instruction);
1559+
15471560
Value *Shadow = nullptr;
15481561
for (const auto &ShadowData : InstructionChecks) {
15491562
assert(ShadowData.OrigIns == Instruction);
15501563
IRBuilder<> IRB(Instruction);
15511564

15521565
Value *ConvertedShadow = ShadowData.Shadow;
1566+
15531567
if (auto *ConstantShadow = dyn_cast<Constant>(ConvertedShadow)) {
15541568
if (!ClCheckConstantShadow || ConstantShadow->isZeroValue()) {
15551569
// Skip, value is initialized or const shadow is ignored.

0 commit comments

Comments
 (0)