Message ID | a7b9ab2109b5e9d16def6b2f1b706a46cc384469.1593612309.git.szabolcs.nagy@arm.com |
---|---|
State | Committed |
Commit | de9301c02e898fb20a609b459d81afda42f39c61 |
Headers | show |
Series | aarch64: branch protection support | expand |
On 01/07/2020 11:39, Szabolcs Nagy wrote: > When glibc is built with branch protection (i.e. with a gcc configured > with --enable-standard-branch-protection), all glibc binaries should > be BTI compatible and marked as such. > > It is easy to link BTI incompatible objects by accident and this is > silent currently which is usually not the expectation, so this is > changed into a link error. (There is no linker flag for failing on > BTI incompatible inputs so all warnings are turned into fatal errors > outside the test system when building glibc with branch protection.) > > Unfortunately, outlined atomic functions are not BTI compatible in > libgcc (PR libgcc/96001), so to build glibc with current gcc use > 'CC=gcc -mno-outline-atomics', this should be fixed in libgcc soon > and then glibc can be built and tested without such workarounds. Is the outlined atomic present in any current released gcc? If so should we add a configure handler to add -mno-outline-atomics for such cases? Besides that LGTM, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > sysdeps/aarch64/Makefile | 8 ++++++++ > sysdeps/aarch64/configure | 2 ++ > sysdeps/aarch64/configure.ac | 1 + > 3 files changed, 11 insertions(+) > > diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile > index 5ae8b082b0..7f82fc055e 100644 > --- a/sysdeps/aarch64/Makefile > +++ b/sysdeps/aarch64/Makefile > @@ -1,5 +1,13 @@ > long-double-fcts = yes > > +ifeq (yes,$(aarch64-bti)) > +# Mark linker output BTI compatible, it warns on non-BTI inputs. > +sysdep-LDFLAGS += -Wl,-z,force-bti > +# Make warnings fatal outside the test system. > +LDFLAGS-lib.so += -Wl,--fatal-warnings > +LDFLAGS-rtld += -Wl,-z,force-bti,--fatal-warnings > +endif > + > ifeq ($(subdir),elf) > sysdep-dl-routines += dl-bti > endif Ok. > diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure > index 70477a7fa5..c637540436 100644 > --- a/sysdeps/aarch64/configure > +++ b/sysdeps/aarch64/configure > @@ -210,6 +210,8 @@ EOF > fi > { $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_aarch64_bti" >&5 > $as_echo "$libc_cv_aarch64_bti" >&6; } > +config_vars="$config_vars > +aarch64-bti = $libc_cv_aarch64_bti" > if test $libc_cv_aarch64_bti = yes; then > $as_echo "#define HAVE_AARCH64_BTI 1" >>confdefs.h > > diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac > index 798f494740..2c2817514d 100644 > --- a/sysdeps/aarch64/configure.ac > +++ b/sysdeps/aarch64/configure.ac > @@ -36,6 +36,7 @@ EOF > libc_cv_aarch64_bti=yes > fi > rm -rf conftest.*]) > +LIBC_CONFIG_VAR([aarch64-bti], [$libc_cv_aarch64_bti]) > if test $libc_cv_aarch64_bti = yes; then > AC_DEFINE(HAVE_AARCH64_BTI) > fi > Ok.
The 07/06/2020 14:37, Adhemerval Zanella via Libc-alpha wrote: > On 01/07/2020 11:39, Szabolcs Nagy wrote: > > When glibc is built with branch protection (i.e. with a gcc configured > > with --enable-standard-branch-protection), all glibc binaries should > > be BTI compatible and marked as such. > > > > It is easy to link BTI incompatible objects by accident and this is > > silent currently which is usually not the expectation, so this is > > changed into a link error. (There is no linker flag for failing on > > BTI incompatible inputs so all warnings are turned into fatal errors > > outside the test system when building glibc with branch protection.) > > > > Unfortunately, outlined atomic functions are not BTI compatible in > > libgcc (PR libgcc/96001), so to build glibc with current gcc use > > 'CC=gcc -mno-outline-atomics', this should be fixed in libgcc soon > > and then glibc can be built and tested without such workarounds. > > Is the outlined atomic present in any current released gcc? If so > should we add a configure handler to add -mno-outline-atomics for > such cases? outlined atomics is supported for a while now (since gcc-9 i think) but it is now the default in gcc-10. i am testing patches to fix libgcc in gcc trunk and then backport to gcc-10 branch. my preference is to only support fixed gcc and use the -mno-outline-atomics as a temporary workaround so we can test the patches with gcc-10.1.0 it's not a good workaround for using the resulting toolchain for building user code with branch protection (which will have the same problem: silently dropping bti protection) so i think it's better to fail the build. > > Besides that LGTM, thanks. > > Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > > --- > > sysdeps/aarch64/Makefile | 8 ++++++++ > > sysdeps/aarch64/configure | 2 ++ > > sysdeps/aarch64/configure.ac | 1 + > > 3 files changed, 11 insertions(+) > > > > diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile > > index 5ae8b082b0..7f82fc055e 100644 > > --- a/sysdeps/aarch64/Makefile > > +++ b/sysdeps/aarch64/Makefile > > @@ -1,5 +1,13 @@ > > long-double-fcts = yes > > > > +ifeq (yes,$(aarch64-bti)) > > +# Mark linker output BTI compatible, it warns on non-BTI inputs. > > +sysdep-LDFLAGS += -Wl,-z,force-bti > > +# Make warnings fatal outside the test system. > > +LDFLAGS-lib.so += -Wl,--fatal-warnings > > +LDFLAGS-rtld += -Wl,-z,force-bti,--fatal-warnings > > +endif > > + > > ifeq ($(subdir),elf) > > sysdep-dl-routines += dl-bti > > endif > > Ok. > > > diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure > > index 70477a7fa5..c637540436 100644 > > --- a/sysdeps/aarch64/configure > > +++ b/sysdeps/aarch64/configure > > @@ -210,6 +210,8 @@ EOF > > fi > > { $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_aarch64_bti" >&5 > > $as_echo "$libc_cv_aarch64_bti" >&6; } > > +config_vars="$config_vars > > +aarch64-bti = $libc_cv_aarch64_bti" > > if test $libc_cv_aarch64_bti = yes; then > > $as_echo "#define HAVE_AARCH64_BTI 1" >>confdefs.h > > > > diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac > > index 798f494740..2c2817514d 100644 > > --- a/sysdeps/aarch64/configure.ac > > +++ b/sysdeps/aarch64/configure.ac > > @@ -36,6 +36,7 @@ EOF > > libc_cv_aarch64_bti=yes > > fi > > rm -rf conftest.*]) > > +LIBC_CONFIG_VAR([aarch64-bti], [$libc_cv_aarch64_bti]) > > if test $libc_cv_aarch64_bti = yes; then > > AC_DEFINE(HAVE_AARCH64_BTI) > > fi > > > > Ok. --
On 06/07/2020 15:01, Szabolcs Nagy wrote: > The 07/06/2020 14:37, Adhemerval Zanella via Libc-alpha wrote: >> On 01/07/2020 11:39, Szabolcs Nagy wrote: >>> When glibc is built with branch protection (i.e. with a gcc configured >>> with --enable-standard-branch-protection), all glibc binaries should >>> be BTI compatible and marked as such. >>> >>> It is easy to link BTI incompatible objects by accident and this is >>> silent currently which is usually not the expectation, so this is >>> changed into a link error. (There is no linker flag for failing on >>> BTI incompatible inputs so all warnings are turned into fatal errors >>> outside the test system when building glibc with branch protection.) >>> >>> Unfortunately, outlined atomic functions are not BTI compatible in >>> libgcc (PR libgcc/96001), so to build glibc with current gcc use >>> 'CC=gcc -mno-outline-atomics', this should be fixed in libgcc soon >>> and then glibc can be built and tested without such workarounds. >> >> Is the outlined atomic present in any current released gcc? If so >> should we add a configure handler to add -mno-outline-atomics for >> such cases? > > outlined atomics is supported for a while now (since > gcc-9 i think) but it is now the default in gcc-10. > > i am testing patches to fix libgcc in gcc trunk and > then backport to gcc-10 branch. > > my preference is to only support fixed gcc and use > the -mno-outline-atomics as a temporary workaround > so we can test the patches with gcc-10.1.0 > > it's not a good workaround for using the resulting > toolchain for building user code with branch protection > (which will have the same problem: silently dropping > bti protection) so i think it's better to fail the build. Right, I think it should be ok to sign it with a build failure.
diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile index 5ae8b082b0..7f82fc055e 100644 --- a/sysdeps/aarch64/Makefile +++ b/sysdeps/aarch64/Makefile @@ -1,5 +1,13 @@ long-double-fcts = yes +ifeq (yes,$(aarch64-bti)) +# Mark linker output BTI compatible, it warns on non-BTI inputs. +sysdep-LDFLAGS += -Wl,-z,force-bti +# Make warnings fatal outside the test system. +LDFLAGS-lib.so += -Wl,--fatal-warnings +LDFLAGS-rtld += -Wl,-z,force-bti,--fatal-warnings +endif + ifeq ($(subdir),elf) sysdep-dl-routines += dl-bti endif diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure index 70477a7fa5..c637540436 100644 --- a/sysdeps/aarch64/configure +++ b/sysdeps/aarch64/configure @@ -210,6 +210,8 @@ EOF fi { $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_aarch64_bti" >&5 $as_echo "$libc_cv_aarch64_bti" >&6; } +config_vars="$config_vars +aarch64-bti = $libc_cv_aarch64_bti" if test $libc_cv_aarch64_bti = yes; then $as_echo "#define HAVE_AARCH64_BTI 1" >>confdefs.h diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac index 798f494740..2c2817514d 100644 --- a/sysdeps/aarch64/configure.ac +++ b/sysdeps/aarch64/configure.ac @@ -36,6 +36,7 @@ EOF libc_cv_aarch64_bti=yes fi rm -rf conftest.*]) +LIBC_CONFIG_VAR([aarch64-bti], [$libc_cv_aarch64_bti]) if test $libc_cv_aarch64_bti = yes; then AC_DEFINE(HAVE_AARCH64_BTI) fi