[PATCH] D124493: Move Sanitizer metadata to be on-GlobalValue.

Vitaly Buka via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 2 15:28:45 PDT 2022


vitalybuka added a comment.

In D124493#3477432 <https://reviews.llvm.org/D124493#3477432>, @filcab wrote:

> Hi @hctim, thanks for the patch.
> I have one question, though. Do you really need to remove the information you removed?
> Some people might be testing ASan binaries without access to debug info (because it's been stripped or it's not been loaded and is not available for the sanitizers to get symbols from, etc), and having the full global information would be useful for those reports.
>
> Can this be done by having a flag to make clang not emit the source information + the sanitizer patch to get the line and column?
> That way we could keep the current behaviour of emitting more info, and you'd still get your savings + use of debuginfo.
>
> Thank you,
> Filipe

It's going to cost us supporting two implementations of the same thing? It would be nice to pick a single approach if existing behavior is not critical.



================
Comment at: clang/lib/CodeGen/CMakeLists.txt:83
   PatternInit.cpp
-  SanitizerMetadata.cpp
+  SanitizerMetadataFactory.cpp
   SwiftCallingConv.cpp
----------------
why do you need this rename?

depending on your response you should either:
1. revert
2. rename as NFC in a separate patch and rebase the rest


================
Comment at: clang/lib/CodeGen/SanitizerMetadataFactory.h:6
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
----------------
can you please either revert 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124493



More information about the llvm-commits mailing list