From 0e3b85a730bc3ef2bcbc22bff33678005df9bafa Mon Sep 17 00:00:00 2001 From: Alexander Potapenko Date: Thu, 20 Dec 2018 10:05:00 +0000 Subject: [PATCH] [MSan] Don't emit __msan_instrument_asm_load() calls LLVM treats void* pointers passed to assembly routines as pointers to sized types. We used to emit calls to __msan_instrument_asm_load() for every such void*, which sometimes led to false positives. A less error-prone (and truly "conservative") approach is to unpoison only assembly output arguments. llvm-svn: 349734 --- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp | 14 ++++---------- .../MemorySanitizer/msan_asm_conservative.ll | 6 ------ 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp index 0bbf3a9..1fad7fc 100644 --- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp @@ -99,9 +99,8 @@ /// also possible that the arguments only indicate the offset for a base taken /// from a segment register, so it's dangerous to treat any asm() arguments as /// pointers. We take a conservative approach generating calls to -/// __msan_instrument_asm_load(ptr, size) and /// __msan_instrument_asm_store(ptr, size) -/// , which defer the memory checking/unpoisoning to the runtime library. +/// , which defer the memory unpoisoning to the runtime library. /// The latter can perform more complex address checks to figure out whether /// it's safe to touch the shadow memory. /// Like with atomic operations, we call __msan_instrument_asm_store() before @@ -570,7 +569,7 @@ private: Value *MsanMetadataPtrForLoadN, *MsanMetadataPtrForStoreN; Value *MsanMetadataPtrForLoad_1_8[4]; Value *MsanMetadataPtrForStore_1_8[4]; - Value *MsanInstrumentAsmStoreFn, *MsanInstrumentAsmLoadFn; + Value *MsanInstrumentAsmStoreFn; /// Helper to choose between different MsanMetadataPtrXxx(). Value *getKmsanShadowOriginAccessFn(bool isStore, int size); @@ -779,9 +778,6 @@ void MemorySanitizer::initializeCallbacks(Module &M) { StringRef(""), StringRef(""), /*hasSideEffects=*/true); - MsanInstrumentAsmLoadFn = - M.getOrInsertFunction("__msan_instrument_asm_load", IRB.getVoidTy(), - PointerType::get(IRB.getInt8Ty(), 0), IntptrTy); MsanInstrumentAsmStoreFn = M.getOrInsertFunction("__msan_instrument_asm_store", IRB.getVoidTy(), PointerType::get(IRB.getInt8Ty(), 0), IntptrTy); @@ -3482,19 +3478,17 @@ struct MemorySanitizerVisitor : public InstVisitor { Type *OpType = Operand->getType(); // Check the operand value itself. insertShadowCheck(Operand, &I); - if (!OpType->isPointerTy()) { + if (!OpType->isPointerTy() || !isOutput) { assert(!isOutput); return; } - Value *Hook = - isOutput ? MS.MsanInstrumentAsmStoreFn : MS.MsanInstrumentAsmLoadFn; Type *ElType = OpType->getPointerElementType(); if (!ElType->isSized()) return; int Size = DL.getTypeStoreSize(ElType); Value *Ptr = IRB.CreatePointerCast(Operand, IRB.getInt8PtrTy()); Value *SizeVal = ConstantInt::get(MS.IntptrTy, Size); - IRB.CreateCall(Hook, {Ptr, SizeVal}); + IRB.CreateCall(MS.MsanInstrumentAsmStoreFn, {Ptr, SizeVal}); } /// Get the number of output arguments returned by pointers. diff --git a/llvm/test/Instrumentation/MemorySanitizer/msan_asm_conservative.ll b/llvm/test/Instrumentation/MemorySanitizer/msan_asm_conservative.ll index bc6c44c..9bcd2f7 100644 --- a/llvm/test/Instrumentation/MemorySanitizer/msan_asm_conservative.ll +++ b/llvm/test/Instrumentation/MemorySanitizer/msan_asm_conservative.ll @@ -179,8 +179,6 @@ entry: } ; CHECK-LABEL: @f_2i_2o_mem -; CHECK-CONS: call void @__msan_instrument_asm_load({{.*}}@is1{{.*}}, i64 4) -; CHECK-CONS: call void @__msan_instrument_asm_load({{.*}}@is2{{.*}}, i64 4) ; CHECK-CONS: call void @__msan_instrument_asm_store({{.*}}@id1{{.*}}, i64 4) ; CHECK-CONS: call void @__msan_instrument_asm_store({{.*}}@id2{{.*}}, i64 4) ; CHECK: call void asm "", "=*m,=*m,*m,*m,~{dirflag},~{fpsr},~{flags}"(i32* @id1, i32* @id2, i32* @is1, i32* @is2) @@ -198,7 +196,6 @@ entry: ; CHECK-LABEL: @f_1i_1o_memreg ; CHECK: [[IS1_F7:%.*]] = load i32, i32* @is1, align 4 -; CHECK-CONS: call void @__msan_instrument_asm_load({{.*}}@is1{{.*}}, i64 4) ; CHECK-CONS: call void @__msan_instrument_asm_store({{.*}}@id1{{.*}}, i64 4) ; CHECK: call void @__msan_warning ; CHECK: call i32 asm "", "=r,=*m,r,*m,~{dirflag},~{fpsr},~{flags}"(i32* @id1, i32 [[IS1_F7]], i32* @is1) @@ -257,9 +254,6 @@ entry: } ; CHECK-LABEL: @f_3i_3o_complex_mem -; CHECK-CONS: call void @__msan_instrument_asm_load({{.*}}@pair1{{.*}}, i64 8) -; CHECK-CONS: call void @__msan_instrument_asm_load(i8* @c1, i64 1) -; CHECK-CONS: call void @__msan_instrument_asm_load({{.*}}@memcpy_s1{{.*}}, i64 8) ; CHECK-CONS: call void @__msan_instrument_asm_store({{.*}}@pair2{{.*}}, i64 8) ; CHECK-CONS: call void @__msan_instrument_asm_store({{.*}}@c2{{.*}}, i64 1) ; CHECK-CONS: call void @__msan_instrument_asm_store({{.*}}@memcpy_d1{{.*}}, i64 8) -- 2.7.4