2016-08-31 16:37 GMT+03:00 Alexander Ivchenko <aivch...@gmail.com>:
> 2016-08-31 12:18 GMT+03:00 Ilya Enkovich <enkovich....@gmail.com>:
>> 2016-08-30 21:53 GMT+03:00 Alexander Ivchenko <aivch...@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.
>
> Are you sure about that? May be I missed something, buy when I do
>> gcc out-of-bounds.c -mmpx -fcheck-pointer-bounds -static -###
>
> I got: ... -whole-archive -lmpx --no-whole-archive -lpthread
> -lmpxwrappers --start-group -lgcc -lgcc_eh -lc --end-group ...
> So no guards for static case. Could you clarify?
>
When you use -static or -static-libmpx then MPX runtime static library
should be linked
in a similar way. In your current version you don't have guards for
-static but have them
for -static-libmpx. So I propose to use"
%{!static-libmpx:" MPX_LD_AS_NEEDED_GUARD_PUSH " ...
%{!static-libmpx:" MPX_LD_AS_NEEDED_GUARD_POP " ...
>
>
>>> 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.
>
> wrappers also are not linked to the binary with -as-needed. So if I
> remove guards from libmpxwrappers:
MPX runtime library has to be linked in even if we don't use any
symbol from it due
to its static constructors used to initilize MPX. For wrappers we
actually don't need to
link it if we don't have any wrapper used. I think you should see
wrappers linked
with --as-needed if you actually use some wrapped function in your test code.
BTW looks like we have wrong '--whole-archive' for -static-libmpxwrappers case.
There is inconsistency because we don't use --whole-archive when just
-static is passed.
Ilya
>>> gcc out-of-bounds.c -mmpx -fcheck-pointer-bounds -Wl,-as-needed -###
> ... -as-needed --push-state --no-as-needed -lmpx --pop-state
> -lmpxwrappers -z bndplt -lgcc --as-needed -lgcc_s --no-as-needed -lc
> -lgcc --as-needed -lgcc_ ...
>
>> ldd a.out
> linux-vdso.so.1 (0x00007ffd987cf000)
> libmpx.so.2 => not found
> libc.so.6 => /lib64/libc.so.6 (0x00007f0f27bf3000)
> /lib64/ld-linux-x86-64.so.2 (0x000055bfba052000)
>
>
>
>> 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....@gmail.com>:
>>>> 2016-08-25 12:27 GMT+03:00 Alexander Ivchenko <aivch...@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 "\
>>>>> .
