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

[Dan Liew via Phabricator via llvm-commits]

delcypher marked an inline comment as done.
delcypher added a comment.

@vitalybuka Is this good to go?



================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:2265
   // gets closed.
-  IRBuilder<> IRB_Dtor(CreateAsanModuleDtor(M));
-  IRB_Dtor.CreateCall(AsanUnregisterGlobals,
-                      {IRB.CreatePointerCast(AllGlobals, IntptrTy),
-                       ConstantInt::get(IntptrTy, N)});
+  if (DestructorKind != AsanDtorKind::None) {
+    IRBuilder<> IRB_Dtor(CreateAsanModuleDtor(M));
----------------
delcypher wrote:
> vitalybuka wrote:
> > Can you check if some pre-existing tests fail if constructor accidentally initialized it to  AsanDtorKind::None)
> 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.
@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

```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96571/new/

https://reviews.llvm.org/D96571