From patchwork Fri Apr 11 09:37:50 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chung-Lin Tang X-Patchwork-Id: 338435 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org 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 46876140087 for ; Fri, 11 Apr 2014 19:38:06 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:cc :subject:content-type; q=dns; s=default; b=l7p6KvWdgb1MdLF97Fuvo quLVTxFe+9VIakn43LvTJXCiVIOQ2vKiqfPALcQmec/l3O31fV+XawChumLv6tgr L71JXbeCQfpnXY+gLuVgRZsyCfsJE1ovMp+pfpMQl0r6rIHZHnR/or28XL7D/Y9q sDeUP4Gu7YGxyam2Xl0V14= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:cc :subject:content-type; s=default; bh=YRD8EJ0hSBtKe+cEVBWdd1sTLQ8 =; b=LsCjXdHxnAgfhtoBAS6B+SB8XxYiAlDv+/CQxNfrX9a0sKqi5miT7NWb4C5 +YNTzi+ZVmiewTQU0WwZ0EtUgdTaibtOFc3QcAiG+h8VKm39/NKSd/+PU2vV5tRF LWry2LAvyD9sFnARTsJowdsMK3zf95NvVlAVTat2tdqdjUsM= Received: (qmail 19108 invoked by alias); 11 Apr 2014 09:38:01 -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 19098 invoked by uid 89); 11 Apr 2014 09:38:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL, BAYES_00 autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Message-ID: <5347B7EE.6020507@mentor.com> Date: Fri, 11 Apr 2014 17:37:50 +0800 From: Chung-Lin Tang User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 MIME-Version: 1.0 To: CC: Carlos O'Donell , David Miller , "Joseph S. Myers" Subject: [PATCH] Remove divide from _ELF_DYNAMIC_DO_RELOC Hi, this follows a part of my Nios II port submission, where there was a patch to add a target macro to rtld.c to provide an inline divide during RTLD_BOOTSTRAP: https://sourceware.org/ml/libc-alpha/2014-03/msg00931.html Joseph's pointers to the older Xtensa port submission in April 2007: https://sourceware.org/ml/libc-alpha/2007-04/msg00002.html which had exactly the same problem, prompted me to investigate into the role of this divide in the dynamic relocation code in the current trunk. First of all, as the xtensa thread points out, the original context for using this MIN(nrelative,relsize/sizeof (ElfW(Rel))) expression is: https://www.sourceware.org/ml/libc-alpha/2002-06/msg00150.html which as this old Jun-2002 thread began with, is due to not initializing ranges[2] in _ELF_DYNAMIC_DO_RELOC. This is a part of the code handling the third range of the ELF_MACHINE_PLTREL_OVERLAP case, a case which has been deleted since April 2012: https://www.sourceware.org/ml/libc-alpha/2012-04/msg00103.html Also, since this 2011 patch: https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=e453f6cd0ccdd64a3f5f156e2c5f70085e9289e7 which abstracted out nrelative as a parameter, and its calculation only applied to for ranges[0], the need for doing this form of safe-guarding seems no longer true. IIUC, if binutils always gets this correct, DT_REL[A]COUNT <= relsize/sizeof(ElfW(reloc)) should always hold. The attached patch assigns the raw DT_REL[A]COUNT value to ranges[0].nrelative, removing the divide and MIN(). We might add other checks in elf_dynamic_do_Rel() to ensure r+=nrelative is really within the range, though I think this is probably not needed. I've CCed some of the people that appeared in that 2012 PLTREL overlap thread. Can anybody more familiar with this part of ld.so comment on if this is correct? In terms of improving performance this is probably an insignificant change, however for targets like Nios II (and xtensa) avoiding the need for a __udivsi3 routine really lets us avoid ugly hacks. FWIW, I've ran tests on a i686-linux machine with no regressions, though this might not be a valid corner case check. (note: on the question of why GCC did not transform the divide by constant 12 into a multiply form, which GCC is indeed capable of, is still to be investigated, but is a separate compiler problem) Thanks, Chung-Lin 2014-04-11 Chung-Lin Tang * elf/dynamic-link.h (_ELF_DYNAMIC_DO_RELOC): Remove MIN() and assign raw DT_REL[A]COUNT value to ranges[0].nrelative. diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h index 7b3e295..34ef88a 100644 --- a/elf/dynamic-link.h +++ b/elf/dynamic-link.h @@ -122,8 +122,7 @@ elf_machine_lazy_rel (struct link_map *map, ranges[0].size = (map)->l_info[DT_##RELOC##SZ]->d_un.d_val; \ if (map->l_info[VERSYMIDX (DT_##RELOC##COUNT)] != NULL) \ ranges[0].nrelative \ - = MIN (map->l_info[VERSYMIDX (DT_##RELOC##COUNT)]->d_un.d_val, \ - ranges[0].size / sizeof (ElfW(reloc))); \ + = map->l_info[VERSYMIDX (DT_##RELOC##COUNT)]->d_un.d_val; \ } \ if ((map)->l_info[DT_PLTREL] \ && (!test_rel || (map)->l_info[DT_PLTREL]->d_un.d_val == DT_##RELOC)) \