[libcxx-commits] [PATCH] D125268: [libc++abi] Refactor exception type demangling into a separate function
Louis Dionne via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Wed May 11 07:13:37 PDT 2022
ldionne added inline comments.
================
Comment at: libcxxabi/src/cxa_default_handlers.cpp:64-65
static_cast<const __shim_type_info*>(exception_header->exceptionType);
-#if !defined(LIBCXXABI_NON_DEMANGLING_TERMINATE)
- // Try to get demangled name of thrown_type
- int status;
char buf[1024];
- size_t len = sizeof(buf);
- const char* name = __cxa_demangle(thrown_type->name(), buf, &len, &status);
- if (status != 0)
- name = thrown_type->name();
-#else
- const char* name = thrown_type->name();
-#endif
+ char const* name = demangle(thrown_type->name(), buf);
// If the uncaught exception can be caught with std::exception&
----------------
urnathan wrote:
> ldionne wrote:
> > urnathan wrote:
> > > This isn't code you're changing the meaning of, but this is broken. the buffer passed to __cxa_demangle must come from malloc. If ever a thrown type demangles to more than 1024 chars, people are going to be sad.
> > Thank you, that is an excellent observation.
> thanks, but you're now going to leak. you need something like:
> ```
> if (name != thrown_type->name())
> std::free (name);
> ```
> somewhere. (I'm going to presume the optimizer can spot that's always false in the LIBCXXABI_NON_DEMANGLING_TERMINATE case).
>
> N.B. our demangler calls std::terminate in the event of memory exhaustion, rather than return null. I'm slowly trying to resolve that problem.
I know we're leaking, but I was thinking it didn't matter because we were terminating anyways. This is actually what we were doing previously if the demangled name was more than 1024 characters long. `__cxa_demangle` would `realloc` and the resulting pointer would be leaked (assuming `realloc` doesn't try to `free` if the given buffer wasn't allocated with `malloc`).
But hey, I can fix this easily with `unique_ptr` anyways.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125268/new/
https://reviews.llvm.org/D125268
More information about the libcxx-commits
mailing list