[ASan/Win] Fix PR22545 __asan_unregister_globals is not called when using -MD runtimes

[samsonov]

Looks OK to me, but let's see if Kostya is fine with this change.

lib/Transforms/Instrumentation/AddressSanitizer.cpp

1286

if (TargetTriple.isOSWindows()) { ... }

[timurrrr]

In fact this turned out to be a bug in appendToGlobalDtors(...), so I'm likely to go fix that function instead.

[timurrrr]

Reopening per http://llvm.org/bugs/show_bug.cgi?id=22545 (see comments 13-14, 17: fixing LLVM is possible, but pretty hard and it would basically be a workaround for a CRT bug)

[timurrrr]

Kostya, can you please review?

[timurrrr]

lib/Transforms/Instrumentation/AddressSanitizer.cpp

1286

Done.
I actually made a step further and used a condition from MCObjectFileInfo::InitCOFFMCObjectFileInfo() that controls the section to use for global_dtors instead.

[kcc]

lib/Transforms/Instrumentation/AddressSanitizer.cpp

1286

why can't you call atexit in the run-time lib?
Also, I am not a fan of OS-specific changes in the instrumentation pass.

[timurrrr]

Where exactly do you suggest me to call atexit() in the RTL?

[kcc]

in __asan_register_globals ?

[timurrrr]

OK, that's possible.

Since atexit portability restrictions apply, that'd still require
platform-specific instrumentation AND RTL change as well.
Do you suggest to go down that path?

[kcc]

In D7597#125736, @timurrrr wrote:

OK, that's possible.

Since atexit portability restrictions apply, that'd still require
platform-specific instrumentation AND RTL change as well.
Do you suggest to go down that path?

Why can't you do it inn rtl-only? I don't get it.

[timurrrr]

Because global_dtors DO work when the MT runtime is used. Only the MD
runtime doesn't. (See bug)

[kcc]

So? If the condition is run-time, the check should be in run-time too.

[timurrrr]

I don't quite understand your proposal.

Is it
a) leave the instrumentation without any changes
b) in the RTL, make asan_unregister_global a no-op on Windows
c) in the RTL, make asan_register_global call atexit() to schedule
asan::UnregisterGlobal?

Or am I missing something?

Please note atexit() does not allow one to pass arguments/context to the
callee function.

[timurrrr]

The condition is link-time (i.e. later than compile-time but earlier than
run-time)

[timurrrr]

Kostya has almost persuaded me that this should be fixed at the RTL level [offline].
I'm going to try that approach tomorrow...

lib/Transforms/Instrumentation/AddressSanitizer.cpp

bool AddressSanitizerModule::InstrumentGlobals(IRBuilder<> &IRB, Module &M) {
   // Create calls for poisoning before initializers run and unpoisoning after.
   if (HasDynamicallyInitializedGlobals)
     createInitializerPoisonCalls(M, ModuleName);
   IRB.CreateCall2(AsanRegisterGlobals,
                   IRB.CreatePointerCast(AllGlobals, IntptrTy),
                   ConstantInt::get(IntptrTy, n));
 
   // We also need to unregister globals at the end, e.g. when a shared library
-  // gets closed.
+  // gets closed, by using global_dtors.
   Function *AsanDtorFunction = Function::Create(
       FunctionType::get(Type::getVoidTy(*C), false),
       GlobalValue::InternalLinkage, kAsanModuleDtorName, &M);
   BasicBlock *AsanDtorBB = BasicBlock::Create(*C, "", AsanDtorFunction);
   IRBuilder<> IRB_Dtor(ReturnInst::Create(*C, AsanDtorBB));
   IRB_Dtor.CreateCall2(AsanUnregisterGlobals,
                        IRB.CreatePointerCast(AllGlobals, IntptrTy),
                        ConstantInt::get(IntptrTy, n));
+  // MSVC CRT doesn't call LLVM global_dtors, so we have to use atexit()
+  // to work around (See PR22545).
+  if (TargetTriple.isKnownWindowsMSVCEnvironment() ||
+      TargetTriple.isWindowsItaniumEnvironment()) {
+    Function *AtExit = checkInterfaceFunction(M.getOrInsertFunction(
+      "atexit", IRB.getInt32Ty(), AsanDtorFunction->getType(), nullptr));
+    AtExit->setLinkage(Function::ExternalLinkage);
+    IRB.CreateCall(AtExit, AsanDtorFunction);
+  } else {
     appendToGlobalDtors(M, AsanDtorFunction, kAsanCtorAndDtorPriority);
+  }
 
   DEBUG(dbgs() << M);
   return true;
 }
 
 bool AddressSanitizerModule::runOnModule(Module &M) {
   DataLayoutPass *DLP = getAnalysisIfAvailable<DataLayoutPass>();
   if (!DLP)

test/Instrumentation/AddressSanitizer/instrument_global.ll

-; RUN: opt < %s -asan -asan-module -S | FileCheck %s
+; RUN: opt < %s -mtriple x86_64-unknown-linux-gnu -asan -asan-module -S | FileCheck %s --check-prefix=CHECK --check-prefix=LINUX
+; RUN: opt < %s -mtriple i686-pc-windows-msvc -asan -asan-module -S | FileCheck %s --check-prefix=CHECK --check-prefix=WIN32
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
-target triple = "x86_64-unknown-linux-gnu"
 @xxx = global i32 0, align 4
 
 ; If a global is present, __asan_[un]register_globals should be called from
 ; module ctor/dtor
 
 ; CHECK: llvm.global_ctors
 ; CHECK: @__asan_gen_ = private constant [8 x i8] c"<stdin>\00", align 1
-; CHECK: llvm.global_dtors
+; LINUX: llvm.global_dtors
+; WIN32-NOT: llvm.global_dtors
 
 ; Test that we don't instrument global arrays with static initializer
 ; indexed with constants in-bounds. But instrument all other cases.
 
 @GlobSt = global [10 x i32] zeroinitializer, align 16  ; static initializer
 @GlobDy = global [10 x i32] zeroinitializer, align 16  ; dynamic initializer
 @GlobEx = external global [10 x i32] , align 16        ; extern initializer
 

 
 
 !llvm.asan.globals = !{!0}
 !0 = !{[10 x i32]* @GlobDy, null, null, i1 true, i1 false}
 
 ; CHECK-LABEL: define internal void @asan.module_ctor
 ; CHECK-NOT: ret
 ; CHECK: call void @__asan_register_globals
-; CHECK: ret
+; WIN32-NEXT: call i32 @atexit(void ()* @asan.module_dtor)
+; CHECK-NEXT: ret
 
 ; CHECK-LABEL: define internal void @asan.module_dtor
-; CHECK-NOT: ret
-; CHECK: call void @__asan_unregister_globals
-; CHECK: ret
+; CHECK-NEXT: call void @__asan_unregister_globals
+; CHECK-NEXT: ret