[KMSAN] Enable -msan-handle-asm-conservative by default

[glider]

Summary

This change enables conservative assembly instrumentation in KMSAN builds by default.
It's still possible to disable it with -msan-handle-asm-conservative=0 if something breaks.
It's now impossible to enable conservative instrumentation for userspace builds, but it's not used anyway.

[glider]

Updated the tests. Those were written with userspace instrumentation in mind, which isn't the case anymore.

[dvyukov]

As per offline discussion: none of these tests test something that was used in user-space. So it does not make sense forking them for userspace/kernel.

lib/Transforms/Instrumentation/MemorySanitizer.cpp

 
 // When compiling the Linux kernel, we sometimes see false positives related to
 // MSan being unable to understand that inline assembly calls may initialize
 // local variables.
 // This flag makes the compiler conservatively unpoison every memory location
 // passed into an assembly call. Note that this may cause false positives.
 // Because it's impossible to figure out the array sizes, we can only unpoison
 // the first sizeof(type) bytes for each type* pointer.
+// The instrumentation is only enabled in KMSAN builds, and only if
+// -msan-handle-asm-conservative is on. This is done because we may want to
+// quickly disable assembly instrumentation when it breaks.
 static cl::opt<bool> ClHandleAsmConservative(
     "msan-handle-asm-conservative",
     cl::desc("conservative handling of inline assembly"), cl::Hidden,
-    cl::init(false));
+    cl::init(true));
 
 // This flag controls whether we check the shadow of the address
 // operand of load or store. Such bugs are very rare, since load from
 // a garbage address typically results in SEGV, but still happen
 // (e.g. only lower bits of address are garbage, or the access happens
 // early at program startup where malloc-ed memory is more likely to
 // be zeroed. As of 2012-08-28 this flag adds 20% slowdown.
 static cl::opt<bool> ClCheckAccessAddress("msan-check-access-address",
void visitCallSite(CallSite CS) {
     assert((CS.isCall() || CS.isInvoke()) && "Unknown type of CallSite");
     if (CS.isCall()) {
       CallInst *Call = cast<CallInst>(&I);
 
       // For inline asm, do the usual thing: check argument shadow and mark all
       // outputs as clean. Note that any side effects of the inline asm that are
       // not immediately visible in its constraints are not handled.
       if (Call->isInlineAsm()) {
-        if (ClHandleAsmConservative)
+        if (ClHandleAsmConservative && MS.CompileKernel)
           visitAsmInstruction(I);
         else
           visitInstruction(I);
         return;
       }
 
       assert(!isa<IntrinsicInst>(&I) && "intrinsics are handled elsewhere");
 

test/Instrumentation/MemorySanitizer/msan_asm_conservative.ll

 ; Test for handling of asm constraints in MSan instrumentation.
-; RUN: opt < %s -msan -msan-check-access-address=0 -msan-handle-asm-conservative=0 -S | FileCheck -check-prefixes=CHECK,CHECK-NONCONS %s
-; RUN: opt < %s -msan -msan-check-access-address=0 -msan-handle-asm-conservative=1 -S | FileCheck -check-prefixes=CHECK,CHECK-CONS %s
+; RUN: opt < %s -msan -msan-kernel=1 -msan-check-access-address=0 -msan-handle-asm-conservative=0 -S | FileCheck -check-prefixes=CHECK,CHECK-NONCONS %s
+; RUN: opt < %s -msan -msan-kernel=1 -msan-check-access-address=0 -msan-handle-asm-conservative=1 -S | FileCheck -check-prefixes=CHECK,CHECK-CONS %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 %struct.pair = type { i32, i32 }
 
 @id1 = common dso_local global i32 0, align 4
 @is1 = common dso_local global i32 0, align 4
entry:
   %0 = load i32, i32* @is1, align 4
   %1 = call i32 asm "", "=r,r,~{dirflag},~{fpsr},~{flags}"(i32 %0)
   store i32 %1, i32* @id1, align 4
   ret void
 }
 
 ; CHECK-LABEL: @f_1i_1o_reg
 ; CHECK: [[IS1_F1:%.*]] = load i32, i32* @is1, align 4
-; CHECK: call void @__msan_warning_noreturn()
+; CHECK: call void @__msan_warning
 ; CHECK: call i32 asm "",{{.*}}(i32 [[IS1_F1]])
-; CHECK: store i32 0,{{.*}}ptrtoint (i32* @id1 to i64)
+; CHECK: [[PACK1_F1:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id1{{.*}})
+; CHECK: [[EXT1_F1:%.*]] = extractvalue { i8*, i32* } [[PACK1_F1]], 0
+; CHECK: [[CAST1_F1:%.*]] = bitcast i8* [[EXT1_F1]] to i32*
+; CHECK: store i32 0, i32* [[CAST1_F1]]
 
 
 ; Two input registers, two output registers:
 ;   asm("" : "=r" (id1), "=r" (id2) : "r" (is1), "r"(is2));
 define dso_local void @f_2i_2o_reg() sanitize_memory {
 entry:
   %0 = load i32, i32* @is1, align 4
   %1 = load i32, i32* @is2, align 4
   %2 = call { i32, i32 } asm "", "=r,=r,r,r,~{dirflag},~{fpsr},~{flags}"(i32 %0, i32 %1)
   %asmresult = extractvalue { i32, i32 } %2, 0
   %asmresult1 = extractvalue { i32, i32 } %2, 1
   store i32 %asmresult, i32* @id1, align 4
   store i32 %asmresult1, i32* @id2, align 4
   ret void
 }
 
 ; CHECK-LABEL: @f_2i_2o_reg
 ; CHECK: [[IS1_F2:%.*]] = load i32, i32* @is1, align 4
 ; CHECK: [[IS2_F2:%.*]] = load i32, i32* @is2, align 4
-; CHECK: call void @__msan_warning_noreturn()
-; CHECK: call void @__msan_warning_noreturn()
+; CHECK: call void @__msan_warning
+; CHECK: call void @__msan_warning
 ; CHECK: call { i32, i32 } asm "",{{.*}}(i32 [[IS1_F2]], i32 [[IS2_F2]])
-; CHECK: store i32 0,{{.*}}ptrtoint (i32* @id1 to i64)
-; CHECK: store i32 0,{{.*}}ptrtoint (i32* @id2 to i64)
+; CHECK: [[PACK1_F2:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id1{{.*}})
+; CHECK: [[EXT1_F2:%.*]] = extractvalue { i8*, i32* } [[PACK1_F2]], 0
+; CHECK: [[CAST1_F2:%.*]] = bitcast i8* [[EXT1_F2]] to i32*
+; CHECK: store i32 0, i32* [[CAST1_F2]]
+; CHECK: [[PACK2_F2:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id2{{.*}})
+; CHECK: [[EXT2_F2:%.*]] = extractvalue { i8*, i32* } [[PACK2_F2]], 0
+; CHECK: [[CAST2_F2:%.*]] = bitcast i8* [[EXT2_F2]] to i32*
+; CHECK: store i32 0, i32* [[CAST2_F2]]
 
 ; Input same as output, used twice:
 ;   asm("" : "=r" (id1), "=r" (id2) : "r" (id1), "r" (id2));
 define dso_local void @f_2i_2o_reuse2_reg() sanitize_memory {
 entry:
   %0 = load i32, i32* @id1, align 4
   %1 = load i32, i32* @id2, align 4
   %2 = call { i32, i32 } asm "", "=r,=r,r,r,~{dirflag},~{fpsr},~{flags}"(i32 %0, i32 %1)
   %asmresult = extractvalue { i32, i32 } %2, 0
   %asmresult1 = extractvalue { i32, i32 } %2, 1
   store i32 %asmresult, i32* @id1, align 4
   store i32 %asmresult1, i32* @id2, align 4
   ret void
 }
 
 ; CHECK-LABEL: @f_2i_2o_reuse2_reg
 ; CHECK: [[ID1_F3:%.*]] = load i32, i32* @id1, align 4
 ; CHECK: [[ID2_F3:%.*]] = load i32, i32* @id2, align 4
-; CHECK: call void @__msan_warning_noreturn()
-; CHECK: call void @__msan_warning_noreturn()
+; CHECK: call void @__msan_warning
+; CHECK: call void @__msan_warning
 ; CHECK: call { i32, i32 } asm "",{{.*}}(i32 [[ID1_F3]], i32 [[ID2_F3]])
-; CHECK: store i32 0,{{.*}}ptrtoint (i32* @id1 to i64)
-; CHECK: store i32 0,{{.*}}ptrtoint (i32* @id2 to i64)
+; CHECK: [[PACK1_F3:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id1{{.*}})
+; CHECK: [[EXT1_F3:%.*]] = extractvalue { i8*, i32* } [[PACK1_F3]], 0
+; CHECK: [[CAST1_F3:%.*]] = bitcast i8* [[EXT1_F3]] to i32*
+; CHECK: store i32 0, i32* [[CAST1_F3]]
+; CHECK: [[PACK2_F3:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id2{{.*}})
+; CHECK: [[EXT2_F3:%.*]] = extractvalue { i8*, i32* } [[PACK2_F3]], 0
+; CHECK: [[CAST2_F3:%.*]] = bitcast i8* [[EXT2_F3]] to i32*
+; CHECK: store i32 0, i32* [[CAST2_F3]]
 
 
 ; One of the input registers is also an output:
 ;   asm("" : "=r" (id1), "=r" (id2) : "r" (id1), "r"(is1));
 define dso_local void @f_2i_2o_reuse1_reg() sanitize_memory {
 entry:
   %0 = load i32, i32* @id1, align 4
   %1 = load i32, i32* @is1, align 4
   %2 = call { i32, i32 } asm "", "=r,=r,r,r,~{dirflag},~{fpsr},~{flags}"(i32 %0, i32 %1)
   %asmresult = extractvalue { i32, i32 } %2, 0
   %asmresult1 = extractvalue { i32, i32 } %2, 1
   store i32 %asmresult, i32* @id1, align 4
   store i32 %asmresult1, i32* @id2, align 4
   ret void
 }
 
 ; CHECK-LABEL: @f_2i_2o_reuse1_reg
 ; CHECK: [[ID1_F4:%.*]] = load i32, i32* @id1, align 4
 ; CHECK: [[IS1_F4:%.*]] = load i32, i32* @is1, align 4
-; CHECK: call void @__msan_warning_noreturn()
-; CHECK: call void @__msan_warning_noreturn()
+; CHECK: call void @__msan_warning
+; CHECK: call void @__msan_warning
 ; CHECK: call { i32, i32 } asm "",{{.*}}(i32 [[ID1_F4]], i32 [[IS1_F4]])
-; CHECK: store i32 0,{{.*}}ptrtoint (i32* @id1 to i64)
-; CHECK: store i32 0,{{.*}}ptrtoint (i32* @id2 to i64)
+; CHECK: [[PACK1_F4:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id1{{.*}})
+; CHECK: [[EXT1_F4:%.*]] = extractvalue { i8*, i32* } [[PACK1_F4]], 0
+; CHECK: [[CAST1_F4:%.*]] = bitcast i8* [[EXT1_F4]] to i32*
+; CHECK: store i32 0, i32* [[CAST1_F4]]
+; CHECK: [[PACK2_F4:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id2{{.*}})
+; CHECK: [[EXT2_F4:%.*]] = extractvalue { i8*, i32* } [[PACK2_F4]], 0
+; CHECK: [[CAST2_F4:%.*]] = bitcast i8* [[EXT2_F4]] to i32*
+; CHECK: store i32 0, i32* [[CAST2_F4]]
 
 
 ; One input register, three output registers:
 ;   asm("" : "=r" (id1), "=r" (id2), "=r" (id3) : "r" (is1));
 define dso_local void @f_1i_3o_reg() sanitize_memory {
 entry:
   %0 = load i32, i32* @is1, align 4
   %1 = call { i32, i32, i32 } asm "", "=r,=r,=r,r,~{dirflag},~{fpsr},~{flags}"(i32 %0)
   %asmresult = extractvalue { i32, i32, i32 } %1, 0
   %asmresult1 = extractvalue { i32, i32, i32 } %1, 1
   %asmresult2 = extractvalue { i32, i32, i32 } %1, 2
   store i32 %asmresult, i32* @id1, align 4
   store i32 %asmresult1, i32* @id2, align 4
   store i32 %asmresult2, i32* @id3, align 4
   ret void
 }
 
 ; CHECK-LABEL: @f_1i_3o_reg
 ; CHECK: [[IS1_F5:%.*]] = load i32, i32* @is1, align 4
-; CHECK: call void @__msan_warning_noreturn()
+; CHECK: call void @__msan_warning
 ; CHECK: call { i32, i32, i32 } asm "",{{.*}}(i32 [[IS1_F5]])
-; CHECK: store i32 0,{{.*}}ptrtoint (i32* @id1 to i64)
-; CHECK: store i32 0,{{.*}}ptrtoint (i32* @id2 to i64)
-; CHECK: store i32 0,{{.*}}ptrtoint (i32* @id3 to i64)
+; CHECK: [[PACK1_F5:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id1{{.*}})
+; CHECK: [[EXT1_F5:%.*]] = extractvalue { i8*, i32* } [[PACK1_F5]], 0
+; CHECK: [[CAST1_F5:%.*]] = bitcast i8* [[EXT1_F5]] to i32*
+; CHECK: store i32 0, i32* [[CAST1_F5]]
+; CHECK: [[PACK2_F5:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id2{{.*}})
+; CHECK: [[EXT2_F5:%.*]] = extractvalue { i8*, i32* } [[PACK2_F5]], 0
+; CHECK: [[CAST2_F5:%.*]] = bitcast i8* [[EXT2_F5]] to i32*
+; CHECK: store i32 0, i32* [[CAST2_F5]]
+; CHECK: [[PACK3_F5:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id3{{.*}})
+; CHECK: [[EXT3_F5:%.*]] = extractvalue { i8*, i32* } [[PACK3_F5]], 0
+; CHECK: [[CAST3_F5:%.*]] = bitcast i8* [[EXT3_F5]] to i32*
+; CHECK: store i32 0, i32* [[CAST3_F5]]
 
 
 ; 2 input memory args, 2 output memory args:
 ;  asm("" : "=m" (id1), "=m" (id2) : "m" (is1), "m"(is2))
 define dso_local void @f_2i_2o_mem() sanitize_memory {
 entry:
   call void asm "", "=*m,=*m,*m,*m,~{dirflag},~{fpsr},~{flags}"(i32* @id1, i32* @id2, i32* @is1, i32* @is2)
   ret void
entry:
   store i32 %1, i32* @id1, align 4
   ret void
 }
 
 ; 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_noreturn()
+; CHECK: call void @__msan_warning
 ; CHECK: call i32 asm "", "=r,=*m,r,*m,~{dirflag},~{fpsr},~{flags}"(i32* @id1, i32 [[IS1_F7]], i32* @is1)
 
 
 ; Three outputs, first and last returned via regs, second via mem:
 ;  asm("" : "=r" (id1), "=m"(id2), "=r" (id3):);
 define dso_local void @f_3o_reg_mem_reg() sanitize_memory {
 entry:
   %0 = call { i32, i32 } asm "", "=r,=*m,=r,~{dirflag},~{fpsr},~{flags}"(i32* @id2)
entry:
   ret void
 }
 
 ; CHECK-LABEL: @f_3i_3o_complex_reg
 ; CHECK: [[PAIR1_F9:%.*]] = load {{.*}} @pair1
 ; CHECK: [[C1_F9:%.*]] = load {{.*}} @c1
 ; CHECK: [[MEMCPY_S1_F9:%.*]] = load {{.*}} @memcpy_s1
 ; CHECK-CONS: call void @__msan_instrument_asm_store({{.*}}@pair2{{.*}}, i64 8)
-; CHECK: call void @__msan_warning_noreturn()
-; CHECK: call void @__msan_warning_noreturn()
-; CHECK: call void @__msan_warning_noreturn()
+; CHECK: call void @__msan_warning
+; CHECK: call void @__msan_warning
+; CHECK: call void @__msan_warning
 ; CHECK: call { i8, i8* (i8*, i8*, i32)* } asm "", "=*r,=r,=r,r,r,r,~{dirflag},~{fpsr},~{flags}"(%struct.pair* @pair2, {{.*}}[[PAIR1_F9]], i8 [[C1_F9]], {{.*}} [[MEMCPY_S1_F9]])
 
 ; Three inputs and three outputs of different types: a pair, a char, a function pointer.
 ; Everything is passed in memory:
 ;  asm("" : "=m" (pair2), "=m" (c2), "=m" (memcpy_d1) : "m"(pair1), "m"(c1), "m"(memcpy_s1));
 define dso_local void @f_3i_3o_complex_mem() sanitize_memory {
 entry:
   call void asm "", "=*m,=*m,=*m,*m,*m,*m,~{dirflag},~{fpsr},~{flags}"(%struct.pair* @pair2, i8* @c2, i8* (i8*, i8*, i32)** @memcpy_d1, %struct.pair* @pair1, i8* @c1, i8* (i8*, i8*, i32)** @memcpy_s1)

test/Instrumentation/MemorySanitizer/msan_x86_bts_asm.ll

 ; Test for the conservative assembly handling mode used by KMSAN.
-; RUN: opt < %s -msan -msan-check-access-address=0 -msan-handle-asm-conservative=0 -S | FileCheck -check-prefixes=CHECK,CHECK-NONCONS %s
-; RUN: opt < %s -msan -msan-check-access-address=0 -msan-handle-asm-conservative=1 -S | FileCheck -check-prefixes=CHECK,CHECK-CONS %s
+; RUN: opt < %s -msan -msan-kernel=1 -msan-check-access-address=0 -msan-handle-asm-conservative=0 -S | FileCheck -check-prefixes=CHECK,CHECK-NONCONS %s
+; RUN: opt < %s -msan -msan-kernel=1 -msan-check-access-address=0 -msan-handle-asm-conservative=1 -S | FileCheck -check-prefixes=CHECK,CHECK-CONS %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 ; The IR below was generated from the following source:
 ;  int main() {
 ;    bool bit;
 ;    unsigned long value = 2;

 
 if.then:                                          ; preds = %entry
   ret i32 0
 
 if.else:                                          ; preds = %entry
   ret i32 1
 }
 
+; %nr is first poisoned, then unpoisoned (written to). Need to optimize this in the future.
+; CHECK: [[NRC1:%.*]] = bitcast i64* %nr to i8*
+; CHECK: call void @__msan_poison_alloca(i8* [[NRC1]]{{.*}})
+; CHECK: [[NRC2:%.*]] = bitcast i64* %nr to i8*
+; CHECK: call { i8*, i32* } @__msan_metadata_ptr_for_store_8(i8* [[NRC2]])
+
 ; Hooks for inputs usually go before the assembly statement. But here we have none,
 ; because %nr is passed by value. However we check %nr for being initialized.
-; CHECK-CONS: [[NRC:%.*]] = ptrtoint i64* %nr to i64
+; CHECK-CONS: [[NRC3:%.*]] = bitcast i64* %nr to i8*
+; CHECK-CONS: call { i8*, i32* } @__msan_metadata_ptr_for_load_8(i8* [[NRC3]])
 
 ; In the conservative mode, call the store hooks for %bit and %addr:
 ; CHECK-CONS: call void @__msan_instrument_asm_store(i8* %bit, i64 1)
 ; CHECK-CONS: [[ADDR8S:%.*]] = bitcast i64** %addr to i8*
 ; CHECK-CONS: call void @__msan_instrument_asm_store(i8* [[ADDR8S]], i64 8)
 
 ; Landing pad for the %nr check above.
-; CHECK-CONS: call void @__msan_warning_noreturn()
+; CHECK-CONS: call void @__msan_warning
 
 ; CHECK: call void asm "btsq $2, $1; setc $0"
 
 ; Calculating the shadow offset of %bit.
-; CHECK: [[PTR:%.*]] = ptrtoint {{.*}} %bit to i64
-; CHECK: [[SH_NUM:%.*]] = xor i64 [[PTR]]
-; CHECK: [[SHADOW:%.*]] = inttoptr i64 [[SH_NUM]] {{.*}}
+; CHECKz: [[PTR:%.*]] = ptrtoint {{.*}} %bit to i64
+; CHECKz: [[SH_NUM:%.*]] = xor i64 [[PTR]]
+; CHECKz: [[SHADOW:%.*]] = inttoptr i64 [[SH_NUM]] {{.*}}
+
+; CHECK: [[META:%.*]] = call {{.*}} @__msan_metadata_ptr_for_load_1(i8* %bit)
+; CHECK: [[SHADOW:%.*]] = extractvalue { i8*, i32* } [[META]], 0
 
 ; Now load the shadow value for the boolean.
 ; CHECK: [[MSLD:%.*]] = load {{.*}} [[SHADOW]]
 ; CHECK: [[MSPROP:%.*]] = trunc i8 [[MSLD]] to i1
 
 ; Is the shadow poisoned?
 ; CHECK: [[MSCMP:%.*]] = icmp ne i1 [[MSPROP]], false
 ; CHECK: br i1 [[MSCMP]], label %[[IFTRUE:.*]], label {{.*}}
 
 ; If yes, raise a warning.
 ; CHECK: <label>:[[IFTRUE]]
-; CHECK: call void @__msan_warning_noreturn()
+; CHECK: call void @__msan_warning