[libcxx-commits] [libcxx] [libc++][hardening] Rework how the assertion handler can be overridden. (PR #77883)

[Konstantin Varlamov via libcxx-commits]

var-const wrote:

> After reading this for a bit, putting the macro in __config_site probably doesn't work, due to circular includes.
> 
> Can you say a bit more about which problem this solves?
> 
> Also, since the file is only getting copied, why is it called `.in`?
> 
> I think on our end, for layering reasons, we'll have to provide a version of this file that calls a weak symbol that calls std::abort and then provide a strong override in our base library that calls our dump override (…since a small number of test binaries don't link in our base library). So we'll basically have to reinvent what's in libc++. Not the end of the world, but a bunch of busywork, and it's not super clear why this approach is better to me. (It also doesn't _have_ to be clear to me, of course! I'm still curious why the change was made.)

A big part of the motivation for this change is to make sure that the codegen can come down to a single instruction (intended to be `__builtin_trap`, or `__builtin_verbose_trap` once that becomes available). This is important for code size and was raised in the RFC; we also know some users that need this. Unfortunately, while the mechanism we had originally that allowed for link-time overriding is convenient, it also results in generating a function call that can't be optimized away in the general case (the fact that `verbose_abort` is a variadic function doesn't help, either, the codegen for variadics isn't great). This imposes a code size penalty even on users who don't make use of overriding (or else would force them to do the macro redefinition).

Also, we feel that this should primarily be controlled by vendors, not users in most cases, and thus should be a configuration-time mechanism. If we ship libc++ on a platform where we can't afford to call `__verbose_abort`, without configuration-time customization every user would have to redefine the `_LIBCPP_VERBOSE_ABORT` macro, which is really cumbersome.

We're really sorry it creates additional churn for you, but I think the most important thing is to make sure there is no downgrade in functionality, i.e., you would still be able to use the overrides you're currently using. It's a little hard to balance, but basically we're trying to impose the minimal performance penalty by default while still allowing for full customizability for platforms and projects that need it.

> Also, since the file is only getting copied, why is it called `.in`?

I don't feel very strongly about it, but I felt that the name makes it more obvious that this header is used as an input to the build configuration and not included (or meant to be included) as is from its directory. Also, while we don't currently plan to use e.g. variable expansion, I think it's more of an implementation detail whether the header is copied as-is or somehow modified by build rules.

https://github.com/llvm/llvm-project/pull/77883