[libcxx-commits] [PATCH] D149092: [libcxxabi] disable lib{std}c++ assertions

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

In D149092#4310328 <https://reviews.llvm.org/D149092#4310328>, @nickdesaulniers wrote:

> Thanks for taking a look!
>
> In D149092#4310156 <https://reviews.llvm.org/D149092#4310156>, @philnik wrote:
>
>> Are you sure there is no way around this? It seems like a bad idea to disable assertions when the user specifically requested them. These symbols are also provided by the time the program is run, so it should (at least theoretically) be possible to resolve the symbols then, since AFAIK the symbols are loaded lazily.
>
> Is libcxxabi _always_ linked against libcxx?  Is there a linker flag to say "an undefined reference to `std::__1::__libcpp_verbose_abort(char const*, ...)` is not an error?"
>
> https://reviews.llvm.org/D149092#4293607 proposes using an `alias` attribute to alias the symbol `__libcpp_verbose_abort` to `abort_message` (they would have the same implementation anyways).  I don't know if that's portable; IIRC alias requires a mangled name and IIRC platforms using mach-o add an underscore prefix? Also, I don't know if alias is an ELF only thing.
>
> I could also just implement `std::__1::__libcpp_verbose_abort(char const*, ...)` in terms of libcxxabi's `abort_message`. It kind of sucks to have one variadic function call another variadic function since the args are simply forwarded.
>
> `__libcpp_verbose_abort` is defined as a weak symbol in libcxx/src/verbose_abort.cpp. I don't think I'd want to provide a strong symbol in libcxxabi. Perhaps I should provide a definition of `__libcpp_verbose_abort` in libcxxabi/src/abort_message.cpp?
>
>   diff --git a/libcxxabi/src/abort_message.cpp b/libcxxabi/src/abort_message.cpp
>   index 859a5031b93f..79774fe1b3af 100644
>   --- a/libcxxabi/src/abort_message.cpp
>   +++ b/libcxxabi/src/abort_message.cpp
>   @@ -77,3 +77,14 @@ void abort_message(const char* format, ...)
>    
>        abort();
>    }
>   +
>   +#if defined(_LIBCPP_ENABLE_ASSERTIONS) && defined(_LIBCPP_BEGIN_NAMESPACE_STD)
>   +_LIBCPP_BEGIN_NAMESPACE_STD
>   +void __libcpp_verbose_abort (const char *fmt, ...) {
>   +  va_list list;
>   +  va_start(list, fmt);
>   +  abort_message("%s", fmt);
>   +  va_end(list);
>   +}
>   +_LIBCPP_END_NAMESPACE_STD
>   +#endif
>
> IDK

I also can't tell you what is the right thing to do here. I think @ldionne may want to take a look. I think it would make sense to move it into libc++abi, but that wouldn't work when not linking against it, so I'm not sure this is the right call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149092