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

Sun Aug 8 02:35:50 PDT 2021

Mordante added a comment.

> 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?

Is there a risk that this option breaks working code? If so I think it would be good to add a CI job to validate this build. This can be done by modifying `run-buildbot` and `buildkite-pipeline.yml`, both in the directory `libcxx/utils/ci/`.

Note I'm not familiar with libc++abi, so didn't review that part of the changes.

================
Comment at: libcxx/CMakeLists.txt:87
 option(LIBCXX_ENABLE_ASSERTIONS "Enable assertions independent of build mode." OFF)
+option(LIBCXX_ENABLE_CET "Build libc++ with CET enabled." OFF)
 option(LIBCXX_ENABLE_SHARED "Build libc++ as a shared library." ON)
----------------
This needs more documentation. CET isn't a well known feature. Please also update `libcxx/docs/BuildingLibcxx.rst` regarding this feature. The comment for this patch already contains a lot of useful information, that I would like to see here.

================
Comment at: libcxx/CMakeLists.txt:158

+if (LIBCXX_ENABLE_CET AND MSVC)
+  message(FATAL_ERROR "libc++ CET support is not available for MSVC!")
----------------
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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107560