This is an archive of the discontinued LLVM Phabricator instance.

[libc++][libc++abi][CET] Support building libc++ and libc++abi with CET enabled.
Needs RevisionPublic

Authored by xiongji90 on Aug 5 2021, 5:13 AM.

Details

Reviewers
hjl.tools
compnerd
manojgupta
MaskRay
ldionne
Group Reviewers
Restricted Project
Restricted Project
Summary

Control-flow Enforcement Technology (CET), published by Intel, introduces indirect branch tracking (IBT) to guard all indirect 'jmp' and 'call'. The target of indirect 'jmp' or 'call' begin with 'endbr' instruction. Currently, CET has been implemented in x86 GNU Linux platforms.
In order to enable CET when building applications, we need to ensure:

  1. building source code with '-fcf-protection=full'
  2. all libraries linked must be CET enabled

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.

Diff Detail

Event Timeline

xiongji90 created this revision.Aug 5 2021, 5:13 AM
xiongji90 requested review of this revision.Aug 5 2021, 5:13 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptAug 5 2021, 5:13 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
xiongji90 added a comment.EditedAug 5 2021, 5:15 AM

We have built libc++ tests with CET enabled and ran those tests in a Fedora34 system with CET enabled. No fails found. However, for exception handling tests, we can't run them with llvm unwinder in CET enabled environment, after https://reviews.llvm.org/D105968/new/ is accepted, we can run libc++/libc++abi tests with llvm unwinder in CET enabled environment.

MaskRay accepted this revision.Aug 7 2021, 10:14 AM

Reasonable, but you need a libc++/libc++abi approver.

Wait. Can't the user just specify -fcf-protection=full in CMAKE_CXX_FLAGS?

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.

libcxx/CMakeLists.txt
87

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.

158

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.

ldionne requested changes to this revision.Aug 10 2021, 12:28 PM
ldionne added a subscriber: ldionne.

Wait. Can't the user just specify -fcf-protection=full in CMAKE_CXX_FLAGS?

This.

CET seems like a fairly narrow-use option, and I would rather you use a general option to enable it than add yet another configuration option to libc++. WDYT?

This revision now requires changes to proceed.Aug 10 2021, 12:28 PM

Wait. Can't the user just specify -fcf-protection=full in CMAKE_CXX_FLAGS?

Is there a way to specify flags for libc++ when building libc++ with llvm/clang.

CET requires all libraries to be built with CET flags. So the use case is to build libraries with CET enabled but not clang itself as the build host may not have CET support. However, this clang binary can still link the instrumented libraries for running on a device with CET support.
Use case example: -DLLVM_ENABLE_PROJECTS="clang;libcxx;libcxxabi;libunwind;compiler-rt". Runtimes build could use it as well to produce runtimes that are CET instrumented.

Hi, @Mordante , @ldionne , @manojgupta , @MaskRay
Since "-fcf-protection=full" is a compiling flag for C and CXX sources, I think users can add this flag via CMAKE_CXX_FLAGS. However, adding "-fcf-protection=full" to CMAKE_CXX_FLAGS will enable CET for all components we build.
As @manojgupta mentioned, in order to build application with CET enabled, we need to:

  1. compile the application's source code with "-fcf-protection=full"
  2. link the .o files generated form 1 with libraries which are CET enabled

We don't require the compiler used to build the application to be CET enabled. If we build libc++, libc++abi with other components such as clang++, lld, ldb...and add "-fcf-protection=full" to CMAKE_CXX_FLAGS, this flag will be used for all components which is not necessary.
Although we can use "-fcf-protection=full" to build all components whose sources don't include any assembly code, it may be not enough to enable CET for those components. For example, I once built clang, lld, lldb with "-fcf-protection=full", most tests passed in CET enable platform but we also had a few runtime errors most of which are related to JIT code. So, before resolving all issues, I think it is not safe to add "-fcf-protection=full" to CMAKE_CXX_FLAGS.
Currently, CET has been fully enabled in GNU toolchain (all tools such as compiler, linker, debugger, all libraries such libgcc, libstdc++...)while llvm toolchain has not. A practical approach is enabling the components one by one, the first step is enabling libc++, libc++abi, libunwind.
If we have a way to pass "-fcf-protection=full" to libc++ CXX flag, I think we can avoid the option LIBCXX_ENABLE_CET here.
Thanks very much.

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.

libcxx/CMakeLists.txt
87

Hi, @Mordante
Since we have not decided whether we need to add LIBCXX_ENABLE_CET option, it seems better for us to discuss and make final decision about the option firstly, if we finally decide to add this option, I will add some documentation as you required.
Thanks very much.

158

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.

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.

libcxx/CMakeLists.txt
158

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.

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.

Hi, @Mordante
Do you have some scenario that libcxxabi is used alone withouth libcxx? If there is no such scenario, I also agree that LIBCXXABI_ENABLE_CET is unnecessary. Maybe we can do following:

  1. Just use LIBCXX_ENABLE_CET in libcxxabi and remove LIBCXXABI_ENABLE_CET
  2. We can use such code in libcxxabi CMakeFile "option(LIBCXXABI_ENABLE_CET "Build libc++abi with CET enabled." ${LIBCXX_ENABLE_CET})"

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.

Do you mean we should add a way to set LIBCXX(ABI)_COMPILE_FLAGS in CMakeFile instead of adding the compiling flag to common CXX flag?

Thanks very much.

libcxx/CMakeLists.txt
158

Hi, @Mordante
Since CET is only implemented in Intel platform, I can check whether the compiler supports "-fcf-protection=full" and report error if it doesn't support.
Thanks very much.