On Wed, 29 Jul 2020, Richard Biener wrote:

> On Wed, 29 Jul 2020, H.J. Lu wrote:
> 
> > On Wed, Jul 29, 2020 at 4:01 AM Richard Biener
> > <richard.guent...@gmail.com> wrote:
> > >
> > > On Fri, Jul 17, 2020 at 5:32 PM H.J. Lu via Gcc-patches
> > > <gcc-patches@gcc.gnu.org> wrote:
> > > >
> > > > On Fri, Jul 17, 2020 at 6:27 AM Richard Biener <rguent...@suse.de> 
> > > > wrote:
> > > > >
> > > > > On Fri, 17 Jul 2020, H.J. Lu wrote:
> > > > >
> > > > > > On Fri, Jul 17, 2020 at 12:08 AM Richard Biener <rguent...@suse.de> 
> > > > > > wrote:
> > > > > > >
> > > > > > > On Wed, 15 Jul 2020, Joseph Myers wrote:
> > > > > > >
> > > > > > > > On Wed, 15 Jul 2020, Richard Biener wrote:
> > > > > > > >
> > > > > > > > > But note one of the issues is that when not cross-compiling 
> > > > > > > > > we're
> > > > > > > > > using a single libiberty for target and host objects (likewise
> > > > > > > >
> > > > > > > > There shouldn't be a target libiberty, since commit
> > > > > > > > 8499116aa30a46993deff5acf73985df6b16fb8b (re PR 
> > > > > > > > regression/47836 (Some
> > > > > > > > Cross Compiler can't build target-libiberty or target-zlib), 
> > > > > > > > Wed Jun 22
> > > > > > > > 19:40:45 2011 +0000).  If something is causing target libiberty 
> > > > > > > > to be
> > > > > > > > built, that's a bug that should be fixed.
> > > > > > > >
> > > > > > > > > That said, giving configury an idea whether it configures for
> > > > > > > > > the host, the target or the build would be required here - 
> > > > > > > > > Joseph,
> > > > > > > > > is there an existing mechanism for example libiberty can use
> > > > > > > > > here?
> > > > > > > >
> > > > > > > > Makefile.def has some settings specific to host or build, e.g.
> > > > > > > >
> > > > > > > > build_modules= { module= libcpp;
> > > > > > > >                  extra_configure_flags='--disable-nls 
> > > > > > > > am_cv_func_iconv=no';};
> > > > > > > >
> > > > > > > > or
> > > > > > > >
> > > > > > > > host_modules= { module= libiberty; bootstrap=true;
> > > > > > > >                 
> > > > > > > > extra_configure_flags='@extra_host_libiberty_configure_flags@';};
> > > > > > >
> > > > > > > Ah, OK.  Looks like we should be able to add a
> > > > > > > @extra_target_cet_configure_flags@, funnel that to the 
> > > > > > > target_modules
> > > > > > > and keep CET disabled by default in the modules configury.
> > > > > > >
> > > > > > > Similarly (if HJ is correct) we'd need to add
> > > > > > > @extra_{host,build}_cet_configure_flags@ for the purpose of 
> > > > > > > lto-plugin
> > > > > > > which only has a host module (and for bootstrap host == build, so 
> > > > > > > it's
> > > > > > > shared there but we still have separate libiberties for 
> > > > > > > host/build...)
> > > > > > >
> > > > > >
> > > > > > We need -fcf-protection only on object files which will be dlopened 
> > > > > > on
> > > > > > CET enabled build and host.
> > > > >
> > > > > Why is there a distinction between dlopen and execution?  IIRC
> > > > > ld falls back to non-CET operation when dlopening a non-CET shared 
> > > > > object?
> > > >
> > > > BTW, ld.so refuses to dlopen a legacy shared object after CET has been 
> > > > enabled.
> > > > This behavior can be controlled when configuring glibc:
> > > >
> > > > '--enable-cet'
> > > > '--enable-cet=permissive'
> > > >      Enable Intel Control-flow Enforcement Technology (CET) support.
> > > >      When the GNU C Library is built with '--enable-cet' or
> > > >      '--enable-cet=permissive', the resulting library is protected with
> > > >      indirect branch tracking (IBT) and shadow stack (SHSTK).  When CET
> > > >      is enabled, the GNU C Library is compatible with all existing
> > > >      executables and shared libraries.  This feature is currently
> > > >      supported on i386, x86_64 and x32 with GCC 8 and binutils 2.29 or
> > > >      later.  Note that when CET is enabled, the GNU C Library requires
> > > >      CPUs capable of multi-byte NOPs, like x86-64 processors as well as
> > > >      Intel Pentium Pro or newer.  With '--enable-cet', it is an error to
> > > >      dlopen a non CET enabled shared library in CET enabled application.
> > > >      With '--enable-cet=permissive', CET is disabled when dlopening a
> > > >      non CET enabled shared library in CET enabled application.
> > >
> > > So getting back to this one of the issues is that --enable-cet is used
> > > for both GCC_CET_FLAGS and GCC_CET_HOST_FLAGS where
> > > for the host flag part I'd use --enable-cet=auto but for the target 
> > > library
> > > part I definitely want to know if --enable-cet cannot be honored.
> > >
> > > Your current patch would still prohibit a non-bootstrap build with a host
> > > compiler not supporting CET and requesting CET enabled target libraries,
> > > thus
> > >
> > > ../configure --enable-cet --disable-bootstrap
> > >
> > > would fail.  Shouldn't we - for the host part - simply treat 'yes' equal
> > > to 'auto'?  If not, then we should have --enable-cet-host.  Which would
> > > be somewhat misleading since cc1 isn't built CET enabled, just
> > > lto-plugin.so is, so better --enable-lto-plugin-cet?
> > >
> > > Thus I'd go with the simpler of both:
> > >
> > > diff --git a/config/cet.m4 b/config/cet.m4
> > > index d9608699cd5..fb4e4275413 100644
> > > --- a/config/cet.m4
> > > +++ b/config/cet.m4
> > > @@ -64,7 +64,7 @@ case "$host" in
> > >      save_CFLAGS="$CFLAGS"
> > >      CFLAGS="$CFLAGS -fcf-protection"
> > >      case "$enable_cet" in
> > > -      auto)
> > > +      auto|yes)
> > >         # Check if target supports multi-byte NOPs
> > >         # and if assembler supports CET insn.
> > >         AC_COMPILE_IFELSE(
> > > @@ -80,15 +80,6 @@ asm ("setssbsy");
> > >          [enable_cet=yes],
> > >          [enable_cet=no])
> > >         ;;
> > > -      yes)
> > > -       # Check if assembler supports CET.
> > > -       AC_COMPILE_IFELSE(
> > > -        [AC_LANG_PROGRAM(
> > > -         [],
> > > -         [asm ("setssbsy");])],
> > > -        [],
> > > -        [AC_MSG_ERROR([assembler with CET support is required for
> > > --enable-cet])])
> > > -       ;;
> > >      esac
> > >      CFLAGS="$save_CFLAGS"
> > >      ;;
> > >
> > > is that OK with you?
> > >
> > 
> > How about this one instead?
> 
> It still has
> 
> +            && (test ! -f ../stage_current \
> +                || test `cat ../stage_current` != "stage1"); then
> 
> which I find ugly since it depends on directory layout.
> 
> Thinking about the LTO plugin issue again I wonder in which case
> we end up using the stage1 lto-plugin?  Shouldn't stage2-gcc
> use the stage2 lto-plugin which should pass the CET test?  Thus,
> a extra_stage1_configure_flags=--disable-cet for the libiberty
> and lto-plugin modules should work, too, with Makefile.tpl
> getting taught about this via
> 
>           [+ IF extra_stage[+id+]_configure_flags +] \
>           [+extra_stage[+id+]_configure_flags+][+ ENDIF 
> extra_stage[+id+]_configure_flags +]
> 
> where it hopefully supports nested [+ +], otherwise some
> IF id == 1 and expanding [+extra_stage1_configure_flags+] only.
> 
> For the non-bootstrap case there's another place where
> extra_configure_flags is expanded which we might run into
> with --disable-bootstrap (not sure).
> 
> The other idea was to make sth like POSTSTAGE1 available
> in the environment so your test above can check for that
> instead.  There's already POSTSTAGE1_FLAGS_TO_PAS and
> STAGE1_FLAGS_TO_PASS (extra arguments to make), adding
> POSTSTAGE1=1 to POSTSTAGE1_FLAGS_TO_PASS might be enough.

I can't get the latter to work without more changes though,
the make arguments do not end up in the environment.

Note that any workable solution is fine with me, I just
don't feel comfortable approving the solution involving
../curr_stage and friends.  Joseph, would HJs latest
patch be OK technically?

Thanks,
Richard.

Reply via email to