diff options
author | Louis Dionne <ldionne@apple.com> | 2020-05-15 14:54:27 -0400 |
---|---|---|
committer | Louis Dionne <ldionne@apple.com> | 2020-06-26 00:47:48 -0400 |
commit | 53623d4aa7105dd431b388c6fddea3a71b52ab14 (patch) | |
tree | 5c1f0d9f717fe4bcaed956680767ac59ad07c668 /clang/lib/Basic/Module.cpp | |
parent | 8013ce4490225faf8aae911ee88087ce445e604c (diff) | |
download | llvm-53623d4aa7105dd431b388c6fddea3a71b52ab14.zip llvm-53623d4aa7105dd431b388c6fddea3a71b52ab14.tar.gz llvm-53623d4aa7105dd431b388c6fddea3a71b52ab14.tar.bz2 |
[libc++] Always generate a __config_site header
Before this patch, the __config_site header was only generated when at
least one __config_site macro needed to be defined. This lead to two
different code paths in how libc++ is configured, depending on whether
a __config_site header was generated or not. After this patch, the
__config_site is always generated, but it can be empty in case there
are no macros to define in it.
More context on why this change is important
--------------------------------------------
In addition to being confusing, this double-code-path situation lead to
broken code being checked in undetected in 2405bd689815, which introduced
the LIBCXX_HAS_MERGED_TYPEINFO_NAMES_DEFAULT CMake setting. Specifically,
the _LIBCPP_HAS_MERGED_TYPEINFO_NAMES_DEFAULT <__config_site> macro was
supposed NOT to be defined unless LIBCXX_HAS_MERGED_TYPEINFO_NAMES_DEFAULT
was specified explicitly on the CMake command line. Instead, what happened
is that it was defined to 0 if it wasn't specified explicitly and a
<__config_site> header was generated. And defining that macro to 0 had
the important effect of using the non-unique RTTI comparison implementation,
which changes the ABI.
This change in behavior wasn't noticed because the <__config_site> header
is not generated by default. However, the Apple configuration does cause
a <__config_site> header to be generated, which lead to the wrong RTTI
implementation being used, and to https://llvm.org/PR45549. We came close
to an ABI break in the dylib, but were saved due to a downstream-only
change that overrode the decision of the <__config_site> for the purpose
of RTTI comparisons in libc++abi. This is an incredible luck that we should
not rely on ever again.
While the problem itself was fixed with 2464d8135e2a by setting
LIBCXX_HAS_MERGED_TYPEINFO_NAMES_DEFAULT explicitly in the Apple
CMake cache and then in d0fcdcd28f95 by making the setting less
brittle, the point still is that we should have had a single code
path from the beginning. Unlike most normal libraries, the macros
that configure libc++ are really complex, there's a lot of them and
they control important properties of the C++ runtime. There must be
a single code path for that, and it must be simple and robust.
Differential Revision: https://reviews.llvm.org/D80927
Diffstat (limited to 'clang/lib/Basic/Module.cpp')
0 files changed, 0 insertions, 0 deletions