[libcxx-commits] [PATCH] D92397: [libc++] Always define a key function for std::bad_function_call in the dylib

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a subscriber: dim.
ldionne added a comment.

IMO this is safe to land, however please add it to the ABI changelog.



================
Comment at: libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.exceptions.no_new_in_libcxx.abilist:3
 {'is_defined': False, 'name': '_ZNKSt13runtime_error4whatEv', 'type': 'FUNC'}
+{'is_defined': False, 'name': '_ZNKSt9exception4whatEv', 'type': 'FUNC'}
 {'is_defined': False, 'name': '_ZNSt11logic_errorD2Ev', 'type': 'FUNC'}
----------------
var-const wrote:
> ldionne wrote:
> > var-const wrote:
> > > This is from running `generate-cxx-abilist` on Linux (in a Docker container). I'm not sure about `exception::what`, but some symbols below (e.g. `_ZTVNSt3__120__time_get_c_storageIcEE`) seem clearly unrelated -- perhaps the ABI list wasn't regenerated on Linux at some point?
> > This is a bit strange indeed. I would suggest you don't add these unrelated symbols and see what the CI says. Normally, we're checking this list in the CI so it should never get out of sync.
> I copied this file from the CI now, as we discussed. Most extra symbols are gone, but this one persists. Should I add it to the changelog?
Yeah, add it to the changelog for now.

I didn't understand why this symbol is marked as needed, so I did some investigation. It turns out that `std::exception::what() const` is defined in libc++abi. Previously, libc++ would NOT have any dependency on `std::exception::what() const`, because all the exception classes would define their own `what()` method. However, with this change, the `bad_function_call` vtable inside `libc++.dylib` contains `std::exception::what() const` since it is not overriden in `bad_function_call`, leading to an undefined reference to that symbol in `libc++.dylib`.

I don't think this is an issue, because folks should already be linking against a libc++abi.so that provides the symbol. @dim I think you folks are using libcxxrt as an ABI library -- can you confirm that you're providing `std::exception::what() const` in that ABI library?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92397