[PATCH] D127911: Delete 'llvm.asan.globals' for global metadata.

2022-06-28 Thread Mitch Phillips via Phabricator via cfe-commits 
hctim added a comment.

In D127911#3614242 , @thakis wrote:

> Looks like this breaks check-clang on windows: 
> http://45.33.8.238/win/61067/step_7.txt
>
> Please take a look and revert for now if it takes a while to fix.

Thanks. Added a2095d1aff84 
. Bot 
looks green now: http://45.33.8.238/win/61118/summary.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127911

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127911: Delete 'llvm.asan.globals' for global metadata.

2022-06-28 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

Looks like this breaks check-clang on windows: 
http://45.33.8.238/win/61067/step_7.txt

Please take a look and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127911

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127911: Delete 'llvm.asan.globals' for global metadata.

2022-06-27 Thread Mitch Phillips via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
hctim marked an inline comment as done.
Closed by commit rGdacfa24f75c3: Delete llvm.asan.globals for 
global metadata. (authored by hctim).

Changed prior to commit:
  https://reviews.llvm.org/D127911?vs=439918=440404#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127911

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/lib/CodeGen/SanitizerMetadata.h
  clang/test/CodeGen/asan-globals.cpp
  clang/test/CodeGen/hwasan-globals.cpp
  clang/test/CodeGen/memtag-globals.cpp
  clang/test/CodeGen/sanitize-init-order.cpp
  compiler-rt/test/asan/TestCases/global-location-nodebug.cpp
  compiler-rt/test/asan/TestCases/global-location.cpp
  llvm/include/llvm/Transforms/Instrumentation/AddressSanitizer.h
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/test/Instrumentation/AddressSanitizer/global_metadata.ll
  llvm/test/Instrumentation/AddressSanitizer/global_metadata_array.ll
  llvm/test/Instrumentation/AddressSanitizer/global_with_comdat.ll
  llvm/test/Instrumentation/AddressSanitizer/instrument_global.ll
  llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
  llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
  llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
  llvm/tools/opt/NewPMDriver.cpp

Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -357,8 +357,6 @@
  ArrayRef) {
 AddressSanitizerOptions Opts;
 if (Name == "asan-pipeline") {
-  MPM.addPass(
-  RequireAnalysisPass());
   MPM.addPass(ModuleAddressSanitizerPass(Opts));
   return true;
 }
Index: llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
@@ -2,7 +2,7 @@
 ; Make sure asan does not instrument __sancov_gen_
 
 ; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S  | FileCheck %s
-; RUN: opt < %s -passes='module(require,sancov-module,asan-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S  | FileCheck %s
+; RUN: opt < %s -passes='module(sancov-module,asan-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S  | FileCheck %s
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 $Foo = comdat any
Index: llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
===
--- llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
+++ llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
@@ -10,13 +10,12 @@
 ; CHECK-SAME: linkonce_odr dso_local constant { [5 x i8], [27 x i8] }
 ; CHECK-SAME: { [5 x i8] c"asdf\00", [27 x i8] zeroinitializer }, comdat, align 32
 
-; CHECK: @"__asan_global_??_C@_04JIHMPGLA@asdf?$AA@" =
+; CHECK:  @"__asan_global_??_C@_04JIHMPGLA@asdf?$AA@" =
 ; CHECK-SAME: private global { i64, i64, i64, i64, i64, i64, i64, i64 }
 ; CHECK-SAME: { i64 ptrtoint ({ [5 x i8], [27 x i8] }* @"??_C@_04JIHMPGLA@asdf?$AA@" to i64),
-; CHECK-SAME:   i64 5, i64 32, 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@_04JIHMPGLA@asdf?$AA@"), align 64
+; CHECK-SAME:   i64 5, i64 32, i64 ptrtoint ([7 x i8]* @___asan_gen_.1 to i64), i64 ptrtoint ([8
+; CHECK-SAME:   x i8]* @___asan_gen_ to i64), i64 0, i64 0, i64 0 }, section ".ASAN$GL",
+; CHECK-SAME:   comdat($"??_C@_04JIHMPGLA@asdf?$AA@"), align 64
 
 ; ModuleID = 't.cpp'
 source_filename = "t.cpp"
@@ -35,11 +34,9 @@
 
 attributes #0 = { nounwind sanitize_address uwtable }
 
-!llvm.asan.globals = !{!0}
 !llvm.module.flags = !{!2, !3}
 !llvm.ident = !{!4}
 
-!0 = !{[5 x i8]* @"??_C@_04JIHMPGLA@asdf?$AA@", !1, !"", 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}
Index: llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
===
--- llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
+++ llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
@@ -2,17 +2,10 @@
 ; RUN: opt < %s

[PATCH] D127911: Delete 'llvm.asan.globals' for global metadata.

2022-06-27 Thread Vitaly Buka via Phabricator via cfe-commits 
vitalybuka added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1355-1356
   // at all, we assume it has dynamic initializer (in other TU).
-  //
-  // FIXME: Metadata should be attched directly to the global directly instead
-  // of being added to llvm.asan.globals.
-  return G->hasInitializer() && !GlobalsMD.get(G).IsDynInit;
+  if (G->hasSanitizerMetadata() && G->getSanitizerMetadata().IsDynInit)
+return false;
+

hctim wrote:
> vitalybuka wrote:
> > vitalybuka wrote:
> > > hctim wrote:
> > > > vitalybuka wrote:
> > > > > I believe previous was like this.
> > > > > if you want to change that lets do another patch. 
> > > > refactored it slightly, it's clear to me now (and IMHO much clearer to 
> > > > reason about, i suck at flipping multiple conditions in my head) that 
> > > > it's the same code
> > > Before: G->hasInitializer() && !GlobalsMD.get(G).IsDynInit;
> > > Now:  G->hasInitializer() && !(G->hasSanitizerMetadata() && 
> > > G->getSanitizerMetadata().IsDynInit)
> > > 
> > > Which is fine, because previously NoMD == !IsDynInit
> > > 
> > > So logic-wise this version is LGTM
> > > equivalent one-liner is even cleaner:
> > > return G->hasInitializer() && !(G->hasSanitizerMetadata() && 
> > > G->getSanitizerMetadata().IsDynInit)
> > > Before: G->hasInitializer() && !GlobalsMD.get(G).IsDynInit;
> > "Before" is "Before the patch"
> > 
> I personally find the multi-liner much easier to read than the one-liner, 
> okay to leave?
up to you


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127911

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127911: Delete 'llvm.asan.globals' for global metadata.

2022-06-27 Thread Mitch Phillips via Phabricator via cfe-commits 
hctim marked 3 inline comments as done.
hctim added inline comments.



Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:67-72
+  if (FsanitizeArgument.has(SanitizerKind::Address) && !Meta.NoAddress) {
 IsDynInit &= !CGM.isInNoSanitizeList(SanitizerKind::Address |
  SanitizerKind::KernelAddress,
  GV, Loc, Ty, "init");
 Meta.IsDynInit = IsDynInit;
   }

hctim wrote:
> vitalybuka wrote:
> > I recommend to move this change into another patch
> > 
> > and it should probably be:
> > Meta.IsDynInit &= IsDynInit && 
> > FsanitizeArgument.has(SanitizerKind::Address) && !Meta.NoAddress && 
> > !CGM.isInNoSanitizeLis;
> sure, will punt to follow-up patch (leaving comment open, will close it out 
> when i've added the dependency)
(punted to D128672)



Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1355-1356
   // at all, we assume it has dynamic initializer (in other TU).
-  //
-  // FIXME: Metadata should be attched directly to the global directly instead
-  // of being added to llvm.asan.globals.
-  return G->hasInitializer() && !GlobalsMD.get(G).IsDynInit;
+  if (G->hasSanitizerMetadata() && G->getSanitizerMetadata().IsDynInit)
+return false;
+

vitalybuka wrote:
> vitalybuka wrote:
> > hctim wrote:
> > > vitalybuka wrote:
> > > > I believe previous was like this.
> > > > if you want to change that lets do another patch. 
> > > refactored it slightly, it's clear to me now (and IMHO much clearer to 
> > > reason about, i suck at flipping multiple conditions in my head) that 
> > > it's the same code
> > Before: G->hasInitializer() && !GlobalsMD.get(G).IsDynInit;
> > Now:  G->hasInitializer() && !(G->hasSanitizerMetadata() && 
> > G->getSanitizerMetadata().IsDynInit)
> > 
> > Which is fine, because previously NoMD == !IsDynInit
> > 
> > So logic-wise this version is LGTM
> > equivalent one-liner is even cleaner:
> > return G->hasInitializer() && !(G->hasSanitizerMetadata() && 
> > G->getSanitizerMetadata().IsDynInit)
> > Before: G->hasInitializer() && !GlobalsMD.get(G).IsDynInit;
> "Before" is "Before the patch"
> 
I personally find the multi-liner much easier to read than the one-liner, okay 
to leave?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127911

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127911: Delete 'llvm.asan.globals' for global metadata.

2022-06-24 Thread Vitaly Buka via Phabricator via cfe-commits 
vitalybuka added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1355-1356
   // at all, we assume it has dynamic initializer (in other TU).
-  //
-  // FIXME: Metadata should be attched directly to the global directly instead
-  // of being added to llvm.asan.globals.
-  return G->hasInitializer() && !GlobalsMD.get(G).IsDynInit;
+  if (G->hasSanitizerMetadata() && G->getSanitizerMetadata().IsDynInit)
+return false;
+

vitalybuka wrote:
> hctim wrote:
> > vitalybuka wrote:
> > > I believe previous was like this.
> > > if you want to change that lets do another patch. 
> > refactored it slightly, it's clear to me now (and IMHO much clearer to 
> > reason about, i suck at flipping multiple conditions in my head) that it's 
> > the same code
> Before: G->hasInitializer() && !GlobalsMD.get(G).IsDynInit;
> Now:  G->hasInitializer() && !(G->hasSanitizerMetadata() && 
> G->getSanitizerMetadata().IsDynInit)
> 
> Which is fine, because previously NoMD == !IsDynInit
> 
> So logic-wise this version is LGTM
> equivalent one-liner is even cleaner:
> return G->hasInitializer() && !(G->hasSanitizerMetadata() && 
> G->getSanitizerMetadata().IsDynInit)
> Before: G->hasInitializer() && !GlobalsMD.get(G).IsDynInit;
"Before" is "Before the patch"



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127911

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127911: Delete 'llvm.asan.globals' for global metadata.

2022-06-24 Thread Vitaly Buka via Phabricator via cfe-commits 
vitalybuka accepted this revision.
vitalybuka added inline comments.
This revision is now accepted and ready to land.



Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1355-1356
   // at all, we assume it has dynamic initializer (in other TU).
-  //
-  // FIXME: Metadata should be attched directly to the global directly instead
-  // of being added to llvm.asan.globals.
-  return G->hasInitializer() && !GlobalsMD.get(G).IsDynInit;
+  if (G->hasSanitizerMetadata() && G->getSanitizerMetadata().IsDynInit)
+return false;
+

hctim wrote:
> vitalybuka wrote:
> > I believe previous was like this.
> > if you want to change that lets do another patch. 
> refactored it slightly, it's clear to me now (and IMHO much clearer to reason 
> about, i suck at flipping multiple conditions in my head) that it's the same 
> code
Before: G->hasInitializer() && !GlobalsMD.get(G).IsDynInit;
Now:  G->hasInitializer() && !(G->hasSanitizerMetadata() && 
G->getSanitizerMetadata().IsDynInit)

Which is fine, because previously NoMD == !IsDynInit

So logic-wise this version is LGTM
equivalent one-liner is even cleaner:
return G->hasInitializer() && !(G->hasSanitizerMetadata() && 
G->getSanitizerMetadata().IsDynInit)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127911

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127911: Delete 'llvm.asan.globals' for global metadata.

2022-06-24 Thread Mitch Phillips via Phabricator via cfe-commits 
hctim updated this revision to Diff 439918.
hctim marked 3 inline comments as done.
hctim added a comment.

Vitaly's comments, round 2.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127911

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/lib/CodeGen/SanitizerMetadata.h
  clang/test/CodeGen/asan-globals.cpp
  clang/test/CodeGen/hwasan-globals.cpp
  clang/test/CodeGen/memtag-globals.cpp
  clang/test/CodeGen/sanitize-init-order.cpp
  compiler-rt/test/asan/TestCases/global-location-nodebug.cpp
  compiler-rt/test/asan/TestCases/global-location.cpp
  llvm/include/llvm/Transforms/Instrumentation/AddressSanitizer.h
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/test/Instrumentation/AddressSanitizer/global_metadata.ll
  llvm/test/Instrumentation/AddressSanitizer/global_metadata_array.ll
  llvm/test/Instrumentation/AddressSanitizer/global_with_comdat.ll
  llvm/test/Instrumentation/AddressSanitizer/instrument_global.ll
  llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
  llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
  llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
  llvm/tools/opt/NewPMDriver.cpp

Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -357,8 +357,6 @@
  ArrayRef) {
 AddressSanitizerOptions Opts;
 if (Name == "asan-pipeline") {
-  MPM.addPass(
-  RequireAnalysisPass());
   MPM.addPass(ModuleAddressSanitizerPass(Opts));
   return true;
 }
Index: llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
@@ -2,7 +2,7 @@
 ; Make sure asan does not instrument __sancov_gen_
 
 ; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S  | FileCheck %s
-; RUN: opt < %s -passes='module(require,sancov-module,asan-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S  | FileCheck %s
+; RUN: opt < %s -passes='module(sancov-module,asan-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S  | FileCheck %s
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 $Foo = comdat any
Index: llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
===
--- llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
+++ llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
@@ -10,13 +10,12 @@
 ; CHECK-SAME: linkonce_odr dso_local constant { [5 x i8], [27 x i8] }
 ; CHECK-SAME: { [5 x i8] c"asdf\00", [27 x i8] zeroinitializer }, comdat, align 32
 
-; CHECK: @"__asan_global_??_C@_04JIHMPGLA@asdf?$AA@" =
+; CHECK:  @"__asan_global_??_C@_04JIHMPGLA@asdf?$AA@" =
 ; CHECK-SAME: private global { i64, i64, i64, i64, i64, i64, i64, i64 }
 ; CHECK-SAME: { i64 ptrtoint ({ [5 x i8], [27 x i8] }* @"??_C@_04JIHMPGLA@asdf?$AA@" to i64),
-; CHECK-SAME:   i64 5, i64 32, 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@_04JIHMPGLA@asdf?$AA@"), align 64
+; CHECK-SAME:   i64 5, i64 32, i64 ptrtoint ([7 x i8]* @___asan_gen_.1 to i64), i64 ptrtoint ([8
+; CHECK-SAME:   x i8]* @___asan_gen_ to i64), i64 0, i64 0, i64 0 }, section ".ASAN$GL",
+; CHECK-SAME:   comdat($"??_C@_04JIHMPGLA@asdf?$AA@"), align 64
 
 ; ModuleID = 't.cpp'
 source_filename = "t.cpp"
@@ -35,11 +34,9 @@
 
 attributes #0 = { nounwind sanitize_address uwtable }
 
-!llvm.asan.globals = !{!0}
 !llvm.module.flags = !{!2, !3}
 !llvm.ident = !{!4}
 
-!0 = !{[5 x i8]* @"??_C@_04JIHMPGLA@asdf?$AA@", !1, !"", 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}
Index: llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
===
--- llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
+++ llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
@@ -2,17 +2,10 @@
 ; RUN: opt < %s -passes='asan-pipeline' -asan-mapping-scale=5 -S | FileCheck %s
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"

[PATCH] D127911: Delete 'llvm.asan.globals' for global metadata.

2022-06-24 Thread Mitch Phillips via Phabricator via cfe-commits 
hctim added inline comments.



Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:67-72
+  if (FsanitizeArgument.has(SanitizerKind::Address) && !Meta.NoAddress) {
 IsDynInit &= !CGM.isInNoSanitizeList(SanitizerKind::Address |
  SanitizerKind::KernelAddress,
  GV, Loc, Ty, "init");
 Meta.IsDynInit = IsDynInit;
   }

vitalybuka wrote:
> I recommend to move this change into another patch
> 
> and it should probably be:
> Meta.IsDynInit &= IsDynInit && FsanitizeArgument.has(SanitizerKind::Address) 
> && !Meta.NoAddress && !CGM.isInNoSanitizeLis;
sure, will punt to follow-up patch (leaving comment open, will close it out 
when i've added the dependency)



Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:34
 #include "llvm/BinaryFormat/MachO.h"
+#include "llvm/Demangle/Demangle.h"
 #include "llvm/IR/Argument.h"

vitalybuka wrote:
> Please don't demangle in this patch, or keep as close as possible to the 
> current behaviour
> Also isn't demangling by compliler-rt is better? mangled form is shorter.
as discussed, current descriptor has the demangled name because it's provided 
by clang frontend in `llvm.asan.globals`.

to keep this migration as close to the original as possible, keeping demangle 
of names in descriptors here, but added a TODO for follow-up work to instead 
demangle in the runtime.



Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1355-1356
   // at all, we assume it has dynamic initializer (in other TU).
-  //
-  // FIXME: Metadata should be attched directly to the global directly instead
-  // of being added to llvm.asan.globals.
-  return G->hasInitializer() && !GlobalsMD.get(G).IsDynInit;
+  if (G->hasSanitizerMetadata() && G->getSanitizerMetadata().IsDynInit)
+return false;
+

vitalybuka wrote:
> I believe previous was like this.
> if you want to change that lets do another patch. 
refactored it slightly, it's clear to me now (and IMHO much clearer to reason 
about, i suck at flipping multiple conditions in my head) that it's the same 
code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127911

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127911: Delete 'llvm.asan.globals' for global metadata.

2022-06-24 Thread Vitaly Buka via Phabricator via cfe-commits 
vitalybuka added a comment.

The rest is LGTM




Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:67-72
+  if (FsanitizeArgument.has(SanitizerKind::Address) && !Meta.NoAddress) {
 IsDynInit &= !CGM.isInNoSanitizeList(SanitizerKind::Address |
  SanitizerKind::KernelAddress,
  GV, Loc, Ty, "init");
 Meta.IsDynInit = IsDynInit;
   }

I recommend to move this change into another patch

and it should probably be:
Meta.IsDynInit &= IsDynInit && FsanitizeArgument.has(SanitizerKind::Address) && 
!Meta.NoAddress && !CGM.isInNoSanitizeLis;



Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:34
 #include "llvm/BinaryFormat/MachO.h"
+#include "llvm/Demangle/Demangle.h"
 #include "llvm/IR/Argument.h"

Please don't demangle in this patch, or keep as close as possible to the 
current behaviour
Also isn't demangling by compliler-rt is better? mangled form is shorter.



Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1355-1356
   // at all, we assume it has dynamic initializer (in other TU).
-  //
-  // FIXME: Metadata should be attched directly to the global directly instead
-  // of being added to llvm.asan.globals.
-  return G->hasInitializer() && !GlobalsMD.get(G).IsDynInit;
+  if (G->hasSanitizerMetadata() && G->getSanitizerMetadata().IsDynInit)
+return false;
+

I believe previous was like this.
if you want to change that lets do another patch. 



Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:2330
+ConstantInt::get(IntptrTy, Meta.IsDynInit),
+ConstantInt::get(IntptrTy, 0),
 ConstantExpr::getPointerCast(ODRIndicator, IntptrTy));

hctim wrote:
> vitalybuka wrote:
> > MD was fine, less changed lines
> done
Constant::getNullValue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127911

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127911: Delete 'llvm.asan.globals' for global metadata.

2022-06-24 Thread Mitch Phillips via Phabricator via cfe-commits 
hctim updated this revision to Diff 439912.
hctim marked 2 inline comments as done.
hctim added a comment.

Vitaly's comments - round 1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127911

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/lib/CodeGen/SanitizerMetadata.h
  clang/test/CodeGen/asan-globals.cpp
  clang/test/CodeGen/hwasan-globals.cpp
  clang/test/CodeGen/memtag-globals.cpp
  clang/test/CodeGen/sanitize-init-order.cpp
  compiler-rt/test/asan/TestCases/global-location-nodebug.cpp
  compiler-rt/test/asan/TestCases/global-location.cpp
  llvm/include/llvm/Transforms/Instrumentation/AddressSanitizer.h
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/test/Instrumentation/AddressSanitizer/global_metadata.ll
  llvm/test/Instrumentation/AddressSanitizer/global_metadata_array.ll
  llvm/test/Instrumentation/AddressSanitizer/global_with_comdat.ll
  llvm/test/Instrumentation/AddressSanitizer/instrument_global.ll
  llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
  llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
  llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
  llvm/tools/opt/NewPMDriver.cpp

Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -357,8 +357,6 @@
  ArrayRef) {
 AddressSanitizerOptions Opts;
 if (Name == "asan-pipeline") {
-  MPM.addPass(
-  RequireAnalysisPass());
   MPM.addPass(ModuleAddressSanitizerPass(Opts));
   return true;
 }
Index: llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
@@ -2,7 +2,7 @@
 ; Make sure asan does not instrument __sancov_gen_
 
 ; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S  | FileCheck %s
-; RUN: opt < %s -passes='module(require,sancov-module,asan-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S  | FileCheck %s
+; RUN: opt < %s -passes='module(sancov-module,asan-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S  | FileCheck %s
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 $Foo = comdat any
Index: llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
===
--- llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
+++ llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
@@ -10,13 +10,12 @@
 ; CHECK-SAME: linkonce_odr dso_local constant { [5 x i8], [27 x i8] }
 ; CHECK-SAME: { [5 x i8] c"asdf\00", [27 x i8] zeroinitializer }, comdat, align 32
 
-; CHECK: @"__asan_global_??_C@_04JIHMPGLA@asdf?$AA@" =
+; CHECK:  @"__asan_global_??_C@_04JIHMPGLA@asdf?$AA@" =
 ; CHECK-SAME: private global { i64, i64, i64, i64, i64, i64, i64, i64 }
 ; CHECK-SAME: { i64 ptrtoint ({ [5 x i8], [27 x i8] }* @"??_C@_04JIHMPGLA@asdf?$AA@" to i64),
-; CHECK-SAME:   i64 5, i64 32, 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@_04JIHMPGLA@asdf?$AA@"), align 64
+; CHECK-SAME:   i64 5, i64 32, i64 ptrtoint ([7 x i8]* @___asan_gen_.1 to i64), i64 ptrtoint ([8
+; CHECK-SAME:   x i8]* @___asan_gen_ to i64), i64 0, i64 0, i64 0 }, section ".ASAN$GL",
+; CHECK-SAME:   comdat($"??_C@_04JIHMPGLA@asdf?$AA@"), align 64
 
 ; ModuleID = 't.cpp'
 source_filename = "t.cpp"
@@ -35,11 +34,9 @@
 
 attributes #0 = { nounwind sanitize_address uwtable }
 
-!llvm.asan.globals = !{!0}
 !llvm.module.flags = !{!2, !3}
 !llvm.ident = !{!4}
 
-!0 = !{[5 x i8]* @"??_C@_04JIHMPGLA@asdf?$AA@", !1, !"", 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}
Index: llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
===
--- llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
+++ llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
@@ -2,17 +2,10 @@
 ; RUN: opt < %s -passes='asan-pipeline' -asan-mapping-scale=5 -S | FileCheck %s
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"

[PATCH] D127911: Delete 'llvm.asan.globals' for global metadata.

2022-06-24 Thread Mitch Phillips via Phabricator via cfe-commits 
hctim marked 2 inline comments as done.
hctim added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:2100
+GlobalVariable *Metadata = CreateMetadataGlobal(
+M, Initializer, llvm::demangle(std::string(G->getName(;
 

vitalybuka wrote:
> was this demanded before?
removed, think this got accidentally added during a sweep of demangling names 
for the produced metadata, which is necessary now that clang doesn't produce 
the info.



Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:2330
+ConstantInt::get(IntptrTy, Meta.IsDynInit),
+ConstantInt::get(IntptrTy, 0),
 ConstantExpr::getPointerCast(ODRIndicator, IntptrTy));

vitalybuka wrote:
> MD was fine, less changed lines
done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127911

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127911: Delete 'llvm.asan.globals' for global metadata.

2022-06-24 Thread Vitaly Buka via Phabricator via cfe-commits 
vitalybuka added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:2100
+GlobalVariable *Metadata = CreateMetadataGlobal(
+M, Initializer, llvm::demangle(std::string(G->getName(;
 

was this demanded before?



Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:2330
+ConstantInt::get(IntptrTy, Meta.IsDynInit),
+ConstantInt::get(IntptrTy, 0),
 ConstantExpr::getPointerCast(ODRIndicator, IntptrTy));

MD was fine, less changed lines


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127911

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127911: Delete 'llvm.asan.globals' for global metadata.

2022-06-24 Thread Mitch Phillips via Phabricator via cfe-commits 
hctim updated this revision to Diff 439880.
hctim added a comment.

Rebase on main / landed changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127911

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/lib/CodeGen/SanitizerMetadata.h
  clang/test/CodeGen/asan-globals.cpp
  clang/test/CodeGen/hwasan-globals.cpp
  clang/test/CodeGen/memtag-globals.cpp
  clang/test/CodeGen/sanitize-init-order.cpp
  compiler-rt/test/asan/TestCases/global-location-nodebug.cpp
  compiler-rt/test/asan/TestCases/global-location.cpp
  llvm/include/llvm/Transforms/Instrumentation/AddressSanitizer.h
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/test/Instrumentation/AddressSanitizer/global_metadata.ll
  llvm/test/Instrumentation/AddressSanitizer/global_metadata_array.ll
  llvm/test/Instrumentation/AddressSanitizer/global_with_comdat.ll
  llvm/test/Instrumentation/AddressSanitizer/instrument_global.ll
  llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
  llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
  llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
  llvm/tools/opt/NewPMDriver.cpp

Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -357,8 +357,6 @@
  ArrayRef) {
 AddressSanitizerOptions Opts;
 if (Name == "asan-pipeline") {
-  MPM.addPass(
-  RequireAnalysisPass());
   MPM.addPass(ModuleAddressSanitizerPass(Opts));
   return true;
 }
Index: llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
@@ -2,7 +2,7 @@
 ; Make sure asan does not instrument __sancov_gen_
 
 ; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S  | FileCheck %s
-; RUN: opt < %s -passes='module(require,sancov-module,asan-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S  | FileCheck %s
+; RUN: opt < %s -passes='module(sancov-module,asan-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S  | FileCheck %s
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 $Foo = comdat any
Index: llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
===
--- llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
+++ llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
@@ -10,13 +10,12 @@
 ; CHECK-SAME: linkonce_odr dso_local constant { [5 x i8], [27 x i8] }
 ; CHECK-SAME: { [5 x i8] c"asdf\00", [27 x i8] zeroinitializer }, comdat, align 32
 
-; CHECK: @"__asan_global_??_C@_04JIHMPGLA@asdf?$AA@" =
+; CHECK:  @"__asan_global_??_C@_04JIHMPGLA@asdf?$AA@" =
 ; CHECK-SAME: private global { i64, i64, i64, i64, i64, i64, i64, i64 }
 ; CHECK-SAME: { i64 ptrtoint ({ [5 x i8], [27 x i8] }* @"??_C@_04JIHMPGLA@asdf?$AA@" to i64),
-; CHECK-SAME:   i64 5, i64 32, 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@_04JIHMPGLA@asdf?$AA@"), align 64
+; CHECK-SAME:   i64 5, i64 32, i64 ptrtoint ([7 x i8]* @___asan_gen_.1 to i64), i64 ptrtoint ([8
+; CHECK-SAME:   x i8]* @___asan_gen_ to i64), i64 0, i64 0, i64 0 }, section ".ASAN$GL",
+; CHECK-SAME:   comdat($"??_C@_04JIHMPGLA@asdf?$AA@"), align 64
 
 ; ModuleID = 't.cpp'
 source_filename = "t.cpp"
@@ -35,11 +34,9 @@
 
 attributes #0 = { nounwind sanitize_address uwtable }
 
-!llvm.asan.globals = !{!0}
 !llvm.module.flags = !{!2, !3}
 !llvm.ident = !{!4}
 
-!0 = !{[5 x i8]* @"??_C@_04JIHMPGLA@asdf?$AA@", !1, !"", 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}
Index: llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
===
--- llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
+++ llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
@@ -2,17 +2,10 @@
 ; RUN: opt < %s -passes='asan-pipeline' -asan-mapping-scale=5 -S | FileCheck %s
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
 target triple =

[PATCH] D127911: Delete 'llvm.asan.globals' for global metadata.

2022-06-23 Thread Mitch Phillips via Phabricator via cfe-commits 
hctim added a comment.

In D127911#3605774 , @MaskRay wrote:

>> This saves about ~0.275% of the optimised clang binary.
>
> Worth clarifying a bit which -O level and whether -g is used.

`clang-15` binary, no `-g`, `-DLLVM_ENABLE_ASSERTIONS=ON 
-DCMAKE_BUILD_TYPE=Release`:

Before: 1041657808
After: 1024109680

Not sure what led to the original measurement, but seems like this saves ~1.7% 
of binary size for ASan. I'll update the description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127911

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127911: Delete 'llvm.asan.globals' for global metadata.

2022-06-23 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

> This saves about ~0.275% of the optimised clang binary.

Worth clarifying a bit which -O level and whether -g is used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127911

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127911: Delete 'llvm.asan.globals' for global metadata.

2022-06-23 Thread Mitch Phillips via Phabricator via cfe-commits 
hctim added a comment.

friendly ping @vitalybuka


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127911

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127911: Delete 'llvm.asan.globals' for global metadata.

2022-06-15 Thread Mitch Phillips via Phabricator via cfe-commits 
hctim updated this revision to Diff 437385.
hctim added a comment.

Small test fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127911

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/lib/CodeGen/SanitizerMetadata.h
  clang/test/CodeGen/asan-globals.cpp
  clang/test/CodeGen/hwasan-globals.cpp
  clang/test/CodeGen/memtag-globals.cpp
  clang/test/CodeGen/sanitize-init-order.cpp
  compiler-rt/test/asan/TestCases/global-location.cpp
  llvm/include/llvm/Transforms/Instrumentation/AddressSanitizer.h
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/test/Instrumentation/AddressSanitizer/global_metadata.ll
  llvm/test/Instrumentation/AddressSanitizer/global_metadata_array.ll
  llvm/test/Instrumentation/AddressSanitizer/global_with_comdat.ll
  llvm/test/Instrumentation/AddressSanitizer/instrument_global.ll
  llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
  llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
  llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
  llvm/tools/opt/NewPMDriver.cpp

Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -357,8 +357,6 @@
  ArrayRef) {
 AddressSanitizerOptions Opts;
 if (Name == "asan-pipeline") {
-  MPM.addPass(
-  RequireAnalysisPass());
   MPM.addPass(ModuleAddressSanitizerPass(Opts));
   return true;
 }
Index: llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
@@ -2,7 +2,7 @@
 ; Make sure asan does not instrument __sancov_gen_
 
 ; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S  | FileCheck %s
-; RUN: opt < %s -passes='module(require,sancov-module,asan-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S  | FileCheck %s
+; RUN: opt < %s -passes='module(sancov-module,asan-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S  | FileCheck %s
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 $Foo = comdat any
Index: llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
===
--- llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
+++ llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
@@ -10,13 +10,12 @@
 ; CHECK-SAME: linkonce_odr dso_local constant { [5 x i8], [27 x i8] }
 ; CHECK-SAME: { [5 x i8] c"asdf\00", [27 x i8] zeroinitializer }, comdat, align 32
 
-; CHECK: @"__asan_global_??_C@_04JIHMPGLA@asdf?$AA@" =
+; CHECK:  @"__asan_global_??_C@_04JIHMPGLA@asdf?$AA@" =
 ; CHECK-SAME: private global { i64, i64, i64, i64, i64, i64, i64, i64 }
 ; CHECK-SAME: { i64 ptrtoint ({ [5 x i8], [27 x i8] }* @"??_C@_04JIHMPGLA@asdf?$AA@" to i64),
-; CHECK-SAME:   i64 5, i64 32, 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@_04JIHMPGLA@asdf?$AA@"), align 64
+; CHECK-SAME:   i64 5, i64 32, i64 ptrtoint ([7 x i8]* @___asan_gen_.1 to i64), i64 ptrtoint ([8
+; CHECK-SAME:   x i8]* @___asan_gen_ to i64), i64 0, i64 0, i64 0 }, section ".ASAN$GL",
+; CHECK-SAME:   comdat($"??_C@_04JIHMPGLA@asdf?$AA@"), align 64
 
 ; ModuleID = 't.cpp'
 source_filename = "t.cpp"
@@ -35,11 +34,9 @@
 
 attributes #0 = { nounwind sanitize_address uwtable }
 
-!llvm.asan.globals = !{!0}
 !llvm.module.flags = !{!2, !3}
 !llvm.ident = !{!4}
 
-!0 = !{[5 x i8]* @"??_C@_04JIHMPGLA@asdf?$AA@", !1, !"", 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}
Index: llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
===
--- llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
+++ llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
@@ -2,17 +2,10 @@
 ; RUN: opt < %s -passes='asan-pipeline' -asan-mapping-scale=5 -S | FileCheck %s
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
 target triple = "x86_64-unknown-linux-gnu"
-@xxx = internal global i32 0, align 4  ; With dynamic initializer.
-@XXX

[PATCH] D127911: Delete 'llvm.asan.globals' for global metadata.

2022-06-15 Thread Mitch Phillips via Phabricator via cfe-commits 
hctim created this revision.
hctim added a reviewer: vitalybuka.
Herald added subscribers: Enna1, ormris, hiraditya.
Herald added a project: All.
hctim requested review of this revision.
Herald added projects: clang, Sanitizers, LLVM.
Herald added subscribers: llvm-commits, Sanitizers, cfe-commits.

Now that we have the sanitizer metadata that is actually on the global
variable, and now that we use debuginfo in order to do symbolization of
globals, we can delete the 'llvm.asan.globals' IR synthesis.

This patch deletes the 'location' part of the __asan_global that's
embedded in the binary as well, because it's unnecessary. This saves
about ~0.275% of the optimised clang binary.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127911

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/lib/CodeGen/SanitizerMetadata.h
  clang/test/CodeGen/asan-globals.cpp
  clang/test/CodeGen/hwasan-globals.cpp
  clang/test/CodeGen/memtag-globals.cpp
  clang/test/CodeGen/sanitize-init-order.cpp
  compiler-rt/test/asan/TestCases/global-location.cpp
  llvm/include/llvm/Transforms/Instrumentation/AddressSanitizer.h
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/test/Instrumentation/AddressSanitizer/global_metadata.ll
  llvm/test/Instrumentation/AddressSanitizer/global_metadata_array.ll
  llvm/test/Instrumentation/AddressSanitizer/global_with_comdat.ll
  llvm/test/Instrumentation/AddressSanitizer/instrument_global.ll
  llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
  llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
  llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
  llvm/tools/opt/NewPMDriver.cpp

Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -357,8 +357,6 @@
  ArrayRef) {
 AddressSanitizerOptions Opts;
 if (Name == "asan-pipeline") {
-  MPM.addPass(
-  RequireAnalysisPass());
   MPM.addPass(ModuleAddressSanitizerPass(Opts));
   return true;
 }
Index: llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
@@ -2,7 +2,7 @@
 ; Make sure asan does not instrument __sancov_gen_
 
 ; RUN: opt < %s -passes='module(sancov-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S  | FileCheck %s
-; RUN: opt < %s -passes='module(require,sancov-module,asan-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S  | FileCheck %s
+; RUN: opt < %s -passes='module(sancov-module,asan-module)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S  | FileCheck %s
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 $Foo = comdat any
Index: llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
===
--- llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
+++ llvm/test/Instrumentation/AddressSanitizer/win-string-literal.ll
@@ -10,13 +10,12 @@
 ; CHECK-SAME: linkonce_odr dso_local constant { [5 x i8], [27 x i8] }
 ; CHECK-SAME: { [5 x i8] c"asdf\00", [27 x i8] zeroinitializer }, comdat, align 32
 
-; CHECK: @"__asan_global_??_C@_04JIHMPGLA@asdf?$AA@" =
+; CHECK:  @"__asan_global_??_C@_04JIHMPGLA@asdf?$AA@" =
 ; CHECK-SAME: private global { i64, i64, i64, i64, i64, i64, i64, i64 }
 ; CHECK-SAME: { i64 ptrtoint ({ [5 x i8], [27 x i8] }* @"??_C@_04JIHMPGLA@asdf?$AA@" to i64),
-; CHECK-SAME:   i64 5, i64 32, 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@_04JIHMPGLA@asdf?$AA@"), align 64
+; CHECK-SAME:   i64 5, i64 32, i64 ptrtoint ([7 x i8]* @___asan_gen_.1 to i64), i64 ptrtoint ([8
+; CHECK-SAME:   x i8]* @___asan_gen_ to i64), i64 0, i64 0, i64 0 }, section ".ASAN$GL",
+; CHECK-SAME:   comdat($"??_C@_04JIHMPGLA@asdf?$AA@"), align 64
 
 ; ModuleID = 't.cpp'
 source_filename = "t.cpp"
@@ -35,11 +34,9 @@
 
 attributes #0 = { nounwind sanitize_address uwtable }
 
-!llvm.asan.globals = !{!0}
 !llvm.module.flags = !{!2, !3}
 !llvm.ident = !{!4}
 
-!0 = !{[5 x i8]* @"??_C@_04JIHMPGLA@asdf?$AA@", !1, !"", 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}
Index: llvm/test/Instrumentation/AddressSanitizer/instrument_initializer_metadata.ll
===
---