From patchwork Tue Oct 15 22:31:16 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John David Anglin X-Patchwork-Id: 1177463 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-511069-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=bell.net Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="T6/6AQ7D"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 46t9Bq1C2dz9sPF for ; Wed, 16 Oct 2019 09:31:28 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type :content-transfer-encoding; q=dns; s=default; b=JzX3et/2Vt5aoo+c BaMlGTDlzhitdRogeJye16k+VJ2ITUqewTaw2rbpSzFmMJUHkv3bcanuvJcylMbA HcBFuJYhZZw/LQdDL/SyM9g04MMlq98XqxSILDPf+NYyBtPuWw5vI+aWZvWGPQVb 0/1NP2O91T30+M0xZ8HZs0lOi5M= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:to :from:subject:message-id:date:mime-version:content-type :content-transfer-encoding; s=default; bh=tAtV+4yzGnwVtLoVNw6I0n m03JY=; b=T6/6AQ7DXKdemVhrdMghcKF8uPJhHtGvk6/ybMbthp3jAaHZxfUIRR wbixB4E0JmWs66PagE1/0DS3KZRlXgyC3IigLXlr2XAP+O4NOeomcFnvHMsiFHeG 1i/4D3hxO2rAWpWZqdTsw97S9JD7+yZ5+YdzCmBBU16SeyQUkNROU= Received: (qmail 45003 invoked by alias); 15 Oct 2019 22:31:21 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 44994 invoked by uid 89); 15 Oct 2019 22:31:20 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-8.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_LOW, SPF_PASS, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 spammy= X-HELO: mtlfep02.bell.net Received: from belmont80srvr.owm.bell.net (HELO mtlfep02.bell.net) (184.150.200.80) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 15 Oct 2019 22:31:19 +0000 Received: from bell.net mtlfep02 184.150.200.30 by mtlfep02.bell.net with ESMTP id <20191015223117.ZSE4444.mtlfep02.bell.net@mtlspm01.bell.net> for ; Tue, 15 Oct 2019 18:31:17 -0400 Received: from [192.168.2.49] (really [70.53.53.104]) by mtlspm01.bell.net with ESMTP id <20191015223117.JIAO87666.mtlspm01.bell.net@[192.168.2.49]>; Tue, 15 Oct 2019 18:31:17 -0400 To: GCC Patches From: John David Anglin Subject: [committed] hppa: Improve ordering of accesses during function pointer canonicalization Openpgp: preference=signencrypt Message-ID: <693d3d9d-2846-9a90-5d41-283ac9a318dd@bell.net> Date: Tue, 15 Oct 2019 18:31:16 -0400 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 X-CM-Analysis: v=2.3 cv=ZMOpZkzb c=1 sm=1 tr=0 cx=a_idp_d a=htCe9XT+XAlGhzqgweArVg==:117 a=htCe9XT+XAlGhzqgweArVg==:17 a=IkcTkHD0fZMA:10 a=XobE76Q3jBoA:10 a=mDV3o1hIAAAA:8 a=FXVxh8MvhcvbRm0VQUoA:9 a=ukkfy2bnrXEt8Gnd:21 a=DE8ijDn1cyWJQRug:21 a=QEXdDO2ut3YA:10 a=_FVE-zBwftR9WsbkzFJk:22 X-CM-Envelope: MS4wfAhyk02TNOAEuz2TEr232A5tYHvhof+J+ZRMAt2ccUe9XYC330A7pQotRRdaPqYbpHRSZK45cfYS7paXMTfHFgCK57jopdGZmZEux8c3430n3APSf6eu tht//xxvPxteWZSMgquuUobf7VqH3DdeU5fGuPy2nnWEI/NfFXxIM4GlbvkL46hYNXfpBM7kHDqx/g== The main fix here is to load the relocation offset before the function pointer during function pointer canonicalization. There is still a small race in multi-threaded applications because the dynamic linker currently updates the relocation offset before the function pointer when doing lazy binding. When binutils is fixed to double word align function descriptors, it will be possible to atomically update the descriptor using a floating point store. Then, this race will be eliminated. Tested on hppa-unknown-linux-gnu with no regressions. Committed to trunk and gcc-9 branch. Dave 2019-10-15 John David Anglin * config/pa/fptr.c (_dl_read_access_allowed): Change argument to unsigned int. Adjust callers. (__canonicalize_funcptr_for_compare): Change plabel type to volatile unsigned int *. Load relocation offset before function pointer. Add barrier to ensure ordering. Index: config/pa/fptr.c =================================================================== --- config/pa/fptr.c (revision 276964) +++ config/pa/fptr.c (working copy) @@ -53,7 +53,7 @@ extern unsigned int _GLOBAL_OFFSET_TABLE_; static inline int -_dl_read_access_allowed (unsigned int *addr) +_dl_read_access_allowed (unsigned int addr) { int result; @@ -79,7 +79,8 @@ { static unsigned int fixup_plabel[2] __attribute__((used)); fixup_t fixup; - unsigned int *got, *iptr, *plabel; + volatile unsigned int *plabel; + unsigned int *got, *iptr, reloc_offset; int i; /* -1 and page 0 are special. -1 is used in crtend to mark the end of @@ -94,8 +95,8 @@ to the entry of the PLT stub just before the global offset table. The second word in the plabel contains the relocation offset for the function. */ - plabel = (unsigned int *) ((unsigned int) fptr & ~3); - if (!_dl_read_access_allowed (plabel)) + plabel = (volatile unsigned int *) ((unsigned int) fptr & ~3); + if (!_dl_read_access_allowed ((unsigned int)plabel)) return (unsigned int) fptr; /* Load first word of candidate descriptor. It should be a pointer @@ -102,9 +103,12 @@ with word alignment and point to memory that can be read. */ got = (unsigned int *) plabel[0]; if (((unsigned int) got & 3) != 0 - || !_dl_read_access_allowed (got)) + || !_dl_read_access_allowed ((unsigned int)got)) return (unsigned int) fptr; + /* We need to load the relocation offset before the function address. */ + reloc_offset = plabel[1]; + __sync_synchronize(); got = (unsigned int *) (plabel[0] + GOT_FROM_PLT_STUB); /* Return the address of the function if the plabel has been resolved. */ @@ -140,7 +144,7 @@ /* Call fixup to resolve the function address. got[1] contains the link_map pointer and plabel[1] the relocation offset. */ - fixup ((struct link_map *) got[1], plabel[1]); + fixup ((struct link_map *) got[1], reloc_offset); return plabel[0]; }