[libcxx-commits] [PATCH] D107560: [libc++][libc++abi][CET] Support building libc++ and libc++abi with CET enabled.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

In D107560#2938901 <https://reviews.llvm.org/D107560#2938901>, @xiongji90 wrote:

> In D107560#2933130 <https://reviews.llvm.org/D107560#2933130>, @Mordante wrote:
>
>>> This patch is used to build libc++ and libc++abi with CET enabled. If developers want to build applications with CET enabled and use libc++ and libc++abi, they must link CET enabled libc++ and libc++abi libraries. We introduce 2 options: LIBCXX_ENABLE_CET and LIBCXXABI_ENABLE_CET to enable CET building. This patch also enables running all libcxx and libcxxabi tests with CET enabled.
>>
>> I wonder what happens if only one of the two libraries is build with the flag enabled. Will these libraries still be compatible but with CET disabled? Or will they be incompatible?
>
> Hi, @Mordante 
> Do you mean if we build libc+, libc++abi with CET enabled and use them in a non-CET environment? If so, they can still work.  After applying "-fcf-protection=full", "endbr32/64" instruction will be added to the beginning of all functions, the "endbr32/64" instruction will be regarded as "nop" instruction in non-CET environment.
> Thanks very much.

You've added two flags `LIBCXX_ENABLE_CET` and `LIBCXXABI_ENABLE_CET`. I'm more wondering about when the user enables only `LIBCXXABI_ENABLE_CET` will this work?

So I'm concerned that there are two flags, but it might be required that both have the same value.

libc++ and libc++abi already have their own internal `LIBCXX(ABI|)_COMPILE_FLAGS` in CMake. So maybe adding a way to set their initial value from CMake would be a more generic approach. This would make it easier for Intel and other vendors to use their own special flags without needing changes to libc++ and libc++abi.



================
Comment at: libcxx/CMakeLists.txt:158
 
+if (LIBCXX_ENABLE_CET AND MSVC)
+  message(FATAL_ERROR "libc++ CET support is not available for MSVC!")
----------------
xiongji90 wrote:
> Mordante wrote:
> > Can you also validate the build is on the x86 platform?
> > Question regarding x86, do you mean the 32-bit platform or both the 32-bit and 64-bit platform.
> > To me x86 means only the 32-bit platform.
> Hi, @Mordante 
> "Can you also validate the build is on the x86 platform?"
> 
> Do you mean we need to check whether we are building the library on a x86 platform?
> 
> "Question regarding x86, do you mean the 32-bit platform or both the 32-bit and 64-bit platform."
> 
> Sorry for the confusion, I mean both 32-bit and 64-bit platform and I have tested both 32-bit and 64-bit.
> Thanks very much.
> 
libc++ is available on more platforms than x86 and X86-64. So I think we should either warn for all invalid combinations or none. The test `target_add_compile_flags_if_supported` already validates whether the flag is supported.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107560