[llvm] r340232 - Re-land r334313 "[asan] Instrument comdat globals on COFF targets"

Reid Kleckner via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 20 16:35:45 PDT 2018


Author: rnk
Date: Mon Aug 20 16:35:45 2018
New Revision: 340232

URL: http://llvm.org/viewvc/llvm-project?rev=340232&view=rev
Log:
Re-land r334313 "[asan] Instrument comdat globals on COFF targets"

If we can use comdats, then we can make it so that the global metadata
is thrown away if the prevailing definition of the global was
uninstrumented. I have only tested this on COFF targets, but in theory,
there is no reason that we cannot also do this for ELF.

This will allow us to re-enable string merging with ASan on Windows,
reducing the binary size cost of ASan on Windows.

I tested this change with ASan+PGO, and I fixed an issue with the
__llvm_profile_raw_version symbol. With the old version of my patch, we
would attempt to instrument that symbol on ELF because it had a comdat
with external linkage. If we had been using the linker GC-friendly
metadata scheme, everything would have worked, but clang does not enable
it by default.

Added:
    llvm/trunk/test/Instrumentation/AddressSanitizer/global_metadata_external_comdat.ll
    llvm/trunk/test/Instrumentation/AddressSanitizer/win-string-literal.ll
Modified:
    llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp

Modified: llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp?rev=340232&r1=340231&r2=340232&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp (original)
+++ llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp Mon Aug 20 16:35:45 2018
@@ -1132,6 +1132,10 @@ static GlobalVariable *createPrivateGlob
 
 /// Check if \p G has been created by a trusted compiler pass.
 static bool GlobalWasGeneratedByCompiler(GlobalVariable *G) {
+  // Do not instrument @llvm.global_ctors, @llvm.used, etc.
+  if (G->getName().startswith("llvm."))
+    return true;
+
   // Do not instrument asan globals.
   if (G->getName().startswith(kAsanGenPrefix) ||
       G->getName().startswith(kSanCovGenPrefix) ||
@@ -1653,14 +1657,6 @@ bool AddressSanitizerModule::ShouldInstr
   if (!Ty->isSized()) return false;
   if (!G->hasInitializer()) return false;
   if (GlobalWasGeneratedByCompiler(G)) return false; // Our own globals.
-  // Touch only those globals that will not be defined in other modules.
-  // Don't handle ODR linkage types and COMDATs since other modules may be built
-  // without ASan.
-  if (G->getLinkage() != GlobalVariable::ExternalLinkage &&
-      G->getLinkage() != GlobalVariable::PrivateLinkage &&
-      G->getLinkage() != GlobalVariable::InternalLinkage)
-    return false;
-  if (G->hasComdat()) return false;
   // Two problems with thread-locals:
   //   - The address of the main thread's copy can't be computed at link-time.
   //   - Need to poison all copies, not just the main thread's one.
@@ -1668,6 +1664,33 @@ bool AddressSanitizerModule::ShouldInstr
   // For now, just ignore this Global if the alignment is large.
   if (G->getAlignment() > MinRedzoneSizeForGlobal()) return false;
 
+  // For non-COFF targets, only instrument globals known to be defined by this
+  // TU.
+  // FIXME: We can instrument comdat globals on ELF if we are using the
+  // GC-friendly metadata scheme.
+  if (!TargetTriple.isOSBinFormatCOFF()) {
+    if (!G->hasExactDefinition() || G->hasComdat())
+      return false;
+  } else {
+    // On COFF, don't instrument non-ODR linkages.
+    if (G->isInterposable())
+      return false;
+  }
+
+  // If a comdat is present, it must have a selection kind that implies ODR
+  // semantics: no duplicates, any, or exact match.
+  if (Comdat *C = G->getComdat()) {
+    switch (C->getSelectionKind()) {
+    case Comdat::Any:
+    case Comdat::ExactMatch:
+    case Comdat::NoDuplicates:
+      break;
+    case Comdat::Largest:
+    case Comdat::SameSize:
+      return false;
+    }
+  }
+
   if (G->hasSection()) {
     StringRef Section = G->getSection();
 
@@ -2121,6 +2144,7 @@ bool AddressSanitizerModule::InstrumentG
         new GlobalVariable(M, NewTy, G->isConstant(), Linkage, NewInitializer,
                            "", G, G->getThreadLocalMode());
     NewGlobal->copyAttributesFrom(G);
+    NewGlobal->setComdat(G->getComdat());
     NewGlobal->setAlignment(MinRZ);
 
     // Move null-terminated C strings to "__asan_cstring" section on Darwin.

Added: llvm/trunk/test/Instrumentation/AddressSanitizer/global_metadata_external_comdat.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Instrumentation/AddressSanitizer/global_metadata_external_comdat.ll?rev=340232&view=auto
==============================================================================
--- llvm/trunk/test/Instrumentation/AddressSanitizer/global_metadata_external_comdat.ll (added)
+++ llvm/trunk/test/Instrumentation/AddressSanitizer/global_metadata_external_comdat.ll Mon Aug 20 16:35:45 2018
@@ -0,0 +1,9 @@
+; RUN: opt < %s -mtriple=x86_64-linux -asan -asan-module -asan-globals-live-support=0 -S | FileCheck %s
+
+$my_var = comdat any
+
+ at my_var = global i32 42, align 4, comdat
+; CHECK: @my_var = global i32 42, align 4, comdat
+
+; Don't instrument my_var, it is comdat.
+; CHECK-NOT: __asan_register_globals

Added: llvm/trunk/test/Instrumentation/AddressSanitizer/win-string-literal.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Instrumentation/AddressSanitizer/win-string-literal.ll?rev=340232&view=auto
==============================================================================
--- llvm/trunk/test/Instrumentation/AddressSanitizer/win-string-literal.ll (added)
+++ llvm/trunk/test/Instrumentation/AddressSanitizer/win-string-literal.ll Mon Aug 20 16:35:45 2018
@@ -0,0 +1,46 @@
+; RUN: opt < %s -asan -asan-module -S | FileCheck %s
+
+; Generated like so:
+; $ clang -S -emit-llvm -Xclang -disable-llvm-passes -fsanitize=address -O1 t.cpp -o t.ll
+; const char *getstr() { return "asdf"; }
+
+; CHECK: $"??_C at _04JIHMPGLA@asdf?$AA@" = comdat any
+
+; CHECK: @"??_C at _04JIHMPGLA@asdf?$AA@" =
+; CHECK-SAME: linkonce_odr dso_local unnamed_addr constant { [5 x i8], [59 x i8] }
+; CHECK-SAME: { [5 x i8] c"asdf\00", [59 x i8] zeroinitializer }, comdat, align 32
+
+; CHECK: @"__asan_global_??_C at _04JIHMPGLA@asdf?$AA@" =
+; CHECK-SAME: private global { i64, i64, i64, i64, i64, i64, i64, i64 }
+; CHECK-SAME: { i64 ptrtoint ({ [5 x i8], [59 x i8] }* @"??_C at _04JIHMPGLA@asdf?$AA@" to i64),
+; CHECK-SAME:   i64 5, i64 64, i64 ptrtoint ([17 x i8]* @___asan_gen_.1 to i64),
+; CHECK-SAME:   i64 ptrtoint ([8 x i8]* @___asan_gen_ to i64), i64 0,
+; CHECK-SAME:   i64 ptrtoint ({ [6 x i8]*, i32, i32 }* @___asan_gen_.3 to i64), i64 0 },
+; CHECK-SAME:   section ".ASAN$GL", comdat($"??_C at _04JIHMPGLA@asdf?$AA@"), align 64
+
+; ModuleID = 't.cpp'
+source_filename = "t.cpp"
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.13.26131"
+
+$"??_C at _04JIHMPGLA@asdf?$AA@" = comdat any
+
+@"??_C at _04JIHMPGLA@asdf?$AA@" = linkonce_odr dso_local unnamed_addr constant [5 x i8] c"asdf\00", comdat, align 1
+
+; Function Attrs: nounwind sanitize_address uwtable
+define dso_local i8* @"?getstr@@YAPEBDXZ"() #0 {
+entry:
+  ret i8* getelementptr inbounds ([5 x i8], [5 x i8]* @"??_C at _04JIHMPGLA@asdf?$AA@", i32 0, i32 0)
+}
+
+attributes #0 = { nounwind sanitize_address uwtable }
+
+!llvm.asan.globals = !{!0}
+!llvm.module.flags = !{!2, !3}
+!llvm.ident = !{!4}
+
+!0 = !{[5 x i8]* @"??_C at _04JIHMPGLA@asdf?$AA@", !1, !"<string literal>", i1 false, i1 false}
+!1 = !{!"t.cpp", i32 1, i32 31}
+!2 = !{i32 1, !"wchar_size", i32 2}
+!3 = !{i32 7, !"PIC Level", i32 2}
+!4 = !{!"clang version 7.0.0 "}




More information about the llvm-commits mailing list