[sancov] Leave llvm.localescape in the entry block

[rnk]

Summary

Similar to the change we applied to ASan. The same test case works.

[samsonov]

*sigh* looks like anyone who splits basic blocks might stumble to this localescape problem. Probably it's worth to put this logic somewhere under lib/Transforms/Utils/BasicBlockUtils.cpp?

[rnk]

In D11961#223310, @samsonov wrote:

*sigh* looks like anyone who splits basic blocks might stumble to this localescape problem. Probably it's worth to put this logic somewhere under lib/Transforms/Utils/BasicBlockUtils.cpp?

I mean, what kind of twisted person wants to split the entry block? It's so non-canonical. ;)

I think most instrumentation passes pretty much need this utility, but no optimization pass needs it. Should I create something like lib/Transforms/Instrumentation/Utils.cpp?

[samsonov]

In D11961#223344, @rnk wrote:

In D11961#223310, @samsonov wrote:

*sigh* looks like anyone who splits basic blocks might stumble to this localescape problem. Probably it's worth to put this logic somewhere under lib/Transforms/Utils/BasicBlockUtils.cpp?

I mean, what kind of twisted person wants to split the entry block? It's so non-canonical. ;)

I think most instrumentation passes pretty much need this utility, but no optimization pass needs it. Should I create something like lib/Transforms/Instrumentation/Utils.cpp?

Sounds good.

[rnk]

• Make utility to prepare for entry block splitting

[rnk]

lib/Transforms/Instrumentation/AddressSanitizer.cpp

1805–1806

Hm, this change fails because it moves instrumented static allocas around. Coverage and other instrumentation passes that don't replace static allocas don't have this problem. I think we should leave asan alone.

[rnk]

• Revert asan changes

[rnk]

Woops, this landed accidentally through the usual git svn confusion. Let me know if it needs changes.

[samsonov]

Looks good, thank you!

include/llvm/Transforms/Instrumentation.h

 // This file defines constructor functions for instrumentation passes.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_TRANSFORMS_INSTRUMENTATION_H
 #define LLVM_TRANSFORMS_INSTRUMENTATION_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/IR/BasicBlock.h"
 #include <vector>
 
 #if defined(__GNUC__) && defined(__linux__) && !defined(ANDROID)
 inline void *getDFSanArgTLSPtrForJIT() {
   extern __thread __attribute__((tls_model("initial-exec")))
     void *__dfsan_arg_tls;
   return (void *)&__dfsan_arg_tls;
 }
 
 inline void *getDFSanRetValTLSPtrForJIT() {
   extern __thread __attribute__((tls_model("initial-exec")))
     void *__dfsan_retval_tls;
   return (void *)&__dfsan_retval_tls;
 }
 #endif
 
 namespace llvm {
 
+/// Instrumentation passes often insert conditional checks into entry blocks.
+/// Call this function before splitting the entry block to move instructions
+/// that must remain in the entry block up before the split point. Static
+/// allocas and llvm.localescape calls, for example, must remain in the entry
+/// block.
+BasicBlock::iterator PrepareToSplitEntryBlock(BasicBlock &BB,
+                                              BasicBlock::iterator IP);
+
 class ModulePass;
 class FunctionPass;
 
 // Insert GCOV profiling instrumentation
 struct GCOVOptions {
   static GCOVOptions getDefault();
 
   // Specify whether to emit .gcno files.

lib/Transforms/Instrumentation/AddressSanitizer.cpp

struct AllocaPoisonCall {
     uint64_t Size;
     bool DoPoison;
   };
   SmallVector<AllocaPoisonCall, 8> AllocaPoisonCallVec;
 
   SmallVector<AllocaInst *, 1> DynamicAllocaVec;
   SmallVector<IntrinsicInst *, 1> StackRestoreVec;
   AllocaInst *DynamicAllocaLayout = nullptr;
-  IntrinsicInst *LocalEscapeCall = nullptr;
 
   // Maps Value to an AllocaInst from which the Value is originated.
   typedef DenseMap<Value *, AllocaInst *> AllocaForValueMapTy;
   AllocaForValueMapTy AllocaForValue;
 
   bool HasNonEmptyInlineAsm = false;
   bool HasReturnsTwiceCall = false;
   std::unique_ptr<CallInst> EmptyInlineAsm;
else
       AllocaVec.push_back(&AI);
   }
 
   /// \brief Collect lifetime intrinsic calls to check for use-after-scope
   /// errors.
   void visitIntrinsicInst(IntrinsicInst &II) {
     Intrinsic::ID ID = II.getIntrinsicID();
     if (ID == Intrinsic::stackrestore) StackRestoreVec.push_back(&II);
-    if (ID == Intrinsic::localescape) LocalEscapeCall = &II;
     if (!ClCheckLifetime) return;
     if (ID != Intrinsic::lifetime_start && ID != Intrinsic::lifetime_end)
       return;
     // Found lifetime intrinsic, add ASan instrumentation if necessary.
     ConstantInt *Size = dyn_cast<ConstantInt>(II.getArgOperand(0));
     // If size argument is undefined, don't do anything.
     if (Size->isMinusOne()) return;
     // Check that size doesn't saturate uint64_t and can
void FunctionStackPoisoner::poisonStack() {
   // regular stack slots.
   auto InsBeforeB = InsBefore->getParent();
   assert(InsBeforeB == &F.getEntryBlock());
   for (BasicBlock::iterator I = InsBefore; I != InsBeforeB->end(); ++I)
     if (auto *AI = dyn_cast_or_null<AllocaInst>(I))
       if (NonInstrumentedStaticAllocaVec.count(AI) > 0)
         AI->moveBefore(InsBefore);
 
-  // If we have a call to llvm.localescape, keep it in the entry block.
-  if (LocalEscapeCall) LocalEscapeCall->moveBefore(InsBefore);
+  // Keep entry block intrinsics like llvm.localescape in the entry.
+  InsBefore = PrepareToSplitEntryBlock(*InsBeforeB, InsBefore);
 
   SmallVector<ASanStackVariableDescription, 16> SVD;
   SVD.reserve(AllocaVec.size());
   for (AllocaInst *AI : AllocaVec) {
     ASanStackVariableDescription D = {AI->getName().data(),
                                       ASan.getAllocaSizeInBytes(AI),
                                       AI->getAlignment(), AI, 0};
     SVD.push_back(D);

lib/Transforms/Instrumentation/Instrumentation.cpp

 //===-- Instrumentation.cpp - TransformUtils Infrastructure ---------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines the common initialization infrastructure for the
 // Instrumentation library.
 //
 //===----------------------------------------------------------------------===//
 
-#include "llvm/InitializePasses.h"
+#include "llvm/Transforms/Instrumentation.h"
 #include "llvm-c/Initialization.h"
+#include "llvm/IR/IntrinsicInst.h"
+#include "llvm/InitializePasses.h"
 #include "llvm/PassRegistry.h"
 
 using namespace llvm;
 
+/// Moves I before IP. Returns new insert point.
+static BasicBlock::iterator moveBeforeInsertPoint(BasicBlock::iterator I, BasicBlock::iterator IP) {
+  // If I is IP, move the insert point down.
+  if (I == IP)
+    return ++IP;
+  // Otherwise, move I before IP and return IP.
+  I->moveBefore(IP);
+  return IP;
+}
+
+/// Instrumentation passes often insert conditional checks into entry blocks.
+/// Call this function before splitting the entry block to move instructions
+/// that must remain in the entry block up before the split point. Static
+/// allocas and llvm.localescape calls, for example, must remain in the entry
+/// block.
+BasicBlock::iterator llvm::PrepareToSplitEntryBlock(BasicBlock &BB,
+                                                    BasicBlock::iterator IP) {
+  assert(&BB.getParent()->getEntryBlock() == &BB);
+  for (auto I = IP, E = BB.end(); I != E; ++I) {
+    bool KeepInEntry = false;
+    if (auto *AI = dyn_cast<AllocaInst>(I)) {
+      if (AI->isStaticAlloca())
+        KeepInEntry = true;
+    } else if (auto *II = dyn_cast<IntrinsicInst>(I)) {
+      if (II->getIntrinsicID() == llvm::Intrinsic::localescape)
+        KeepInEntry = true;
+    }
+    if (KeepInEntry)
+      IP = moveBeforeInsertPoint(I, IP);
+  }
+  return IP;
+}
+
 /// initializeInstrumentation - Initialize all passes in the TransformUtils
 /// library.
 void llvm::initializeInstrumentation(PassRegistry &Registry) {
   initializeAddressSanitizerPass(Registry);
   initializeAddressSanitizerModulePass(Registry);
   initializeBoundsCheckingPass(Registry);
   initializeGCOVProfilerPass(Registry);
   initializeInstrProfilingPass(Registry);

lib/Transforms/Instrumentation/SanitizerCoverage.cpp

void SanitizerCoverageModule::InjectCoverageAtBlock(Function &F, BasicBlock &BB,
   // Don't insert coverage for unreachable blocks: we will never call
   // __sanitizer_cov() for them, so counting them in
   // NumberOfInstrumentedBlocks() might complicate calculation of code coverage
   // percentage. Also, unreachable instructions frequently have no debug
   // locations.
   if (isa<UnreachableInst>(BB.getTerminator()))
     return;
   BasicBlock::iterator IP = BB.getFirstInsertionPt(), BE = BB.end();
-  // Skip static allocas at the top of the entry block so they don't become
-  // dynamic when we split the block.  If we used our optimized stack layout,
-  // then there will only be one alloca and it will come first.
-  for (; IP != BE; ++IP) {
-    AllocaInst *AI = dyn_cast<AllocaInst>(IP);
-    if (!AI || !AI->isStaticAlloca())
-      break;
-  }
 
   bool IsEntryBB = &BB == &F.getEntryBlock();
   DebugLoc EntryLoc;
   if (IsEntryBB) {
     if (auto SP = getDISubprogram(&F))
       EntryLoc = DebugLoc::get(SP->getScopeLine(), 0, SP);
+    // Keep static allocas and llvm.localescape calls in the entry block.  Even
+    // if we aren't splitting the block, it's nice for allocas to be before
+    // calls.
+    IP = PrepareToSplitEntryBlock(BB, IP);
   } else {
     EntryLoc = IP->getDebugLoc();
   }
 
   IRBuilder<> IRB(IP);
   IRB.SetCurrentDebugLocation(EntryLoc);
   Value *GuardP = IRB.CreateAdd(
       IRB.CreatePointerCast(GuardArray, IntptrTy),

test/Instrumentation/SanitizerCoverage/localescape.ll

+; RUN: opt < %s -sancov -sanitizer-coverage-level=0 -S | FileCheck %s
+; RUN: opt < %s -sancov -sanitizer-coverage-level=1 -S | FileCheck %s
+; RUN: opt < %s -sancov -sanitizer-coverage-level=2 -S | FileCheck %s
+; RUN: opt < %s -sancov -sanitizer-coverage-level=2 -sanitizer-coverage-block-threshold=0 -S | FileCheck %s
+
+target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
+target triple = "i686-pc-windows-msvc18.0.0"
+
+declare i32 @llvm.eh.typeid.for(i8*) #2
+declare i8* @llvm.frameaddress(i32)
+declare i8* @llvm.x86.seh.recoverfp(i8*, i8*)
+declare i8* @llvm.localrecover(i8*, i8*, i32)
+declare void @llvm.localescape(...) #1
+
+declare i32 @_except_handler3(...)
+declare void @may_throw(i32* %r)
+
+define i32 @main() sanitize_address personality i8* bitcast (i32 (...)* @_except_handler3 to i8*) {
+entry:
+  %r = alloca i32, align 4
+  %__exception_code = alloca i32, align 4
+  call void (...) @llvm.localescape(i32* nonnull %__exception_code)
+  %0 = bitcast i32* %r to i8*
+  store i32 0, i32* %r, align 4
+  invoke void @may_throw(i32* nonnull %r) #4
+          to label %__try.cont unwind label %lpad
+
+lpad:                                             ; preds = %entry
+  %1 = landingpad { i8*, i32 }
+          catch i8* bitcast (i32 ()* @"\01?filt$0@0@main@@" to i8*)
+  %2 = extractvalue { i8*, i32 } %1, 1
+  %3 = call i32 @llvm.eh.typeid.for(i8* bitcast (i32 ()* @"\01?filt$0@0@main@@" to i8*)) #1
+  %matches = icmp eq i32 %2, %3
+  br i1 %matches, label %__except, label %eh.resume
+
+__except:                                         ; preds = %lpad
+  store i32 1, i32* %r, align 4
+  br label %__try.cont
+
+__try.cont:                                       ; preds = %entry, %__except
+  %4 = load i32, i32* %r, align 4
+  ret i32 %4
+
+eh.resume:                                        ; preds = %lpad
+  resume { i8*, i32 } %1
+}
+
+; Check that the alloca remains static and the localescape call remains in the
+; entry block.
+
+; CHECK-LABEL: define i32 @main()
+; CHECK-NOT: br {{.*}}label
+; CHECK: %__exception_code = alloca i32, align 4
+; CHECK-NOT: br {{.*}}label
+; CHECK: call void (...) @llvm.localescape(i32* nonnull %__exception_code)
+
+; Function Attrs: nounwind
+define internal i32 @"\01?filt$0@0@main@@"() #1 {
+entry:
+  %0 = tail call i8* @llvm.frameaddress(i32 1)
+  %1 = tail call i8* @llvm.x86.seh.recoverfp(i8* bitcast (i32 ()* @main to i8*), i8* %0)
+  %2 = tail call i8* @llvm.localrecover(i8* bitcast (i32 ()* @main to i8*), i8* %1, i32 0)
+  %__exception_code = bitcast i8* %2 to i32*
+  %3 = getelementptr inbounds i8, i8* %0, i32 -20
+  %4 = bitcast i8* %3 to { i32*, i8* }**
+  %5 = load { i32*, i8* }*, { i32*, i8* }** %4, align 4
+  %6 = getelementptr inbounds { i32*, i8* }, { i32*, i8* }* %5, i32 0, i32 0
+  %7 = load i32*, i32** %6, align 4
+  %8 = load i32, i32* %7, align 4
+  store i32 %8, i32* %__exception_code, align 4
+  ret i32 1
+}
+
+; CHECK-LABEL: define internal i32 @"\01?filt$0@0@main@@"()
+; CHECK: tail call i8* @llvm.localrecover(i8* bitcast (i32 ()* @main to i8*), i8* {{.*}}, i32 0)
+
+define void @ScaleFilterCols_SSSE3(i8* %dst_ptr, i8* %src_ptr, i32 %dst_width, i32 %x, i32 %dx) sanitize_address {
+entry:
+  %dst_width.addr = alloca i32, align 4
+  store i32 %dst_width, i32* %dst_width.addr, align 4
+  %0 = call { i8*, i8*, i32, i32, i32 } asm sideeffect "", "=r,=r,={ax},=r,=r,=*rm,rm,rm,0,1,2,3,4,5,~{memory},~{cc},~{xmm0},~{xmm1},~{xmm2},~{xmm3},~{xmm4},~{xmm5},~{xmm6},~{dirflag},~{fpsr},~{flags}"(i32* nonnull %dst_width.addr, i32 %x, i32 %dx, i8* %dst_ptr, i8* %src_ptr, i32 0, i32 0, i32 0, i32 %dst_width)
+  ret void
+}
+
+define void @ScaleColsUp2_SSE2() sanitize_address {
+entry:
+  ret void
+}