[asan] Disable instrumentation for available_externally global with COFF

[yingcong-wu]

For COFF, available_externally global will be instrumented because of the lack of filtering, and will trigger the Verifier pass assertion and crash the compilation. This patch will filter out the available_externally global for COFF.

For non-COFF, !G->hasExactDefinition() in line 1954 will filter out the available_externally globals.

There is a related bug reported in https://bugs.llvm.org/show_bug.cgi?id=47950 / #47294. I tried the reproducer posted on the page and this will fix the problem.

Reproducer:

#include <locale>

void grouping_impl() {
  std::use_facet<std::numpunct<char>>(std::locale());
}

// clang -fsanitize=address -D_DLL -std=c++14 -c format.cc

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

@llvm/pr-subscribers-llvm-transforms

Author: Wu Yingcong (yingcong-wu)

Changes

For COFF, available_externally global will be instrumented because of the lack of filtering, and will trigger the Verifier pass assertion and crash the compilation. This patch will filter out the available_externally global for COFF.

For non-COFF, !G->hasExactDefinition() in line 1954 will filter out the available_externally globals.

There is a related bug reported in https://www.mail-archive.com/llvm-bugs@lists.llvm.org/msg46110.html. I tried the reproducer posted on the page and this will fix the problem.

---

Full diff: https://github.com/llvm/llvm-project/pull/81109.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp (+4)
• (added) llvm/test/Instrumentation/AddressSanitizer/do-not-instrument-globals-windows.ll (+10)

diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index d4f5bf8c39356..80643e5b04860 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -1957,6 +1957,10 @@ bool ModuleAddressSanitizer::shouldInstrumentGlobal(GlobalVariable *G) const {
     // On COFF, don't instrument non-ODR linkages.
     if (G->isInterposable())
       return false;
+    // If the global has AvailableExternally linkage, then it is not in this 
+    // module, which means it does not need to be instrumented.
+    if (G->hasAvailableExternallyLinkage())
+      return false;
   }
 
   // If a comdat is present, it must have a selection kind that implies ODR
diff --git a/llvm/test/Instrumentation/AddressSanitizer/do-not-instrument-globals-windows.ll b/llvm/test/Instrumentation/AddressSanitizer/do-not-instrument-globals-windows.ll
new file mode 100644
index 0000000000000..c143f69f126a8
--- /dev/null
+++ b/llvm/test/Instrumentation/AddressSanitizer/do-not-instrument-globals-windows.ll
@@ -0,0 +1,10 @@
+; This test checks that we are not instrumenting unnecessary globals
+; RUN: opt < %s -passes=asan -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc"
+
+@v_available_externally = available_externally global i32 zeroinitializer
+; CHECK-NOT: {{asan_gen.*v_available_externally}}
+
+; CHECK: @asan.module_ctor

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[yingcong-wu]

Oh, the issue is also in the Github #47294.

[yingcong-wu]

Hi @vitalybuka, could you please review this patch?

[yingcong-wu]

Kindly ping @vitalybuka.

Hi @MaskRay, could you please help review this patch?

[MaskRay]

I feel that we should better organize global variable tests by combining some into one file. They are currently scattered everywhere.

basic-msvc64.ll may be a better file for this test since basic.ll tests ELF available_externally.

[yingcong-wu]

Hi @MaskRay , could you please help land this patch?

basic-msvc64.ll may be a better file for this test since basic.ll tests ELF available_externally.

I think basic.ll and basic-msvc64.ll are about testing function instrumentation. So I would like to keep the test in the global related test. But I can move the test to basic-msvc64.ll if you want me to do so.