From: Reid Kleckner <rnk@google.com>
Date: Wed, 13 Jun 2018 20:47:21 +0000 (+0000)
Subject: [WinASan] Don't instrument globals in sections containing '$'
X-Git-Tag: android-x86-8.1-r1~2150
X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=0bcb48f4feb2702c7259862661987c96cc04eb4e;p=android-x86%2Fexternal-llvm.git

[WinASan] Don't instrument globals in sections containing '$'

Such globals are very likely to be part of a sorted section array, such
the .CRT sections used for dynamic initialization. The uses its own
sorted sections called ATL$__a, ATL$__m, and ATL$__z. Instead of special
casing them, just look for the dollar sign, which is what invokes linker
section sorting for COFF.

Avoids issues with ASan and the ATL uncovered after we started
instrumenting comdat globals on COFF.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@334653 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index d5903c74585..a9936018b43 100644
--- a/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -1710,13 +1710,17 @@ bool AddressSanitizerModule::ShouldInstrumentGlobal(GlobalVariable *G) {
       return false;
     }
 
-    // Callbacks put into the CRT initializer/terminator sections
-    // should not be instrumented.
+    // On COFF, if the section name contains '$', it is highly likely that the
+    // user is using section sorting to create an array of globals similar to
+    // the way initialization callbacks are registered in .init_array and
+    // .CRT$XCU. The ATL also registers things in .ATL$__[azm]. Adding redzones
+    // to such globals is counterproductive, because the intent is that they
+    // will form an array, and out-of-bounds accesses are expected.
     // See https://github.com/google/sanitizers/issues/305
     // and http://msdn.microsoft.com/en-US/en-en/library/bb918180(v=vs.120).aspx
-    if (Section.startswith(".CRT")) {
-      LLVM_DEBUG(dbgs() << "Ignoring a global initializer callback: " << *G
-                        << "\n");
+    if (TargetTriple.isOSBinFormatCOFF() && Section.contains('$')) {
+      LLVM_DEBUG(dbgs() << "Ignoring global in sorted section (contains '$'): "
+                        << *G << "\n");
       return false;
     }
 
diff --git a/test/Instrumentation/AddressSanitizer/win-sorted-sections.ll b/test/Instrumentation/AddressSanitizer/win-sorted-sections.ll
new file mode 100644
index 00000000000..85f759cf740
--- /dev/null
+++ b/test/Instrumentation/AddressSanitizer/win-sorted-sections.ll
@@ -0,0 +1,53 @@
+; RUN: opt < %s -asan -asan-module -S | FileCheck %s
+
+; All of these globals should pass through uninstrumented because of their
+; custom section name. The .CRT section is the standard way to register custom
+; initializers, and the ATL uses the ATL section to register other stuff.
+; Either way, if the section has a '$' character, it is probably participating
+; in section sorting, and we should probably not put a redzone around it.
+
+; ModuleID = 't.c'
+source_filename = "t.c"
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.14.26430"
+
+$__pobjMapEntryFirst = comdat any
+$__pobjMapEntryMiddle = comdat any
+$__pobjMapEntryLast = comdat any
+$__crt_init_begin = comdat any
+$__crt_init_callback = comdat any
+$__crt_init_end = comdat any
+
+@__pobjMapEntryFirst = weak_odr dso_local constant i8* null, section "ATL$__a", comdat, align 8
+@__pobjMapEntryMiddle = weak_odr dso_local constant i8* null, section "ATL$__m", comdat, align 8
+@__pobjMapEntryLast = weak_odr dso_local constant i8* null, section "ATL$__z", comdat, align 8
+@__crt_init_begin = weak_odr dso_local constant i8* null, section ".CRT$XCA", comdat, align 8
+@__crt_init_callback = weak_odr dso_local constant i8* null, section ".CRT$XCU", comdat, align 8
+@__crt_init_end = weak_odr dso_local constant i8* null, section ".CRT$XCZ", comdat, align 8
+
+; CHECK: @__pobjMapEntryFirst = weak_odr dso_local constant i8* null, section "ATL$__a", comdat, align 8
+; CHECK: @__pobjMapEntryMiddle = weak_odr dso_local constant i8* null, section "ATL$__m", comdat, align 8
+; CHECK: @__pobjMapEntryLast = weak_odr dso_local constant i8* null, section "ATL$__z", comdat, align 8
+; CHECK: @__crt_init_begin = weak_odr dso_local constant i8* null, section ".CRT$XCA", comdat, align 8
+; CHECK: @__crt_init_callback = weak_odr dso_local constant i8* null, section ".CRT$XCU", comdat, align 8
+; CHECK: @__crt_init_end = weak_odr dso_local constant i8* null, section ".CRT$XCZ", comdat, align 8
+
+!llvm.asan.globals = !{!0, !2, !4, !6, !8, !10}
+!llvm.module.flags = !{!12, !13}
+!llvm.ident = !{!14}
+
+!0 = !{i8** @__pobjMapEntryFirst, !1, !"__pobjMapEntryFirst", i1 false, i1 false}
+!1 = !{!"t.c", i32 6, i32 61}
+!2 = !{i8** @__pobjMapEntryMiddle, !3, !"__pobjMapEntryMiddle", i1 false, i1 false}
+!3 = !{!"t.c", i32 7, i32 61}
+!4 = !{i8** @__pobjMapEntryLast, !5, !"__pobjMapEntryLast", i1 false, i1 false}
+!5 = !{!"t.c", i32 8, i32 61}
+!6 = !{i8** @__crt_init_begin, !7, !"__crt_init_begin", i1 false, i1 false}
+!7 = !{!"t.c", i32 16, i32 62}
+!8 = !{i8** @__crt_init_callback, !9, !"__crt_init_callback", i1 false, i1 false}
+!9 = !{!"t.c", i32 17, i32 62}
+!10 = !{i8** @__crt_init_end, !11, !"__crt_init_end", i1 false, i1 false}
+!11 = !{!"t.c", i32 18, i32 62}
+!12 = !{i32 1, !"wchar_size", i32 2}
+!13 = !{i32 7, !"PIC Level", i32 2}
+!14 = !{!"clang version 7.0.0 "}