[PATCH] D138095: [asan] Keep Itanium mangled names in global metadata

[Zequan Wu via Phabricator via llvm-commits]

zequanwu added a comment.

In D138095#3945215 <https://reviews.llvm.org/D138095#3945215>, @MaskRay wrote:

> The reverse of the combined commits is the following. Any chance you found the wrong culprit?
>
>   diff --git a/compiler-rt/lib/asan/asan_globals.cpp b/compiler-rt/lib/asan/asan_globals.cpp
>   index 2b4a9e01c8a3..b780128c9adb 100644
>   --- a/compiler-rt/lib/asan/asan_globals.cpp
>   +++ b/compiler-rt/lib/asan/asan_globals.cpp
>   @@ -151,4 +151,4 @@ static void CheckODRViolationViaIndicator(const Global *g) {
>            !IsODRViolationSuppressed(g->name))
>   -      ReportODRViolation(g, FindRegistrationSite(g), l->g,
>   -                         FindRegistrationSite(l->g));
>   +      ReportODRViolation(g, FindRegistrationSite(g),
>   +                         l->g, FindRegistrationSite(l->g));
>      }
>   diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp
>   index d505d96bd653..b223f6cd01e3 100644
>   --- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp
>   +++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp
>   @@ -51,13 +51,8 @@ const char *DemangleCXXABI(const char *name) {
>      // own demangler (libc++abi's implementation could be adapted so that
>   -  // it does not allocate). For now, we just call it anyway, and use
>   -  // InternalAlloc to prevent lsan error.
>   -  if (&__cxxabiv1::__cxa_demangle) {
>   -    if (char *demangled_name = __cxxabiv1::__cxa_demangle(name, 0, 0, 0)) {
>   -      size_t size = internal_strlen(demangled_name) + 1;
>   -      char *buf = (char *)InternalAlloc(size);
>   -      internal_memcpy(buf, demangled_name, size);
>   -      free(demangled_name);
>   -      return buf;
>   -    }
>   -  }
>   +  // it does not allocate). For now, we just call it anyway, and we leak
>   +  // the returned value.
>   +  if (&__cxxabiv1::__cxa_demangle)
>   +    if (const char *demangled_name =
>   +          __cxxabiv1::__cxa_demangle(name, 0, 0, 0))
>   +      return demangled_name;
>    
>   diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
>   index a4d4cc73c482..ff05454aa920 100644
>   --- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
>   +++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
>   @@ -2266,8 +2266,7 @@ bool ModuleAddressSanitizer::InstrumentGlobals(IRBuilder<> &IRB, Module &M,
>    
>   -    // The runtime library tries demangling symbol names in the descriptor but
>   -    // functionality like __cxa_demangle may be unavailable (e.g.
>   -    // -static-libstdc++). So we demangle the symbol names here.
>   -    std::string NameForGlobal = G->getName().str();
>   +    // TODO: Symbol names in the descriptor can be demangled by the runtime
>   +    // library. This could save ~0.4% of VM size for a private large binary.
>   +    std::string NameForGlobal = llvm::demangle(G->getName().str());
>        GlobalVariable *Name =
>   -        createPrivateGlobalForString(M, llvm::demangle(NameForGlobal),
>   +        createPrivateGlobalForString(M, NameForGlobal,
>                                         /*AllowMerging*/ true, kAsanGenPrefix);

I just manually double checked. At least this change causes these two tests(`symbolizer-function-offset-dladdr.cpp` and `symbolizer-dladdr.cpp`) to fail, not sure about `interface_symbols_darwin.cpp`.
I have a macbook and can help with debugging the tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138095