[ASan] Introduce a way set different ways of emitting module destructors.

[delcypher]

Summary

Previously there was no way to control how module destructors were emitted
by ModuleAddressSanitizerPass. However, we want language frontends (e.g. Clang)
to be able to decide how to emit these destructors (if at all).

This patch introduces the AsanDtorKind enum that represents the different ways
destructors can be emitted. There are currently only two valid ways to emit destructors.

• Global - Use llvm.global_dtors. This was the previous behavior and is the default.
• None - Do not emit module destructors.

The ModuleAddressSanitizerPass and the various wrappers around it have been updated
to take the AsanDtorKind as an argument.

The -asan-destructor-kind= command line argument has been introduced to make this
easy to test from opt. If this argument is specified it overrides the value passed
to the ModuleAddressSanitizerPass constructor.

Note that AsanDtorKind is not bool because we will introduce a new way to
emit destructors in a subsequent patch.

Note that AsanDtorKind is given its own header file because if it is declared
in Transforms/Instrumentation/AddressSanitizer.h it leads to compile error
(Module is ambiguous) when trying to use it in
clang/Basic/CodeGenOptions.def.

rdar://71609176

[vitalybuka]

llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerOptions.h

12

please fix clang-tidy

14

unused include?

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

2175

Please fix all clang-tidy

2265–2270

Can you check if some pre-existing tests fail if constructor accidentally initialized it to AsanDtorKind::None)

[delcypher]

llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerOptions.h

14

Oops. I thought I had removed that one. I'll fix this.

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

2175

I didn't change the variable name for the IRBuilder so this problem was in the code before my change. Should fixing this be in a separate change seeing as the code clean up isn't really related to change I'm making?

2265–2270

Sure. I'll test this. I would expect test/asan/TestCases/Posix/dlclose-test.cpp and the new llvm/test/Instrumentation/AddressSanitizer/no_global_dtors.ll to fail at a minimum but I will confirm my assumptions and report back.

[delcypher]

• Fix clang-tidy warnings
• Remove unused header include

[delcypher]

@vitalybuka Is this good to go?

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

2265–2270

@vitalybuka If I change ClOverrideDestructorKind to default to AsanDtorKind::None I get the following test failures.

$ ninja check-asan
...
Failed Tests (1):
  AddressSanitizer-x86_64-darwin :: TestCases/Posix/dlclose-test.cpp

$ ninja check-llvm
...
Failed Tests (3):
  LLVM :: Instrumentation/AddressSanitizer/global_metadata_darwin.ll
  LLVM :: Instrumentation/AddressSanitizer/instrument_global.ll
  LLVM :: Instrumentation/AddressSanitizer/no_global_dtors.ll

[vitalybuka]

LGTM

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

2175

Oh, I didn't noticed that this was preexisting.

llvm/test/Instrumentation/AddressSanitizer/no_global_dtors.ll

2

RUN: without line break is more readable
and clang format does not enforce 80char limit in tests

2–23

80char is not enforced in tests
and < can be avoided

llvm/include/llvm/Transforms/Instrumentation/AddressSanitizer.h

 //
 //===----------------------------------------------------------------------===//
 #ifndef LLVM_TRANSFORMS_INSTRUMENTATION_ADDRESSSANITIZER_H
 #define LLVM_TRANSFORMS_INSTRUMENTATION_ADDRESSSANITIZER_H
 
 #include "llvm/IR/Function.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/PassManager.h"
+#include "llvm/Transforms/Instrumentation/AddressSanitizerOptions.h"
 
 namespace llvm {
 
 /// Frontend-provided metadata for source location.
 struct LocationMetadata {
   StringRef Filename;
   int LineNo = 0;
   int ColumnNo = 0;

 /// Public interface to the address sanitizer module pass for instrumenting code
 /// to check for various memory errors.
 ///
 /// This adds 'asan.module_ctor' to 'llvm.global_ctors'. This pass may also
 /// run intependently of the function address sanitizer.
 class ModuleAddressSanitizerPass
     : public PassInfoMixin<ModuleAddressSanitizerPass> {
 public:
-  explicit ModuleAddressSanitizerPass(bool CompileKernel = false,
-                                      bool Recover = false,
-                                      bool UseGlobalGC = true,
-                                      bool UseOdrIndicator = false);
+  explicit ModuleAddressSanitizerPass(
+      bool CompileKernel = false, bool Recover = false, bool UseGlobalGC = true,
+      bool UseOdrIndicator = false,
+      AsanDtorKind DestructorKind = AsanDtorKind::Global);
   PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
   static bool isRequired() { return true; }
 
 private:
   bool CompileKernel;
   bool Recover;
   bool UseGlobalGC;
   bool UseOdrIndicator;
+  AsanDtorKind DestructorKind;
 };
 
 // Insert AddressSanitizer (address sanity checking) instrumentation
 FunctionPass *createAddressSanitizerFunctionPass(bool CompileKernel = false,
                                                  bool Recover = false,
                                                  bool UseAfterScope = false);
 ModulePass *createModuleAddressSanitizerLegacyPassPass(
     bool CompileKernel = false, bool Recover = false, bool UseGlobalsGC = true,
-    bool UseOdrIndicator = true);
+    bool UseOdrIndicator = true,
+    AsanDtorKind DestructorKind = AsanDtorKind::Global);
 
 } // namespace llvm
 
 #endif

llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerOptions.h

+//===--------- Definition of the AddressSanitizer options -------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+// This file defines data types used to set Address Sanitizer options.
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_TRANSFORMS_INSTRUMENTATION_ADDRESSSANITIZEROPTIONS_H
+#define LLVM_TRANSFORMS_INSTRUMENTATION_ADDRESSSANITIZEROPTIONS_H
+
+namespace llvm {
+
+/// Types of ASan module destructors supported
+enum class AsanDtorKind {
+  None,    ///< Do not emit any destructors for ASan
+  Global,  ///< Append to llvm.global_dtors
+  Invalid, ///< Not a valid destructor Kind.
+  // TODO(dliew): Add more more kinds.
+};
+} // namespace llvm
+#endif

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

 
 // This is on by default even though there is a bug in gold:
 // https://sourceware.org/bugzilla/show_bug.cgi?id=19002
 static cl::opt<bool>
     ClWithComdat("asan-with-comdat",
                  cl::desc("Place ASan constructors in comdat sections"),
                  cl::Hidden, cl::init(true));
 
+static cl::opt<AsanDtorKind> ClOverrideDestructorKind(
+    "asan-destructor-kind",
+    cl::desc("Sets the ASan destructor kind. The default is to use the value "
+             "provided to the pass constructor"),
+    cl::values(clEnumValN(AsanDtorKind::None, "none", "No destructors"),
+               clEnumValN(AsanDtorKind::Global, "global",
+                          "Use global destructors")),
+    cl::init(AsanDtorKind::Invalid), cl::Hidden);
+
 // Debug flags.
 
 static cl::opt<int> ClDebug("asan-debug", cl::desc("debug"), cl::Hidden,
                             cl::init(0));
 
 static cl::opt<int> ClDebugStack("asan-debug-stack", cl::desc("debug stack"),
                                  cl::Hidden, cl::init(0));
 
private:
   bool Recover;
   bool UseAfterScope;
 };
 
 class ModuleAddressSanitizer {
 public:
   ModuleAddressSanitizer(Module &M, const GlobalsMetadata *GlobalsMD,
                          bool CompileKernel = false, bool Recover = false,
-                         bool UseGlobalsGC = true, bool UseOdrIndicator = false)
+                         bool UseGlobalsGC = true, bool UseOdrIndicator = false,
+                         AsanDtorKind DestructorKind = AsanDtorKind::Global)
       : GlobalsMD(*GlobalsMD),
         CompileKernel(ClEnableKasan.getNumOccurrences() > 0 ? ClEnableKasan
                                                             : CompileKernel),
         Recover(ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover),
         UseGlobalsGC(UseGlobalsGC && ClUseGlobalsGC && !this->CompileKernel),
         // Enable aliases as they should have no downside with ODR indicators.
         UsePrivateAlias(UseOdrIndicator || ClUsePrivateAlias),
         UseOdrIndicator(UseOdrIndicator || ClUseOdrIndicator),
         // Not a typo: ClWithComdat is almost completely pointless without
         // ClUseGlobalsGC (because then it only works on modules without
         // globals, which are rare); it is a prerequisite for ClUseGlobalsGC;
         // and both suffer from gold PR19002 for which UseGlobalsGC constructor
         // argument is designed as workaround. Therefore, disable both
         // ClWithComdat and ClUseGlobalsGC unless the frontend says it's ok to
         // do globals-gc.
-        UseCtorComdat(UseGlobalsGC && ClWithComdat && !this->CompileKernel) {
+        UseCtorComdat(UseGlobalsGC && ClWithComdat && !this->CompileKernel),
+        DestructorKind(DestructorKind) {
     C = &(M.getContext());
     int LongSize = M.getDataLayout().getPointerSizeInBits();
     IntptrTy = Type::getIntNTy(*C, LongSize);
     TargetTriple = Triple(M.getTargetTriple());
     Mapping = getShadowMapping(TargetTriple, LongSize, this->CompileKernel);
+
+    if (ClOverrideDestructorKind != AsanDtorKind::Invalid)
+      this->DestructorKind = ClOverrideDestructorKind;
+    assert(this->DestructorKind != AsanDtorKind::Invalid);
   }
 
   bool instrumentModule(Module &);
 
 private:
   void initializeCallbacks(Module &M);
 
   bool InstrumentGlobals(IRBuilder<> &IRB, Module &M, bool *CtorComdat);
private:
 
   const GlobalsMetadata &GlobalsMD;
   bool CompileKernel;
   bool Recover;
   bool UseGlobalsGC;
   bool UsePrivateAlias;
   bool UseOdrIndicator;
   bool UseCtorComdat;
+  AsanDtorKind DestructorKind;
   Type *IntptrTy;
   LLVMContext *C;
   Triple TargetTriple;
   ShadowMapping Mapping;
   FunctionCallee AsanPoisonGlobals;
   FunctionCallee AsanUnpoisonGlobals;
   FunctionCallee AsanRegisterGlobals;
   FunctionCallee AsanUnregisterGlobals;
   FunctionCallee AsanRegisterImageGlobals;
   FunctionCallee AsanUnregisterImageGlobals;
   FunctionCallee AsanRegisterElfGlobals;
   FunctionCallee AsanUnregisterElfGlobals;
 
   Function *AsanCtorFunction = nullptr;
   Function *AsanDtorFunction = nullptr;
 };
 
 class ModuleAddressSanitizerLegacyPass : public ModulePass {
 public:
   static char ID;
 
-  explicit ModuleAddressSanitizerLegacyPass(bool CompileKernel = false,
-                                            bool Recover = false,
-                                            bool UseGlobalGC = true,
-                                            bool UseOdrIndicator = false)
+  explicit ModuleAddressSanitizerLegacyPass(
+      bool CompileKernel = false, bool Recover = false, bool UseGlobalGC = true,
+      bool UseOdrIndicator = false,
+      AsanDtorKind DestructorKind = AsanDtorKind::Global)
       : ModulePass(ID), CompileKernel(CompileKernel), Recover(Recover),
-        UseGlobalGC(UseGlobalGC), UseOdrIndicator(UseOdrIndicator) {
+        UseGlobalGC(UseGlobalGC), UseOdrIndicator(UseOdrIndicator),
+        DestructorKind(DestructorKind) {
     initializeModuleAddressSanitizerLegacyPassPass(
         *PassRegistry::getPassRegistry());
   }
 
   StringRef getPassName() const override { return "ModuleAddressSanitizer"; }
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.addRequired<ASanGlobalsMetadataWrapperPass>();
   }
 
   bool runOnModule(Module &M) override {
     GlobalsMetadata &GlobalsMD =
         getAnalysis<ASanGlobalsMetadataWrapperPass>().getGlobalsMD();
     ModuleAddressSanitizer ASanModule(M, &GlobalsMD, CompileKernel, Recover,
-                                      UseGlobalGC, UseOdrIndicator);
+                                      UseGlobalGC, UseOdrIndicator,
+                                      DestructorKind);
     return ASanModule.instrumentModule(M);
   }
 
 private:
   bool CompileKernel;
   bool Recover;
   bool UseGlobalGC;
   bool UseOdrIndicator;
+  AsanDtorKind DestructorKind;
 };
 
 // Stack poisoning does not play well with exception handling.
 // When an exception is thrown, we essentially bypass the code
 // that unpoisones the stack. This is why the run-time library has
 // to intercept __cxa_throw (as well as longjmp, etc) and unpoison the entire
 // stack in the interceptor. This however does not work inside the
 // actual function which catches the exception. Most likely because the
PreservedAnalyses AddressSanitizerPass::run(Function &F,
   }
 
   report_fatal_error(
       "The ASanGlobalsMetadataAnalysis is required to run before "
       "AddressSanitizer can run");
   return PreservedAnalyses::all();
 }
 
-ModuleAddressSanitizerPass::ModuleAddressSanitizerPass(bool CompileKernel,
-                                                       bool Recover,
-                                                       bool UseGlobalGC,
-                                                       bool UseOdrIndicator)
+ModuleAddressSanitizerPass::ModuleAddressSanitizerPass(
+    bool CompileKernel, bool Recover, bool UseGlobalGC, bool UseOdrIndicator,
+    AsanDtorKind DestructorKind)
     : CompileKernel(CompileKernel), Recover(Recover), UseGlobalGC(UseGlobalGC),
-      UseOdrIndicator(UseOdrIndicator) {}
+      UseOdrIndicator(UseOdrIndicator), DestructorKind(DestructorKind) {}
 
 PreservedAnalyses ModuleAddressSanitizerPass::run(Module &M,
                                                   AnalysisManager<Module> &AM) {
   GlobalsMetadata &GlobalsMD = AM.getResult<ASanGlobalsMetadataAnalysis>(M);
   ModuleAddressSanitizer Sanitizer(M, &GlobalsMD, CompileKernel, Recover,
-                                   UseGlobalGC, UseOdrIndicator);
+                                   UseGlobalGC, UseOdrIndicator,
+                                   DestructorKind);
   if (Sanitizer.instrumentModule(M))
     return PreservedAnalyses::none();
   return PreservedAnalyses::all();
 }
 
 INITIALIZE_PASS(ASanGlobalsMetadataWrapperPass, "asan-globals-md",
                 "Read metadata to mark which globals should be instrumented "
                 "when running ASan.",

 
 INITIALIZE_PASS(
     ModuleAddressSanitizerLegacyPass, "asan-module",
     "AddressSanitizer: detects use-after-free and out-of-bounds bugs."
     "ModulePass",
     false, false)
 
 ModulePass *llvm::createModuleAddressSanitizerLegacyPassPass(
-    bool CompileKernel, bool Recover, bool UseGlobalsGC, bool UseOdrIndicator) {
+    bool CompileKernel, bool Recover, bool UseGlobalsGC, bool UseOdrIndicator,
+    AsanDtorKind Destructor) {
   assert(!CompileKernel || Recover);
-  return new ModuleAddressSanitizerLegacyPass(CompileKernel, Recover,
-                                              UseGlobalsGC, UseOdrIndicator);
+  return new ModuleAddressSanitizerLegacyPass(
+      CompileKernel, Recover, UseGlobalsGC, UseOdrIndicator, Destructor);
 }
 
 static size_t TypeSizeToSizeIndex(uint32_t TypeSize) {
   size_t Res = countTrailingZeros(TypeSize / 8);
   assert(Res < kNumberOfAccessSizes);
   return Res;
 }
 
void ModuleAddressSanitizer::InstrumentGlobalsELF(
   // Create a call to register the globals with the runtime.
   IRB.CreateCall(AsanRegisterElfGlobals,
                  {IRB.CreatePointerCast(RegisteredFlag, IntptrTy),
                   IRB.CreatePointerCast(StartELFMetadata, IntptrTy),
                   IRB.CreatePointerCast(StopELFMetadata, IntptrTy)});
 
   // We also need to unregister globals at the end, e.g., when a shared library
   // gets closed.
-  IRBuilder<> IRB_Dtor(CreateAsanModuleDtor(M));
-  IRB_Dtor.CreateCall(AsanUnregisterElfGlobals,
+  if (DestructorKind != AsanDtorKind::None) {
+    IRBuilder<> IrbDtor(CreateAsanModuleDtor(M));
+    IrbDtor.CreateCall(AsanUnregisterElfGlobals,
                        {IRB.CreatePointerCast(RegisteredFlag, IntptrTy),
                         IRB.CreatePointerCast(StartELFMetadata, IntptrTy),
                         IRB.CreatePointerCast(StopELFMetadata, IntptrTy)});
   }
+}
 
 void ModuleAddressSanitizer::InstrumentGlobalsMachO(
     IRBuilder<> &IRB, Module &M, ArrayRef<GlobalVariable *> ExtendedGlobals,
     ArrayRef<Constant *> MetadataInitializers) {
   assert(ExtendedGlobals.size() == MetadataInitializers.size());
 
   // On recent Mach-O platforms, use a structure which binds the liveness of
   // the global variable to the metadata struct. Keep the list of "Liveness" GV
GlobalVariable *RegisteredFlag = new GlobalVariable(
       ConstantInt::get(IntptrTy, 0), kAsanGlobalsRegisteredFlagName);
   RegisteredFlag->setVisibility(GlobalVariable::HiddenVisibility);
 
   IRB.CreateCall(AsanRegisterImageGlobals,
                  {IRB.CreatePointerCast(RegisteredFlag, IntptrTy)});
 
   // We also need to unregister globals at the end, e.g., when a shared library
   // gets closed.
-  IRBuilder<> IRB_Dtor(CreateAsanModuleDtor(M));
-  IRB_Dtor.CreateCall(AsanUnregisterImageGlobals,
+  if (DestructorKind != AsanDtorKind::None) {
+    IRBuilder<> IrbDtor(CreateAsanModuleDtor(M));
+    IrbDtor.CreateCall(AsanUnregisterImageGlobals,
                        {IRB.CreatePointerCast(RegisteredFlag, IntptrTy)});
   }
+}
 
 void ModuleAddressSanitizer::InstrumentGlobalsWithMetadataArray(
     IRBuilder<> &IRB, Module &M, ArrayRef<GlobalVariable *> ExtendedGlobals,
     ArrayRef<Constant *> MetadataInitializers) {
   assert(ExtendedGlobals.size() == MetadataInitializers.size());
   unsigned N = ExtendedGlobals.size();
   assert(N > 0);
 
   // On platforms that don't have a custom metadata section, we emit an array
   // of global metadata structures.
   ArrayType *ArrayOfGlobalStructTy =
       ArrayType::get(MetadataInitializers[0]->getType(), N);
   auto AllGlobals = new GlobalVariable(
       M, ArrayOfGlobalStructTy, false, GlobalVariable::InternalLinkage,
       ConstantArray::get(ArrayOfGlobalStructTy, MetadataInitializers), "");
   if (Mapping.Scale > 3)
     AllGlobals->setAlignment(Align(1ULL << Mapping.Scale));
 
   IRB.CreateCall(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.
-  IRBuilder<> IRB_Dtor(CreateAsanModuleDtor(M));
-  IRB_Dtor.CreateCall(AsanUnregisterGlobals,
+  if (DestructorKind != AsanDtorKind::None) {
+    IRBuilder<> IrbDtor(CreateAsanModuleDtor(M));
+    IrbDtor.CreateCall(AsanUnregisterGlobals,
                        {IRB.CreatePointerCast(AllGlobals, IntptrTy),
                         ConstantInt::get(IntptrTy, N)});
   }
+}
 
 // This function replaces all global variables with new variables that have
 // trailing redzones. It also creates a function that poisons
 // redzones and inserts this function into llvm.global_ctors.
 // Sets *CtorComdat to true if the global registration code emitted into the
 // asan constructor is comdat-compatible.
 bool ModuleAddressSanitizer::InstrumentGlobals(IRBuilder<> &IRB, Module &M,
                                                bool *CtorComdat) {

llvm/test/Instrumentation/AddressSanitizer/no_global_dtors.ll

+; Check Default behaviour still emits dtors
+; RUN: opt < %s -asan -asan-module -enable-new-pm=0 -S | \
+; RUN:   FileCheck -check-prefix=CHECK-DEFAULT %s
+; RUN: opt < %s -passes='asan-pipeline' -S | \
+; RUN:   FileCheck -check-prefix=CHECK-DEFAULT %s
+; CHECK-DEFAULT: llvm.global_dtor{{.+}}asan.module_dtor
+; CHECK-DEFAULT: define internal void @asan.module_dtor
+
+; Check with dtor emission disabled
+; RUN: opt < %s -asan -asan-module -enable-new-pm=0 \
+; RUN:   -asan-destructor-kind=none -S | \
+; RUN:   FileCheck %s
+; RUN: opt < %s -passes='asan-pipeline' \
+; RUN:   -asan-destructor-kind=none -S | \
+; RUN:   FileCheck %s
+; CHECK-NOT: llvm.global_dtor{{.+}}asan.module_dtor
+; CHECK-NOT: define internal void @asan.module_dtor
+
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx11.0.0"
+
+@foo = dso_local global i32 0, align 4