Re: [MPX] Fix for PR77267

[Ilya Enkovich <enkovich dot gnu at gmail dot com>]

2016-08-30 21:53 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
> Would something like that count?
>
> I did not do the warning thing, cause the problem only appears when
> you provide the -Wl,-as-needed option to the linker.
> (In all other cases warning would be redundant). Are we able to check
> that on runtime?
>
>
> diff --git a/gcc/config.in b/gcc/config.in
> index fc3321c..a736de3 100644
> --- a/gcc/config.in
> +++ b/gcc/config.in
> @@ -1538,6 +1538,12 @@
>  #endif
>
>
> +/* Define if your linker supports --push-state/--pop-state */
> +#ifndef USED_FOR_TARGET
> +#undef HAVE_LD_PUSHPOPSTATE_SUPPORT
> +#endif
> +
> +
>  /* Define if your linker links a mix of read-only and read-write sections into
>     a read-write section. */
>  #ifndef USED_FOR_TARGET
> diff --git a/gcc/config/i386/linux-common.h b/gcc/config/i386/linux-common.h
> index 4b9910f..6aa195d 100644
> --- a/gcc/config/i386/linux-common.h
> +++ b/gcc/config/i386/linux-common.h
> @@ -79,13 +79,23 @@ along with GCC; see the file COPYING3.  If not see
>  #endif
>  #endif
>
> +#ifdef HAVE_LD_PUSHPOPSTATE_SUPPORT
> +#define MPX_LD_AS_NEEDED_GUARD_PUSH "--push-state --no-as-needed"
> +#define MPX_LD_AS_NEEDED_GUARD_POP "--pop-state"
> +#else
> +#define MPX_LD_AS_NEEDED_GUARD_PUSH ""
> +#define MPX_LD_AS_NEEDED_GUARD_POP ""
> +#endif
> +
>  #ifndef LIBMPX_SPEC
>  #if defined(HAVE_LD_STATIC_DYNAMIC)
>  #define LIBMPX_SPEC "\
>  %{mmpx:%{fcheck-pointer-bounds:\
>      %{static:--whole-archive -lmpx --no-whole-archive" LIBMPX_LIBS "}\
>      %{!static:%{static-libmpx:" LD_STATIC_OPTION " --whole-archive}\
> -    -lmpx %{static-libmpx:--no-whole-archive " LD_DYNAMIC_OPTION \
> +    " MPX_LD_AS_NEEDED_GUARD_PUSH  " -lmpx " MPX_LD_AS_NEEDED_GUARD_POP "\
> +    %{static-libmpx:--no-whole-archive "\
> +    LD_DYNAMIC_OPTION \

Looks like you add guards for both static-libmpx and dynamic linking cases.
You shouldn't need it for static-libmpx case.

>      LIBMPX_LIBS "}}}}"
>  #else
>  #define LIBMPX_SPEC "\
> @@ -99,7 +109,8 @@ along with GCC; see the file COPYING3.  If not see
>  %{mmpx:%{fcheck-pointer-bounds:%{!fno-chkp-use-wrappers:\
>      %{static:-lmpxwrappers}\
>      %{!static:%{static-libmpxwrappers:" LD_STATIC_OPTION " --whole-archive}\
> -    -lmpxwrappers %{static-libmpxwrappers:--no-whole-archive "\
> +    " MPX_LD_AS_NEEDED_GUARD_PUSH " -lmpxwrappers "
> MPX_LD_AS_NEEDED_GUARD_POP "\
> +    %{static-libmpxwrappers:--no-whole-archive "\
>      LD_DYNAMIC_OPTION "}}}}}"

I believe wrappers should work fine with --as-needed and don't need
this guard. Otherwise looks OK.

Ilya

>  #else
>  #define LIBMPXWRAPPERS_SPEC "\
> diff --git a/gcc/configure b/gcc/configure
> index 871ed0c..0eeee94 100755
> --- a/gcc/configure
> +++ b/gcc/configure
> @@ -29609,6 +29609,30 @@ fi
>  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ld_bndplt_support" >&5
>  $as_echo "$ld_bndplt_support" >&6; }
>
> +# Check linker supports '--push-state'/'--pop-state'
> +ld_pushpopstate_support=no
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking linker
> --push-state/--pop-state options" >&5
> +$as_echo_n "checking linker --push-state/--pop-state options... " >&6; }
> +if test x"$ld_is_gold" = xno; then
> +  if test $in_tree_ld = yes ; then
> +    if test "$gcc_cv_gld_major_version" -eq 2 -a
> "$gcc_cv_gld_minor_version" -ge 25 -o "$gcc_cv_gld_major_version" -gt
> 2; then
> +      ld_pushpopstate_support=yes
> +    fi
> +  elif test x$gcc_cv_ld != x; then
> +    # Check if linker supports --push-state/--pop-state options
> +    if $gcc_cv_ld --help 2>/dev/null | grep -- '--push-state' > /dev/null; then
> +      ld_pushpopstate_support=yes
> +    fi
> +  fi
> +fi
> +if test x"$ld_pushpopstate_support" = xyes; then
> +
> +$as_echo "#define HAVE_LD_PUSHPOPSTATE_SUPPORT 1" >>confdefs.h
> +
> +fi
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ld_pushpopstate_support" >&5
> +$as_echo "$ld_pushpopstate_support" >&6; }
> +
>  # Configure the subdirectories
>  # AC_CONFIG_SUBDIRS($subdirs)
>
> diff --git a/gcc/configure.ac b/gcc/configure.ac
> index 241e82d..93af766 100644
> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -6237,6 +6237,27 @@ if test x"$ld_bndplt_support" = xyes; then
>  fi
>  AC_MSG_RESULT($ld_bndplt_support)
>
> +# Check linker supports '--push-state'/'--pop-state'
> +ld_pushpopstate_support=no
> +AC_MSG_CHECKING(linker --push-state/--pop-state options)
> +if test x"$ld_is_gold" = xno; then
> +  if test $in_tree_ld = yes ; then
> +    if test "$gcc_cv_gld_major_version" -eq 2 -a
> "$gcc_cv_gld_minor_version" -ge 25 -o "$gcc_cv_gld_major_version" -gt
> 2; then
> +      ld_pushpopstate_support=yes
> +    fi
> +  elif test x$gcc_cv_ld != x; then
> +    # Check if linker supports --push-state/--pop-state options
> +    if $gcc_cv_ld --help 2>/dev/null | grep -- '--push-state' > /dev/null; then
> +      ld_pushpopstate_support=yes
> +    fi
> +  fi
> +fi
> +if test x"$ld_pushpopstate_support" = xyes; then
> +  AC_DEFINE(HAVE_LD_PUSHPOPSTATE_SUPPORT, 1,
> + [Define if your linker supports --push-state/--pop-state])
> +fi
> +AC_MSG_RESULT($ld_pushpopstate_support)
> +
>  # Configure the subdirectories
>  # AC_CONFIG_SUBDIRS($subdirs)
>
>
> 2016-08-29 13:09 GMT+03:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
>> 2016-08-25 12:27 GMT+03:00 Alexander Ivchenko <aivchenk@gmail.com>:
>>> The attached patched fixes the usage of MPX in presence of
>>> "-Wl,-as-needed" option. 'make checked' on MPX-enabled machine.
>>>
>>> "--push-state" and "--pop-state" are not supported by gold at the
>>> moment. But that's OK because using MPX with gold only recommended in
>>> static build.
>>
>> What will happen if compiler is configured to use gold by default?
>> Also is there any chance
>> we may use old linker with no push-state/pop-state support? I wonder
>> if you need to make
>> a new configure check and emit a warning similar to what is done for
>> "-z bndplt" option.
>>
>> Thanks,
>> Ilya
>>
>>>
>>> Would that be OK for trunk?
>>>
>>> diff --git a/gcc/config/i386/linux-common.h b/gcc/config/i386/linux-common.h
>>> index dd79ec6..1928b4e 100644
>>> --- a/gcc/config/i386/linux-common.h
>>> +++ b/gcc/config/i386/linux-common.h
>>> @@ -70,7 +70,9 @@ along with GCC; see the file COPYING3.  If not see
>>>  %{mmpx:%{fcheck-pointer-bounds:\
>>>      %{static:--whole-archive -lmpx --no-whole-archive" LIBMPX_LIBS "}\
>>>      %{!static:%{static-libmpx:" LD_STATIC_OPTION " --whole-archive}\
>>> -    -lmpx %{static-libmpx:--no-whole-archive " LD_DYNAMIC_OPTION \
>>> +    %{!fuse-ld=gold:--push-state --no-as-needed} -lmpx\
>>> +    %{!fuse-ld=gold:--pop-state} %{static-libmpx:--no-whole-archive "\
>>> +    LD_DYNAMIC_OPTION \
>>>      LIBMPX_LIBS "}}}}"
>>>  #else
>>>  #define LIBMPX_SPEC "\
>>> @@ -84,7 +86,8 @@ along with GCC; see the file COPYING3.  If not see
>>>  %{mmpx:%{fcheck-pointer-bounds:%{!fno-chkp-use-wrappers:\
>>>      %{static:-lmpxwrappers}\
>>>      %{!static:%{static-libmpxwrappers:" LD_STATIC_OPTION " --whole-archive}\
>>> -    -lmpxwrappers %{static-libmpxwrappers:--no-whole-archive "\
>>> +    %{!fuse-ld=gold:--push-state --no-as-needed} -lmpxwrappers\
>>> +    %{!fuse-ld=gold:--pop-state} %{static-libmpxwrappers:--no-whole-archive "\
>>>      LD_DYNAMIC_OPTION "}}}}}"
>>>  #else
>>>  #define LIBMPXWRAPPERS_SPEC "\
>>> .