[PATCH] D87777: [ASAN] Properly deal with musttail calls in ASAN

[Xun Li via Phabricator via llvm-commits]

lxfind created this revision.
lxfind added reviewers: wenlei, kubamracek, kcc.
Herald added subscribers: llvm-commits, modimo, hiraditya.
Herald added a project: LLVM.
lxfind requested review of this revision.

When address sanitizing a function, stack unpinsoning code is inserted before each ret instruction. However if the ret instruciton is preceded by a musttail call, such transformation broke the musttail call contract and generates invalid IR.
This patch fixes the issue by moving the insertion point prior to the musttail call if there is one.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87777

Files:
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp


Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===================================================================
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -556,6 +556,21 @@
   }
 }
 
+// For a ret instruction followed by a musttail call, we cannot insert anything
+// in between and have to return the new insertion point as the call inst.
+static Instruction *adjustForMusttailCall(Instruction *I) {
+  ReturnInst *RI = dyn_cast<ReturnInst>(I);
+  if (!RI)
+    return I;
+  Instruction *Prev = RI->getPrevNode();
+  if (BitCastInst *BCI = dyn_cast_or_null<BitCastInst>(Prev))
+    Prev = BCI->getPrevNode();
+  if (CallInst *CI = dyn_cast_or_null<CallInst>(Prev))
+    if (CI->isMustTailCall())
+      return CI;
+  return RI;
+}
+
 namespace {
 
 /// Module analysis for getting various metadata about the module.
@@ -999,10 +1014,11 @@
 
   // Unpoison dynamic allocas redzones.
   void unpoisonDynamicAllocas() {
-    for (auto &Ret : RetVec)
-      unpoisonDynamicAllocasBeforeInst(Ret, DynamicAllocaLayout);
+    for (Instruction *Ret : RetVec)
+      unpoisonDynamicAllocasBeforeInst(adjustForMusttailCall(Ret),
+                                       DynamicAllocaLayout);
 
-    for (auto &StackRestoreInst : StackRestoreVec)
+    for (Instruction *StackRestoreInst : StackRestoreVec)
       unpoisonDynamicAllocasBeforeInst(StackRestoreInst,
                                        StackRestoreInst->getOperand(0));
   }
@@ -3303,7 +3319,8 @@
   SmallVector<uint8_t, 64> ShadowAfterReturn;
 
   // (Un)poison the stack before all ret instructions.
-  for (auto Ret : RetVec) {
+  for (Instruction *Ret : RetVec) {
+    Ret = adjustForMusttailCall(Ret);
     IRBuilder<> IRBRet(Ret);
     // Mark the current frame as retired.
     IRBRet.CreateStore(ConstantInt::get(IntptrTy, kRetiredStackFrameMagic),


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D87777.292264.patch
Type: text/x-patch
Size: 1916 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200916/f8b75af9/attachment.bin>