From patchwork Wed Jul 5 17:23:15 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tulio Magno Quites Machado Filho X-Patchwork-Id: 21441 Received: (qmail 74667 invoked by alias); 5 Jul 2017 17:25:43 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 74636 invoked by uid 89); 5 Jul 2017 17:25:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy=H*r:sendmail, H*r:sSMTP, H*r:emulation, 16, 8 X-HELO: mx0a-001b2d01.pphosted.com From: "Tulio Magno Quites Machado Filho" To: libc-alpha@sourceware.org Cc: fweimer@redhat.com, bergner@vnet.ibm.com, joseph@codesourcery.com, gftg@linux.vnet.ibm.com Subject: [RFC] Fix float128 IFUNC relocations on ppc64le [BZ #21707] Date: Wed, 5 Jul 2017 14:23:15 -0300 In-Reply-To: <60e4bbfd-186a-e98d-1322-1a4678fcb000@redhat.com> References: <60e4bbfd-186a-e98d-1322-1a4678fcb000@redhat.com> X-TM-AS-MML: disable x-cbid: 17070517-0032-0000-0000-0000057211A7 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17070517-0033-0000-0000-000011F8218F Message-Id: <20170705172315.14358-1-tuliom@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2017-07-05_10:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=1 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1703280000 definitions=main-1707050293 > On 06/27/2017 09:05 AM, Florian Weimer wrote: >> On 06/27/2017 01:02 AM, Joseph Myers wrote: >>> I'm seeing a testsuite regression for powerpc64le with >>> build-many-glibcs.py with this patch (elf/check-localplt fails), did you >>> not see that in your testing? >>> >>> https://sourceware.org/ml/libc-testresults/2017-q2/msg00427.html >>> >>> The failure is: "Extra PLT reference: libc.so: __getauxval". As the >>> __getauxval reference comes from have_ieee_hw_p in libgcc, presumably you >>> need to allow that PLT reference in localplt.data with an appropriate >>> comment, as it won't be readily possible to avoid it. >> >> The __getauxval call happens from IFUNC resolvers and violates current >> guidelines regarding what can be done from IFUNC resolvers. This is >> another reason to get rid of the PLT reference. >> >> My IFUNC resolver enhancements are not ready for 2.26, and I plan to >> wait for DJ's dl-minimal malloc improvements to land, rather than >> rolling my own memory allocator to back the IFUNC resolver queue. > > The above isn't correct because even if you can call getauxval, it > doesn't have the data to return meaningful results during relocation. > This currently breaks --enable-bind-now builds on pcp64le: > > https://sourceware.org/bugzilla/show_bug.cgi?id=21707 I agree. > For the time being, we just disable --enable-bind-now, but I'd prefer a > proper fix, perhaps the one suggested here: > > https://sourceware.org/ml/libc-alpha/2017-06/msg01383.html Unfortunately, this patch creates another issue: the thread pointer is not initialized at the time of IRELA relocations in static executables. The following changes solve this issue. -- 8< -- The patch proposed by Peter Bergner [1] to libgc in order to fix [BZ #21707] adds a dependency on a symbol provided by the loader, forcing the loader to be linked to tests after libgcc was linked. The change also requires to access the thread pointer during IRELA relocations. Tested on powerpc, powerpc64, powerpc64le, s390x and x86_64. [1] https://sourceware.org/ml/libc-alpha/2017-06/msg01383.html 2017-07-05 Tulio Magno Quites Machado Filho [BZ #21707] * csu/libc-start.c (LIBC_START_MAIN): Perform IREL{,A} relocations after initializing the TCB.. * sysdeps/powerpc/powerpc64le/Makefile (f128-loader-link): New variable. [$(subdir) = math] (test-float128% test-ifloat128%): Force linking to the loader after linking to libgcc. [$(subdir) = wcsmbs stdlib] (bug-strtod bug-strtod2 bug-strtod2) (tst-strtod-round tst-wcstod-round tst-strtod6 tst-strrom) (tst-strfrom-locale strfrom-skeleton): Likewise. --- csu/libc-start.c | 9 ++++++--- sysdeps/powerpc/powerpc64le/Makefile | 10 ++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/csu/libc-start.c b/csu/libc-start.c index c2dd159..e6fa848 100644 --- a/csu/libc-start.c +++ b/csu/libc-start.c @@ -188,12 +188,15 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), ARCH_INIT_CPU_FEATURES (); - /* Perform IREL{,A} relocations. */ - apply_irel (); - /* The stack guard goes into the TCB, so initialize it early. */ __libc_setup_tls (); + /* Perform IREL{,A} relocations. + Note: the relocations must happen after TLS initialization so that + IFUNC resolvers can benefit from thread-local storage, e.g. powerpc's + hwcap and platform fields available in the TCB. */ + apply_irel (); + /* Set up the stack checker's canary. */ uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random); # ifdef THREAD_SET_STACK_GUARD diff --git a/sysdeps/powerpc/powerpc64le/Makefile b/sysdeps/powerpc/powerpc64le/Makefile index bd8a82d..abaa74d 100644 --- a/sysdeps/powerpc/powerpc64le/Makefile +++ b/sysdeps/powerpc/powerpc64le/Makefile @@ -1,6 +1,11 @@ # When building float128 we need to ensure -mfloat128 is # passed to all such object files. +# libgcc requires __tcb_parse_hwcap_and_convert_at_platform when built with +# a binary128 type. That symbol is provided by the loader on dynamically +# linked executables, forcing to link the loader after libgcc link. +f128-loader-link = $(as-needed) $(elf-objpfx)ld.so $(no-as-needed) + ifeq ($(subdir),math) # sqrtf128 requires emulation before POWER9. CPPFLAGS += -I../soft-fp @@ -11,6 +16,8 @@ $(foreach suf,$(all-object-suffixes),%f128_r$(suf)): CFLAGS += -mfloat128 $(objpfx)test-float128%.o $(objpfx)test-float128%.os: CFLAGS += -mfloat128 $(objpfx)test-ifloat128%.o $(objpfx)test-ifloat128%.os: CFLAGS += -mfloat128 CFLAGS-libm-test-support-float128.c += -mfloat128 +$(objpfx)test-float128% $(objpfx)test-ifloat128%: \ + gnulib-tests += $(f128-loader-link) endif # Append flags to string <-> _Float128 routines. @@ -24,6 +31,9 @@ CFLAGS-tst-strtod6.c += -mfloat128 CFLAGS-tst-strfrom.c += -mfloat128 CFLAGS-tst-strfrom-locale.c += -mfloat128 CFLAGS-strfrom-skeleton.c += -mfloat128 +$(foreach test,bug-strtod bug-strtod2 bug-strtod2 tst-strtod-round \ +tst-wcstod-round tst-strtod6 tst-strrom tst-strfrom-locale \ +strfrom-skeleton,$(objpfx)$(pref)): gnulib-tests += $(f128-loader-link) # When building glibc with support for _Float128, the powers of ten tables in # fpioconst.c and in the string conversion functions must be extended.