This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[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  <cltang@codesourcery.com>

	* elf/dynamic-link.h (_ELF_DYNAMIC_DO_RELOC): Remove MIN() and
	assign raw DT_REL[A]COUNT value to ranges[0].nrelative.

Attachment: x.diff
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]