[PATCH] D72222: [Driver][CodeGen] Add -fpatchable-function-entry=N[,0]

Szabolcs Nagy via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 24 02:05:30 PST 2020


nsz added a comment.

In D72222#1837536 <https://reviews.llvm.org/D72222#1837536>, @MaskRay wrote:

> A -DLLVM_ENABLE_ASSERTIONS=on build is required to trigger the assertion failure. My `make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- HOSTGCC=gcc CC=~/llvm/ReleaseAssert/bin/clang LD=~/llvm/ReleaseAssert/bin/ld.lld O=/tmp/arm64 allmodconfig all -j 30` build succeeded. Now I will be in favor of pushing the bugfix to release/10.x .
>
> Not clear about -fpatchable-function-entry=N,M where M>0 (D73070 <https://reviews.llvm.org/D73070>, D73071 <https://reviews.llvm.org/D73071>, D73072 <https://reviews.llvm.org/D73072>). For completeness, I'd like them to be included in release/10.x so we will not have a clang 10 that does not work with M>0.
>
> For `BTI c` issue, GCC has several releases that do not work with -mbranch-protection=bti. The Linux kernel has to develop some mechanism to detect the undesirable placement of `bti c`, if there are -mbranch-protection=bti users. So I don't think that inconsistency in clang 10.0.0 with GCC will be a problem.


note that the gcc fix will be in the gcc-10 release (it got accepted),
and soon backported to gcc-9 (the first release with bti support).
(only aarch64 bti was fixed, that's what affected linux)

the gcc fix keeps bti outside of the patch area in the M=0 case,
which makes the current linux *runtime* patching code simpler.

the choice was made because this is important if there is a mix
of bti and non-bti functions which can happen if

1. compiler detects not_called_directly(f) and omits bti or
2. individual attributes on functions turn on/off bti

with the llvm patch the *runtime* code would have to detect
which patch area has bti and which not to skip over. (fine for
future M>0 usage, but current M=0 case should not be complex,
i.e. linux will have to disable bti+ftrace on llvm if this patch is
accepted until it has the complex runtime code. and the patch
may make static detection of the compiler behaviour harder as
it fails differently than old gcc-9).

i did not look deeply into llvm but i'd expect some hook for
printing the function label, which can print the bti before
the patch area based on some global flag (an ugly hack, but
allows consistent behaviour across the compilers)

i hope this makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72222





More information about the cfe-commits mailing list