From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 117271 invoked by alias); 31 Aug 2016 20:29:14 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 117253 invoked by uid 89); 31 Aug 2016 20:29:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=aivchenk@gmail.com, aivchenkgmailcom, enkovich.gnu@gmail.com, enkovichgnugmailcom X-HELO: mail-ua0-f175.google.com Received: from mail-ua0-f175.google.com (HELO mail-ua0-f175.google.com) (209.85.217.175) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 31 Aug 2016 20:29:03 +0000 Received: by mail-ua0-f175.google.com with SMTP id q42so51067475uaq.1 for ; Wed, 31 Aug 2016 13:29:03 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=QeXqyHQZoa55RGmGSzpImyxxrG9ybbrUXusJH5YDh4k=; b=FO0MxURD//YIVAEsMb4lFhuSwSFXjecn8DKSUZks77l2FmR+VItUzZ/+rw9t96ET+J qODp0Fr6HFp5ZsqozncGi15fX0R4Nl0cnh3EC0aFADT4AcIfOxDyrjZ4VDpNk3KyIEDE JKswzsptkrzM9m0YBc1e/gLlaRWJFRfMr2IHn9Gb8iE18RY/QFUqX0lM50LJrqRtsqyN Nu+30MeArXr9gYhTk4jvMu7jHxZOYTDxnzbg3Abnl4z8DfK2xUsY0TBGQLbTNjTdvV3f jxWwaAOl0efUAWSy+J6l6HqHZxm0UIWkdhyxI+3ak4mDz58dYqg8mFeL0oWrVnFF3ZPb qx+g== X-Gm-Message-State: AE9vXwP5PezvF/ZribqL5x80mNEMbkEEBJwGsXlfsRcLM6PBBMcTghsiRUphPVOtsvL1InVdIGn+GO7tshemxw== X-Received: by 10.176.6.164 with SMTP id g33mr3995324uag.78.1472675341421; Wed, 31 Aug 2016 13:29:01 -0700 (PDT) MIME-Version: 1.0 Received: by 10.176.80.28 with HTTP; Wed, 31 Aug 2016 13:29:00 -0700 (PDT) In-Reply-To: References: From: Ilya Enkovich Date: Wed, 31 Aug 2016 20:29:00 -0000 Message-ID: Subject: Re: [MPX] Fix for PR77267 To: Alexander Ivchenko Cc: GCC Patches , "H.J. Lu" Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2016-08/txt/msg02160.txt.bz2 2016-08-31 16:37 GMT+03:00 Alexander Ivchenko : > 2016-08-31 12:18 GMT+03:00 Ilya Enkovich : >> 2016-08-30 21:53 GMT+03:00 Alexander Ivchenko : >>> 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 : >>>> 2016-08-25 12:27 GMT+03:00 Alexander Ivchenko : >>>>> 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 "\ >>>>> .