[ASan][MSan] Remove EmptyAsm and set the CallInst to nomerge to avoid from merging.

[zequanwu]

Summary

nomerge attribute was added at D78659. So, we can remove the EmptyAsm workaround in ASan the MSan and use this attribute.

[zequanwu]

Update with SanitizerCoverage.

[vitalybuka]

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

1082–1083

HasNonEmptyInlineAsm -> HasInlineAsm

[zequanwu]

address comments.

[uweigand]

It looks like this broke the s390x build bots, since then I'm seeing a lot of ASan and MSan errors:
http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33485

[zequanwu]

In D82322#2119815, @uweigand wrote:

It looks like this broke the s390x build bots, since then I'm seeing a lot of ASan and MSan errors:
http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33485

It seems like all those tests failed because the second CHECK should match output line 4 but missing the line number at the end.
Since the patch passed tests in all other build bots, I wonder if it's because the build bot config is flaky.

[uweigand]

In D82322#2120497, @zequanwu wrote:

In D82322#2119815, @uweigand wrote:

It looks like this broke the s390x build bots, since then I'm seeing a lot of ASan and MSan errors:
http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33485

It seems like all those tests failed because the second CHECK should match output line 4 but missing the line number at the end.
Since the patch passed tests in all other build bots, I wonder if it's because the build bot config is flaky.

@iii had a look and it appears the problem is that this patch https://reviews.llvm.org/D79537 didn't add support on SystemZ, so we get wrong line numbers when merging happens. Ilya is working on a patch.

[zequanwu]

In D82322#2120525, @uweigand wrote:

In D82322#2120497, @zequanwu wrote:

In D82322#2119815, @uweigand wrote:

It looks like this broke the s390x build bots, since then I'm seeing a lot of ASan and MSan errors:
http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33485

It seems like all those tests failed because the second CHECK should match output line 4 but missing the line number at the end.
Since the patch passed tests in all other build bots, I wonder if it's because the build bot config is flaky.

@iii had a look and it appears the problem is that this patch https://reviews.llvm.org/D79537 didn't add support on SystemZ, so we get wrong line numbers when merging happens. Ilya is working on a patch.

Thanks for the fix

llvm/include/llvm/IR/InstrTypes.h

public:
   /// Determine if the invoke cannot be duplicated.
   bool cannotDuplicate() const { return hasFnAttr(Attribute::NoDuplicate); }
   void setCannotDuplicate() {
     addAttribute(AttributeList::FunctionIndex, Attribute::NoDuplicate);
   }
 
   /// Determine if the call cannot be tail merged.
   bool cannotMerge() const { return hasFnAttr(Attribute::NoMerge); }
+  void setCannotMerge() {
+    addAttribute(AttributeList::FunctionIndex, Attribute::NoMerge);
+  }
 
   /// Determine if the invoke is convergent
   bool isConvergent() const { return hasFnAttr(Attribute::Convergent); }
   void setConvergent() {
     addAttribute(AttributeList::FunctionIndex, Attribute::Convergent);
   }
   void setNotConvergent() {
     removeAttribute(AttributeList::FunctionIndex, Attribute::Convergent);

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

private:
   FunctionCallee AsanErrorCallback[2][2][kNumberOfAccessSizes];
   FunctionCallee AsanMemoryAccessCallback[2][2][kNumberOfAccessSizes];
 
   // These arrays is indexed by AccessIsWrite and Experiment.
   FunctionCallee AsanErrorCallbackSized[2][2];
   FunctionCallee AsanMemoryAccessCallbackSized[2][2];
 
   FunctionCallee AsanMemmove, AsanMemcpy, AsanMemset;
-  InlineAsm *EmptyAsm;
   Value *LocalDynamicShadow = nullptr;
   const GlobalsMetadata &GlobalsMD;
   DenseMap<const AllocaInst *, bool> ProcessedAllocas;
 };
 
 class AddressSanitizerLegacyPass : public FunctionPass {
 public:
   static char ID;
struct FunctionStackPoisoner : public InstVisitor<FunctionStackPoisoner> {
   IntrinsicInst *LocalEscapeCall = nullptr;
 
   // Maps Value to an AllocaInst from which the Value is originated.
   using AllocaForValueMapTy = DenseMap<Value *, AllocaInst *>;
   AllocaForValueMapTy AllocaForValue;
 
   bool HasNonEmptyInlineAsm = false;
   bool HasReturnsTwiceCall = false;
-  std::unique_ptr<CallInst> EmptyInlineAsm;
 
   FunctionStackPoisoner(Function &F, AddressSanitizer &ASan)
       : F(F), ASan(ASan), DIB(*F.getParent(), /*AllowUnresolved*/ false),
         C(ASan.C), IntptrTy(ASan.IntptrTy),
         IntptrPtrTy(PointerType::get(IntptrTy, 0)), Mapping(ASan.Mapping),
-        StackAlignment(1 << Mapping.Scale),
-        EmptyInlineAsm(CallInst::Create(ASan.EmptyAsm)) {}
+        StackAlignment(1 << Mapping.Scale) {}
 
   bool runOnFunction() {
     if (!ClStack) return false;
 
     if (ClRedzoneByvalArgs)
       copyArgsPassedByValToAllocas();
 
     // Collect alloca, ret, lifetime instructions etc.
void visitIntrinsicInst(IntrinsicInst &II) {
     if (AI->isStaticAlloca())
       StaticAllocaPoisonCallVec.push_back(APC);
     else if (ClInstrumentDynamicAllocas)
       DynamicAllocaPoisonCallVec.push_back(APC);
   }
 
   void visitCallBase(CallBase &CB) {
     if (CallInst *CI = dyn_cast<CallInst>(&CB)) {
-      HasNonEmptyInlineAsm |= CI->isInlineAsm() &&
-                              !CI->isIdenticalTo(EmptyInlineAsm.get()) &&
-                              &CB != ASan.LocalDynamicShadow;
+      HasNonEmptyInlineAsm |=
+          CI->isInlineAsm() && &CB != ASan.LocalDynamicShadow;
       HasReturnsTwiceCall |= CI->canReturnTwice();
     }
   }
 
   // ---------------------- Helpers.
   void initializeCallbacks(Module &M);
 
   // Copies bytes from ShadowBytes into shadow memory for indexes where
if (SizeArgument) {
     if (Exp == 0)
       Call =
           IRB.CreateCall(AsanErrorCallback[IsWrite][0][AccessSizeIndex], Addr);
     else
       Call = IRB.CreateCall(AsanErrorCallback[IsWrite][1][AccessSizeIndex],
                             {Addr, ExpVal});
   }
 
-  // We don't do Call->setDoesNotReturn() because the BB already has
-  // UnreachableInst at the end.
-  // This EmptyAsm is required to avoid callback merge.
-  IRB.CreateCall(EmptyAsm, {});
+  Call->setCannotMerge();
   return Call;
 }
 
 Value *AddressSanitizer::createSlowPathCmp(IRBuilder<> &IRB, Value *AddrLong,
                                            Value *ShadowValue,
                                            uint32_t TypeSize) {
   size_t Granularity = static_cast<size_t>(1) << Mapping.Scale;
   // Addr & (Granularity - 1)
void AddressSanitizer::initializeCallbacks(Module &M) {
 
   AsanHandleNoReturnFunc =
       M.getOrInsertFunction(kAsanHandleNoReturnName, IRB.getVoidTy());
 
   AsanPtrCmpFunction =
       M.getOrInsertFunction(kAsanPtrCmp, IRB.getVoidTy(), IntptrTy, IntptrTy);
   AsanPtrSubFunction =
       M.getOrInsertFunction(kAsanPtrSub, IRB.getVoidTy(), IntptrTy, IntptrTy);
-  // We insert an empty inline asm after __asan_report* to avoid callback merge.
-  EmptyAsm = InlineAsm::get(FunctionType::get(IRB.getVoidTy(), false),
-                            StringRef(""), StringRef(""),
-                            /*hasSideEffects=*/true);
   if (Mapping.InGlobal)
     AsanShadowGlobal = M.getOrInsertGlobal("__asan_shadow",
                                            ArrayType::get(IRB.getInt8Ty(), 0));
 }
 
 bool AddressSanitizer::maybeInsertAsanInitAtFunctionEntry(Function &F) {
   // For each NSObject descendant having a +load method, this method is invoked
   // by the ObjC runtime before any of the static constructors is called.

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

private:
   /// Custom memory map parameters used when -msan-shadow-base or
   // -msan-origin-base is provided.
   MemoryMapParams CustomMapParams;
 
   MDNode *ColdCallWeights;
 
   /// Branch weights for origin store.
   MDNode *OriginStoreWeights;
-
-  /// An empty volatile inline asm that prevents callback merge.
-  InlineAsm *EmptyAsm;
 };
 
 void insertModuleCtor(Module &M) {
   getOrCreateSanitizerCtorAndInitFunctions(
       M, kMsanModuleCtorName, kMsanInitName,
       /*InitArgTypes=*/{},
       /*InitArgs=*/{},
       // This callback is invoked when the functions are created the first
MemmoveFn = M.getOrInsertFunction(
     "__msan_memmove", IRB.getInt8PtrTy(), IRB.getInt8PtrTy(),
     IRB.getInt8PtrTy(), IntptrTy);
   MemcpyFn = M.getOrInsertFunction(
     "__msan_memcpy", IRB.getInt8PtrTy(), IRB.getInt8PtrTy(), IRB.getInt8PtrTy(),
     IntptrTy);
   MemsetFn = M.getOrInsertFunction(
     "__msan_memset", IRB.getInt8PtrTy(), IRB.getInt8PtrTy(), IRB.getInt32Ty(),
     IntptrTy);
-  // We insert an empty inline asm after __msan_report* to avoid callback merge.
-  EmptyAsm = InlineAsm::get(FunctionType::get(IRB.getVoidTy(), false),
-                            StringRef(""), StringRef(""),
-                            /*hasSideEffects=*/true);
 
   MsanInstrumentAsmStoreFn =
       M.getOrInsertFunction("__msan_instrument_asm_store", IRB.getVoidTy(),
                             PointerType::get(IRB.getInt8Ty(), 0), IntptrTy);
 
   if (CompileKernel) {
     createKernelApi(M);
   } else {
void materializeStores(bool InstrumentWithCalls) {
     }
   }
 
   /// Helper function to insert a warning at IRB's current insert point.
   void insertWarningFn(IRBuilder<> &IRB, Value *Origin) {
     if (!Origin)
       Origin = (Value *)IRB.getInt32(0);
     assert(Origin->getType()->isIntegerTy());
-    IRB.CreateCall(MS.WarningFn, Origin);
-    IRB.CreateCall(MS.EmptyAsm, {});
+    IRB.CreateCall(MS.WarningFn, Origin)->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(Instruction *OrigIns, Value *Shadow, Value *Origin,
                            bool AsCall) {
     IRBuilder<> IRB(OrigIns);

llvm/test/Instrumentation/MemorySanitizer/msan_basic.ll

 
 ; CHECK-LABEL: @LoadAndCmp
 ; CHECK: %0 = load i32,
 ; CHECK: = load
 ; CHECK-ORIGINS: %[[ORIGIN:.*]] = load
 ; CHECK: call void @__msan_warning_with_origin_noreturn(i32
 ; CHECK-ORIGINS-SAME %[[ORIGIN]])
 ; CHECK-CONT: 
-; CHECK-NEXT: call void asm sideeffect
 ; CHECK-NEXT: unreachable
 ; CHECK: br i1 %tobool
 ; CHECK: ret void
 
 ; Check that we store the shadow for the retval.
 define i32 @ReturnInt() nounwind uwtable readnone sanitize_memory {
 entry:
   ret i32 123