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

Mitch Phillips via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 16 10:37:39 PST 2022


hctim accepted this revision as: hctim.
hctim added a comment.
This revision is now accepted and ready to land.

couple nits, otherwise lgtm



================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp:49-52
   // FIXME: __cxa_demangle aggressively insists on allocating memory.
   // There's not much we can do about that, short of providing our
   // own demangler (libc++abi's implementation could be adapted so that
+  // it does not allocate). For now, we just call it anyway, and use
----------------
Can you maybe update this entire fixme?

Looks like all the current callers would be tolerant to a function-local static buffer, but this feels like a gunfoot, so I guess we'll continue to leak the memory, unless you also want to go ahead and create an InternalAlloc-RAII-wrapper class and patch up the callers.

FIXME: This pointer currently leaks in the callers, but we specifically copy the demangled contents into an InternalAlloc so that LSan doesn't see a leak from the malloc() done by cxa_demangle.


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp:55
+  if (&__cxxabiv1::__cxa_demangle) {
+    if (char *demangled_name = __cxxabiv1::__cxa_demangle(name, 0, 0, 0)) {
+      size_t size = internal_strlen(demangled_name) + 1;
----------------
nit: if you're here, can you change these parameters to `nullptr`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138095



More information about the llvm-commits mailing list