| Message ID | 000a01cfe8dd$488fbdf0$d9af39d0$@rt-rk.com |
|---|---|
| State | Superseded |
| Headers |
Received: (qmail 29329 invoked by alias); 16 Oct 2014 01:05:40 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 29313 invoked by uid 89); 16 Oct 2014 01:05:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL, BAYES_00, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mail.rt-rk.com From: "Petar Jovanovic" <petar.jovanovic@rt-rk.com> To: <libc-alpha@sourceware.org> Cc: <petar.jovanovic@imgtec.com>, <davem@davemloft.net> References: <1408493406-86741-1-git-send-email-petar.jovanovic@rt-rk.com> <1408493406-86741-2-git-send-email-petar.jovanovic@rt-rk.com> In-Reply-To: Subject: RE: [PATCH] Fix dynamic linker issue with bind-now Date: Thu, 16 Oct 2014 03:05:31 +0200 Message-ID: <000a01cfe8dd$488fbdf0$d9af39d0$@rt-rk.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit |
Commit Message
Petar Jovanovic
Oct. 16, 2014, 1:05 a.m. UTC
Do you any additional data/feedback to proceed with this issue? Let me know. Thank you. Regards, Petar -----Original Message----- From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] Sent: Wednesday, September 10, 2014 4:44 PM To: 'libc-alpha@sourceware.org' Cc: 'petar.jovanovic@imgtec.com'; 'davem@davemloft.net' Subject: RE: [PATCH] Fix dynamic linker issue with bind-now ping -----Original Message----- From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] Sent: Wednesday, August 20, 2014 2:10 AM To: libc-alpha@sourceware.org Cc: petar.jovanovic@imgtec.com; davem@davemloft.net; Petar Jovanovic Subject: [PATCH] Fix dynamic linker issue with bind-now Fix the bind-now case when DT_REL and DT_JMPREL sections are separate and there is a gap between them. --- elf/dynamic-link.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) + && ((do_lazy) || ranges[0].size == 0 || \ + ranges[0].start + ranges[0].size != start)) \ { \ ranges[1].start = start; \ ranges[1].size = size; \ -- 1.7.9.5
Comments
On 16 Oct 2014 03:05, Petar Jovanovic wrote:
> Do you any additional data/feedback to proceed with this issue?
when Roland asked for a testcase, the implication was to include it in glibc
itself so it'd be compiled & run as part of `make check`. that way we won't
accidentally regress in the future.
-mike
Hi Mike, This was not clear to me, otherwise I would add the test case in a patch. Thanks for the explanation. Here it is again. Let me know if you want some modification. Regards, Petar -----Original Message----- From: Mike Frysinger [mailto:vapier@gentoo.org] Sent: Tuesday, October 21, 2014 2:33 PM To: Petar Jovanovic Cc: libc-alpha@sourceware.org; petar.jovanovic@imgtec.com; davem@davemloft.net Subject: Re: [PATCH] Fix dynamic linker issue with bind-now On 16 Oct 2014 03:05, Petar Jovanovic wrote: > Do you any additional data/feedback to proceed with this issue? when Roland asked for a testcase, the implication was to include it in glibc itself so it'd be compiled & run as part of `make check`. that way we won't accidentally regress in the future. -mike
Any additional comments here? Regards, Petar -----Original Message----- From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] Sent: Friday, October 31, 2014 3:47 AM To: 'Mike Frysinger' Cc: 'libc-alpha@sourceware.org'; 'petar.jovanovic@imgtec.com'; 'davem@davemloft.net' Subject: RE: [PATCH] Fix dynamic linker issue with bind-now Hi Mike, This was not clear to me, otherwise I would add the test case in a patch. Thanks for the explanation. Here it is again. Let me know if you want some modification. Regards, Petar -----Original Message----- From: Mike Frysinger [mailto:vapier@gentoo.org] Sent: Tuesday, October 21, 2014 2:33 PM To: Petar Jovanovic Cc: libc-alpha@sourceware.org; petar.jovanovic@imgtec.com; davem@davemloft.net Subject: Re: [PATCH] Fix dynamic linker issue with bind-now On 16 Oct 2014 03:05, Petar Jovanovic wrote: > Do you any additional data/feedback to proceed with this issue? when Roland asked for a testcase, the implication was to include it in glibc itself so it'd be compiled & run as part of `make check`. that way we won't accidentally regress in the future. -mike
Ping. -----Original Message----- From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] Sent: Friday, November 14, 2014 3:07 AM To: 'Mike Frysinger' Cc: 'libc-alpha@sourceware.org'; 'petar.jovanovic@imgtec.com'; 'davem@davemloft.net' Subject: RE: [PATCH] Fix dynamic linker issue with bind-now Any additional comments here? Regards, Petar -----Original Message----- From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] Sent: Friday, October 31, 2014 3:47 AM To: 'Mike Frysinger' Cc: 'libc-alpha@sourceware.org'; 'petar.jovanovic@imgtec.com'; 'davem@davemloft.net' Subject: RE: [PATCH] Fix dynamic linker issue with bind-now Hi Mike, This was not clear to me, otherwise I would add the test case in a patch. Thanks for the explanation. Here it is again. Let me know if you want some modification. Regards, Petar -----Original Message----- From: Mike Frysinger [mailto:vapier@gentoo.org] Sent: Tuesday, October 21, 2014 2:33 PM To: Petar Jovanovic Cc: libc-alpha@sourceware.org; petar.jovanovic@imgtec.com; davem@davemloft.net Subject: Re: [PATCH] Fix dynamic linker issue with bind-now On 16 Oct 2014 03:05, Petar Jovanovic wrote: > Do you any additional data/feedback to proceed with this issue? when Roland asked for a testcase, the implication was to include it in glibc itself so it'd be compiled & run as part of `make check`. that way we won't accidentally regress in the future. -mike
Hi Petar, I think this patch could really use comments explaining what is happening for future readers of the code (both in dynamic-link.h and the test). I also think tst-dynamic-link is quite a generic name and it might be better to rename it to something more specific e.g. tst-split-dynreloc or something like that. It would also be good to mention how the patch was tested e.g. which platforms. On 2 December 2014 at 23:27, Petar Jovanovic <petar.jovanovic@rt-rk.com> wrote: > Ping. > > -----Original Message----- > From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] > Sent: Friday, November 14, 2014 3:07 AM > To: 'Mike Frysinger' > Cc: 'libc-alpha@sourceware.org'; 'petar.jovanovic@imgtec.com'; 'davem@davemloft.net' > Subject: RE: [PATCH] Fix dynamic linker issue with bind-now > > Any additional comments here? > > Regards, > Petar > > -----Original Message----- > From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] > Sent: Friday, October 31, 2014 3:47 AM > To: 'Mike Frysinger' > Cc: 'libc-alpha@sourceware.org'; 'petar.jovanovic@imgtec.com'; 'davem@davemloft.net' > Subject: RE: [PATCH] Fix dynamic linker issue with bind-now > > Hi Mike, > > This was not clear to me, otherwise I would add the test case in a patch. > Thanks for the explanation. Here it is again. > > Let me know if you want some modification. > > Regards, > Petar > > -----Original Message----- > From: Mike Frysinger [mailto:vapier@gentoo.org] > Sent: Tuesday, October 21, 2014 2:33 PM > To: Petar Jovanovic > Cc: libc-alpha@sourceware.org; petar.jovanovic@imgtec.com; davem@davemloft.net > Subject: Re: [PATCH] Fix dynamic linker issue with bind-now > > On 16 Oct 2014 03:05, Petar Jovanovic wrote: >> Do you any additional data/feedback to proceed with this issue? > > when Roland asked for a testcase, the implication was to include it in glibc itself so it'd be compiled & run as part of `make check`. that way we won't accidentally regress in the future. > -mike >
Hi Will, I have changed the name of the test to tst-split-dynreloc as you suggested. Further, I have added comments in the test. As of dynamic-link.h, it already has a write-up, see lines 100-111, the problem was that the code was not fully in sync with those lines. The particular test can be seen failing on x86, so this change fixes it. Furthermore, this particular change resolves important issue for Chromium tests on MIPS32 I reported [1] 4 months ago. I am attaching the updated version of the patch. Regards, Petar [1] Building/running content_shell for MIPS32, https://sourceware.org/ml/libc-alpha/2014-08/msg00316.html -----Original Message----- From: Will Newton [mailto:will.newton@linaro.org] Sent: Wednesday, December 3, 2014 10:37 AM To: Petar Jovanovic Cc: Mike Frysinger; libc-alpha; petar.jovanovic@imgtec.com; David Miller Subject: Re: [PATCH] Fix dynamic linker issue with bind-now Hi Petar, I think this patch could really use comments explaining what is happening for future readers of the code (both in dynamic-link.h and the test). I also think tst-dynamic-link is quite a generic name and it might be better to rename it to something more specific e.g. tst-split-dynreloc or something like that. It would also be good to mention how the patch was tested e.g. which platforms. On 2 December 2014 at 23:27, Petar Jovanovic <petar.jovanovic@rt-rk.com> wrote: > Ping. > > -----Original Message----- > From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] > Sent: Friday, November 14, 2014 3:07 AM > To: 'Mike Frysinger' > Cc: 'libc-alpha@sourceware.org'; 'petar.jovanovic@imgtec.com'; 'davem@davemloft.net' > Subject: RE: [PATCH] Fix dynamic linker issue with bind-now > > Any additional comments here? > > Regards, > Petar > > -----Original Message----- > From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] > Sent: Friday, October 31, 2014 3:47 AM > To: 'Mike Frysinger' > Cc: 'libc-alpha@sourceware.org'; 'petar.jovanovic@imgtec.com'; 'davem@davemloft.net' > Subject: RE: [PATCH] Fix dynamic linker issue with bind-now > > Hi Mike, > > This was not clear to me, otherwise I would add the test case in a patch. > Thanks for the explanation. Here it is again. > > Let me know if you want some modification. > > Regards, > Petar > > -----Original Message----- > From: Mike Frysinger [mailto:vapier@gentoo.org] > Sent: Tuesday, October 21, 2014 2:33 PM > To: Petar Jovanovic > Cc: libc-alpha@sourceware.org; petar.jovanovic@imgtec.com; > davem@davemloft.net > Subject: Re: [PATCH] Fix dynamic linker issue with bind-now > > On 16 Oct 2014 03:05, Petar Jovanovic wrote: >> Do you any additional data/feedback to proceed with this issue? > > when Roland asked for a testcase, the implication was to include it in glibc itself so it'd be compiled & run as part of `make check`. that way we won't accidentally regress in the future. > -mike > -- Will Newton Toolchain Working Group, Linaro
"Petar Jovanovic" <petar.jovanovic@rt-rk.com> writes: > diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h > index 7b3e295..d5dea8e 100644 > --- a/elf/dynamic-link.h > +++ b/elf/dynamic-link.h > @@ -133,7 +133,9 @@ elf_machine_lazy_rel (struct link_map *map, > \ > if (ranges[0].start + ranges[0].size == (start + size)) \ > ranges[0].size -= size; \ > - if (! ELF_DURING_STARTUP && ((do_lazy) || ranges[0].size == 0)) \ > + if (! ELF_DURING_STARTUP \ > + && ((do_lazy) || ranges[0].size == 0 || \ > + ranges[0].start + ranges[0].size != start)) \ Line break before the operator, not after; tabify indentation. > diff --git a/elf/tst-split-dynreloc.c b/elf/tst-split-dynreloc.c > new file mode 100644 > index 0000000..32d474f > --- /dev/null > +++ b/elf/tst-split-dynreloc.c > @@ -0,0 +1,27 @@ > +#include <stdio.h> > + > +static int __attribute__((section(".bar"))) bar = 0x12345678; Space before paren. > +/* > + * This test will be used to create an executable with a specific > + * section layout in which .rela.dyn and .rela.plt are not contiguous. > + * > + * For x86 case, readelf will report something like: > + * > + * ... > + * [10] .rela.dyn RELA > + * [11] .bar PROGBITS > + * [12] .rela.plt RELA > + * ... > + * > + * This is important as this case was not correctly handled by dynamic > + * linker in the bind-now case, and the second section was never > + * processed. > + */ Remove the asterisk column. > +int main() { > + printf("%s\n", foo); > + return 0; > +} Braces on new line, line break after return type, space before paren, missing prototype. Andreas.
Hi Andreas, Thanks for the comments. > Line break before the operator, not after; tabify indentation. Done. > Space before paren. Done. >Remove the asterisk column. Done. > Braces on new line, line break after return type, space before paren, missing prototype. Done, done, done. What is missing a prototype? Printf? If so, that should be already present in <stdio.h> I am attaching 3rd updated version of the patch. Regards, Petar -----Original Message----- From: Andreas Schwab [mailto:schwab@linux-m68k.org] Sent: Saturday, December 6, 2014 10:38 AM To: Petar Jovanovic Cc: 'Will Newton'; 'Mike Frysinger'; 'libc-alpha'; petar.jovanovic@imgtec.com; 'David Miller' Subject: Re: [PATCH] Fix dynamic linker issue with bind-now "Petar Jovanovic" <petar.jovanovic@rt-rk.com> writes: > diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h > index 7b3e295..d5dea8e 100644 > --- a/elf/dynamic-link.h > +++ b/elf/dynamic-link.h > @@ -133,7 +133,9 @@ elf_machine_lazy_rel (struct link_map *map, > \ > if (ranges[0].start + ranges[0].size == (start + size)) \ > ranges[0].size -= size; \ > - if (! ELF_DURING_STARTUP && ((do_lazy) || ranges[0].size == 0)) \ > + if (! ELF_DURING_STARTUP \ > + && ((do_lazy) || ranges[0].size == 0 || \ > + ranges[0].start + ranges[0].size != start)) \ Line break before the operator, not after; tabify indentation. > diff --git a/elf/tst-split-dynreloc.c b/elf/tst-split-dynreloc.c > new file mode 100644 > index 0000000..32d474f > --- /dev/null > +++ b/elf/tst-split-dynreloc.c > @@ -0,0 +1,27 @@ > +#include <stdio.h> > + > +static int __attribute__((section(".bar"))) bar = 0x12345678; Space before paren. > +/* > + * This test will be used to create an executable with a specific > + * section layout in which .rela.dyn and .rela.plt are not contiguous. > + * > + * For x86 case, readelf will report something like: > + * > + * ... > + * [10] .rela.dyn RELA > + * [11] .bar PROGBITS > + * [12] .rela.plt RELA > + * ... > + * > + * This is important as this case was not correctly handled by dynamic > + * linker in the bind-now case, and the second section was never > + * processed. > + */ Remove the asterisk column. > +int main() { > + printf("%s\n", foo); > + return 0; > +} Braces on new line, line break after return type, space before paren, missing prototype. Andreas.
ping. -----Original Message----- From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] Sent: Sunday, December 7, 2014 2:04 AM To: 'Andreas Schwab' Cc: 'Will Newton'; 'Mike Frysinger'; 'libc-alpha'; 'petar.jovanovic@imgtec.com'; 'David Miller' Subject: RE: [PATCH] Fix dynamic linker issue with bind-now Hi Andreas, Thanks for the comments. > Line break before the operator, not after; tabify indentation. Done. > Space before paren. Done. >Remove the asterisk column. Done. > Braces on new line, line break after return type, space before paren, missing prototype. Done, done, done. What is missing a prototype? Printf? If so, that should be already present in <stdio.h> I am attaching 3rd updated version of the patch. Regards, Petar -----Original Message----- From: Andreas Schwab [mailto:schwab@linux-m68k.org] Sent: Saturday, December 6, 2014 10:38 AM To: Petar Jovanovic Cc: 'Will Newton'; 'Mike Frysinger'; 'libc-alpha'; petar.jovanovic@imgtec.com; 'David Miller' Subject: Re: [PATCH] Fix dynamic linker issue with bind-now "Petar Jovanovic" <petar.jovanovic@rt-rk.com> writes: > diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h index > 7b3e295..d5dea8e 100644 > --- a/elf/dynamic-link.h > +++ b/elf/dynamic-link.h > @@ -133,7 +133,9 @@ elf_machine_lazy_rel (struct link_map *map, > \ > if (ranges[0].start + ranges[0].size == (start + size)) \ > ranges[0].size -= size; \ > - if (! ELF_DURING_STARTUP && ((do_lazy) || ranges[0].size == 0)) \ > + if (! ELF_DURING_STARTUP \ > + && ((do_lazy) || ranges[0].size == 0 || \ > + ranges[0].start + ranges[0].size != start)) \ Line break before the operator, not after; tabify indentation. > diff --git a/elf/tst-split-dynreloc.c b/elf/tst-split-dynreloc.c new > file mode 100644 index 0000000..32d474f > --- /dev/null > +++ b/elf/tst-split-dynreloc.c > @@ -0,0 +1,27 @@ > +#include <stdio.h> > + > +static int __attribute__((section(".bar"))) bar = 0x12345678; Space before paren. > +/* > + * This test will be used to create an executable with a specific > + * section layout in which .rela.dyn and .rela.plt are not contiguous. > + * > + * For x86 case, readelf will report something like: > + * > + * ... > + * [10] .rela.dyn RELA > + * [11] .bar PROGBITS > + * [12] .rela.plt RELA > + * ... > + * > + * This is important as this case was not correctly handled by > +dynamic > + * linker in the bind-now case, and the second section was never > + * processed. > + */ Remove the asterisk column. > +int main() { > + printf("%s\n", foo); > + return 0; > +} Braces on new line, line break after return type, space before paren, missing prototype. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
ping. -----Original Message----- From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] Sent: Wednesday, December 17, 2014 12:26 AM To: 'Andreas Schwab' Cc: 'Will Newton'; 'Mike Frysinger'; 'libc-alpha'; 'petar.jovanovic@imgtec.com'; 'David Miller' Subject: RE: [PATCH] Fix dynamic linker issue with bind-now ping. -----Original Message----- From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] Sent: Sunday, December 7, 2014 2:04 AM To: 'Andreas Schwab' Cc: 'Will Newton'; 'Mike Frysinger'; 'libc-alpha'; 'petar.jovanovic@imgtec.com'; 'David Miller' Subject: RE: [PATCH] Fix dynamic linker issue with bind-now Hi Andreas, Thanks for the comments. > Line break before the operator, not after; tabify indentation. Done. > Space before paren. Done. >Remove the asterisk column. Done. > Braces on new line, line break after return type, space before paren, missing prototype. Done, done, done. What is missing a prototype? Printf? If so, that should be already present in <stdio.h> I am attaching 3rd updated version of the patch. Regards, Petar -----Original Message----- From: Andreas Schwab [mailto:schwab@linux-m68k.org] Sent: Saturday, December 6, 2014 10:38 AM To: Petar Jovanovic Cc: 'Will Newton'; 'Mike Frysinger'; 'libc-alpha'; petar.jovanovic@imgtec.com; 'David Miller' Subject: Re: [PATCH] Fix dynamic linker issue with bind-now "Petar Jovanovic" <petar.jovanovic@rt-rk.com> writes: > diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h index > 7b3e295..d5dea8e 100644 > --- a/elf/dynamic-link.h > +++ b/elf/dynamic-link.h > @@ -133,7 +133,9 @@ elf_machine_lazy_rel (struct link_map *map, > \ > if (ranges[0].start + ranges[0].size == (start + size)) \ > ranges[0].size -= size; \ > - if (! ELF_DURING_STARTUP && ((do_lazy) || ranges[0].size == 0)) \ > + if (! ELF_DURING_STARTUP \ > + && ((do_lazy) || ranges[0].size == 0 || \ > + ranges[0].start + ranges[0].size != start)) \ Line break before the operator, not after; tabify indentation. > diff --git a/elf/tst-split-dynreloc.c b/elf/tst-split-dynreloc.c new > file mode 100644 index 0000000..32d474f > --- /dev/null > +++ b/elf/tst-split-dynreloc.c > @@ -0,0 +1,27 @@ > +#include <stdio.h> > + > +static int __attribute__((section(".bar"))) bar = 0x12345678; Space before paren. > +/* > + * This test will be used to create an executable with a specific > + * section layout in which .rela.dyn and .rela.plt are not contiguous. > + * > + * For x86 case, readelf will report something like: > + * > + * ... > + * [10] .rela.dyn RELA > + * [11] .bar PROGBITS > + * [12] .rela.plt RELA > + * ... > + * > + * This is important as this case was not correctly handled by > +dynamic > + * linker in the bind-now case, and the second section was never > + * processed. > + */ Remove the asterisk column. > +int main() { > + printf("%s\n", foo); > + return 0; > +} Braces on new line, line break after return type, space before paren, missing prototype. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Anyone? Is the patch OK to commit? -----Original Message----- From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] Sent: Tuesday, January 6, 2015 2:14 AM To: 'Andreas Schwab' Cc: 'Will Newton'; 'Mike Frysinger'; 'libc-alpha'; 'petar.jovanovic@imgtec.com'; 'David Miller' Subject: RE: [PATCH] Fix dynamic linker issue with bind-now ping. -----Original Message----- From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] Sent: Wednesday, December 17, 2014 12:26 AM To: 'Andreas Schwab' Cc: 'Will Newton'; 'Mike Frysinger'; 'libc-alpha'; 'petar.jovanovic@imgtec.com'; 'David Miller' Subject: RE: [PATCH] Fix dynamic linker issue with bind-now ping. -----Original Message----- From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] Sent: Sunday, December 7, 2014 2:04 AM To: 'Andreas Schwab' Cc: 'Will Newton'; 'Mike Frysinger'; 'libc-alpha'; 'petar.jovanovic@imgtec.com'; 'David Miller' Subject: RE: [PATCH] Fix dynamic linker issue with bind-now Hi Andreas, Thanks for the comments. > Line break before the operator, not after; tabify indentation. Done. > Space before paren. Done. >Remove the asterisk column. Done. > Braces on new line, line break after return type, space before paren, missing prototype. Done, done, done. What is missing a prototype? Printf? If so, that should be already present in <stdio.h> I am attaching 3rd updated version of the patch. Regards, Petar -----Original Message----- From: Andreas Schwab [mailto:schwab@linux-m68k.org] Sent: Saturday, December 6, 2014 10:38 AM To: Petar Jovanovic Cc: 'Will Newton'; 'Mike Frysinger'; 'libc-alpha'; petar.jovanovic@imgtec.com; 'David Miller' Subject: Re: [PATCH] Fix dynamic linker issue with bind-now "Petar Jovanovic" <petar.jovanovic@rt-rk.com> writes: > diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h index > 7b3e295..d5dea8e 100644 > --- a/elf/dynamic-link.h > +++ b/elf/dynamic-link.h > @@ -133,7 +133,9 @@ elf_machine_lazy_rel (struct link_map *map, > \ > if (ranges[0].start + ranges[0].size == (start + size)) \ > ranges[0].size -= size; \ > - if (! ELF_DURING_STARTUP && ((do_lazy) || ranges[0].size == 0)) \ > + if (! ELF_DURING_STARTUP \ > + && ((do_lazy) || ranges[0].size == 0 || \ > + ranges[0].start + ranges[0].size != start)) \ Line break before the operator, not after; tabify indentation. > diff --git a/elf/tst-split-dynreloc.c b/elf/tst-split-dynreloc.c new > file mode 100644 index 0000000..32d474f > --- /dev/null > +++ b/elf/tst-split-dynreloc.c > @@ -0,0 +1,27 @@ > +#include <stdio.h> > + > +static int __attribute__((section(".bar"))) bar = 0x12345678; Space before paren. > +/* > + * This test will be used to create an executable with a specific > + * section layout in which .rela.dyn and .rela.plt are not contiguous. > + * > + * For x86 case, readelf will report something like: > + * > + * ... > + * [10] .rela.dyn RELA > + * [11] .bar PROGBITS > + * [12] .rela.plt RELA > + * ... > + * > + * This is important as this case was not correctly handled by > +dynamic > + * linker in the bind-now case, and the second section was never > + * processed. > + */ Remove the asterisk column. > +int main() { > + printf("%s\n", foo); > + return 0; > +} Braces on new line, line break after return type, space before paren, missing prototype. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
ping. -----Original Message----- From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] Sent: Thursday, January 15, 2015 8:26 PM To: 'Andreas Schwab' Cc: 'Will Newton'; 'Mike Frysinger'; 'libc-alpha'; 'petar.jovanovic@imgtec.com'; 'David Miller'; 'Roland McGrath' Subject: RE: [PATCH] Fix dynamic linker issue with bind-now Anyone? Is the patch OK to commit? -----Original Message----- From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] Sent: Tuesday, January 6, 2015 2:14 AM To: 'Andreas Schwab' Cc: 'Will Newton'; 'Mike Frysinger'; 'libc-alpha'; 'petar.jovanovic@imgtec.com'; 'David Miller' Subject: RE: [PATCH] Fix dynamic linker issue with bind-now ping. -----Original Message----- From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] Sent: Wednesday, December 17, 2014 12:26 AM To: 'Andreas Schwab' Cc: 'Will Newton'; 'Mike Frysinger'; 'libc-alpha'; 'petar.jovanovic@imgtec.com'; 'David Miller' Subject: RE: [PATCH] Fix dynamic linker issue with bind-now ping. -----Original Message----- From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] Sent: Sunday, December 7, 2014 2:04 AM To: 'Andreas Schwab' Cc: 'Will Newton'; 'Mike Frysinger'; 'libc-alpha'; 'petar.jovanovic@imgtec.com'; 'David Miller' Subject: RE: [PATCH] Fix dynamic linker issue with bind-now Hi Andreas, Thanks for the comments. > Line break before the operator, not after; tabify indentation. Done. > Space before paren. Done. >Remove the asterisk column. Done. > Braces on new line, line break after return type, space before paren, missing prototype. Done, done, done. What is missing a prototype? Printf? If so, that should be already present in <stdio.h> I am attaching 3rd updated version of the patch. Regards, Petar -----Original Message----- From: Andreas Schwab [mailto:schwab@linux-m68k.org] Sent: Saturday, December 6, 2014 10:38 AM To: Petar Jovanovic Cc: 'Will Newton'; 'Mike Frysinger'; 'libc-alpha'; petar.jovanovic@imgtec.com; 'David Miller' Subject: Re: [PATCH] Fix dynamic linker issue with bind-now "Petar Jovanovic" <petar.jovanovic@rt-rk.com> writes: > diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h index > 7b3e295..d5dea8e 100644 > --- a/elf/dynamic-link.h > +++ b/elf/dynamic-link.h > @@ -133,7 +133,9 @@ elf_machine_lazy_rel (struct link_map *map, > \ > if (ranges[0].start + ranges[0].size == (start + size)) \ > ranges[0].size -= size; \ > - if (! ELF_DURING_STARTUP && ((do_lazy) || ranges[0].size == 0)) \ > + if (! ELF_DURING_STARTUP \ > + && ((do_lazy) || ranges[0].size == 0 || \ > + ranges[0].start + ranges[0].size != start)) \ Line break before the operator, not after; tabify indentation. > diff --git a/elf/tst-split-dynreloc.c b/elf/tst-split-dynreloc.c new > file mode 100644 index 0000000..32d474f > --- /dev/null > +++ b/elf/tst-split-dynreloc.c > @@ -0,0 +1,27 @@ > +#include <stdio.h> > + > +static int __attribute__((section(".bar"))) bar = 0x12345678; Space before paren. > +/* > + * This test will be used to create an executable with a specific > + * section layout in which .rela.dyn and .rela.plt are not contiguous. > + * > + * For x86 case, readelf will report something like: > + * > + * ... > + * [10] .rela.dyn RELA > + * [11] .bar PROGBITS > + * [12] .rela.plt RELA > + * ... > + * > + * This is important as this case was not correctly handled by > +dynamic > + * linker in the bind-now case, and the second section was never > + * processed. > + */ Remove the asterisk column. > +int main() { > + printf("%s\n", foo); > + return 0; > +} Braces on new line, line break after return type, space before paren, missing prototype. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
ping. -----Original Message----- From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] Sent: Wednesday, January 28, 2015 11:34 PM To: 'Andreas Schwab' Cc: 'Will Newton'; 'Mike Frysinger'; 'libc-alpha'; 'petar.jovanovic@imgtec.com'; 'David Miller'; 'Roland McGrath' Subject: RE: [PATCH] Fix dynamic linker issue with bind-now ping. -----Original Message----- From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] Sent: Thursday, January 15, 2015 8:26 PM To: 'Andreas Schwab' Cc: 'Will Newton'; 'Mike Frysinger'; 'libc-alpha'; 'petar.jovanovic@imgtec.com'; 'David Miller'; 'Roland McGrath' Subject: RE: [PATCH] Fix dynamic linker issue with bind-now Anyone? Is the patch OK to commit? -----Original Message----- From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] Sent: Tuesday, January 6, 2015 2:14 AM To: 'Andreas Schwab' Cc: 'Will Newton'; 'Mike Frysinger'; 'libc-alpha'; 'petar.jovanovic@imgtec.com'; 'David Miller' Subject: RE: [PATCH] Fix dynamic linker issue with bind-now ping. -----Original Message----- From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] Sent: Wednesday, December 17, 2014 12:26 AM To: 'Andreas Schwab' Cc: 'Will Newton'; 'Mike Frysinger'; 'libc-alpha'; 'petar.jovanovic@imgtec.com'; 'David Miller' Subject: RE: [PATCH] Fix dynamic linker issue with bind-now ping. -----Original Message----- From: Petar Jovanovic [mailto:petar.jovanovic@rt-rk.com] Sent: Sunday, December 7, 2014 2:04 AM To: 'Andreas Schwab' Cc: 'Will Newton'; 'Mike Frysinger'; 'libc-alpha'; 'petar.jovanovic@imgtec.com'; 'David Miller' Subject: RE: [PATCH] Fix dynamic linker issue with bind-now Hi Andreas, Thanks for the comments. > Line break before the operator, not after; tabify indentation. Done. > Space before paren. Done. >Remove the asterisk column. Done. > Braces on new line, line break after return type, space before paren, missing prototype. Done, done, done. What is missing a prototype? Printf? If so, that should be already present in <stdio.h> I am attaching 3rd updated version of the patch. Regards, Petar -----Original Message----- From: Andreas Schwab [mailto:schwab@linux-m68k.org] Sent: Saturday, December 6, 2014 10:38 AM To: Petar Jovanovic Cc: 'Will Newton'; 'Mike Frysinger'; 'libc-alpha'; petar.jovanovic@imgtec.com; 'David Miller' Subject: Re: [PATCH] Fix dynamic linker issue with bind-now "Petar Jovanovic" <petar.jovanovic@rt-rk.com> writes: > diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h index > 7b3e295..d5dea8e 100644 > --- a/elf/dynamic-link.h > +++ b/elf/dynamic-link.h > @@ -133,7 +133,9 @@ elf_machine_lazy_rel (struct link_map *map, > \ > if (ranges[0].start + ranges[0].size == (start + size)) \ > ranges[0].size -= size; \ > - if (! ELF_DURING_STARTUP && ((do_lazy) || ranges[0].size == 0)) \ > + if (! ELF_DURING_STARTUP \ > + && ((do_lazy) || ranges[0].size == 0 || \ > + ranges[0].start + ranges[0].size != start)) \ Line break before the operator, not after; tabify indentation. > diff --git a/elf/tst-split-dynreloc.c b/elf/tst-split-dynreloc.c new > file mode 100644 index 0000000..32d474f > --- /dev/null > +++ b/elf/tst-split-dynreloc.c > @@ -0,0 +1,27 @@ > +#include <stdio.h> > + > +static int __attribute__((section(".bar"))) bar = 0x12345678; Space before paren. > +/* > + * This test will be used to create an executable with a specific > + * section layout in which .rela.dyn and .rela.plt are not contiguous. > + * > + * For x86 case, readelf will report something like: > + * > + * ... > + * [10] .rela.dyn RELA > + * [11] .bar PROGBITS > + * [12] .rela.plt RELA > + * ... > + * > + * This is important as this case was not correctly handled by > +dynamic > + * linker in the bind-now case, and the second section was never > + * processed. > + */ Remove the asterisk column. > +int main() { > + printf("%s\n", foo); > + return 0; > +} Braces on new line, line break after return type, space before paren, missing prototype. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
> --- a/elf/Makefile > +++ b/elf/Makefile > > +LDFLAGS-tst-split-dynreloc = -fPIE -Wl,-T,tst-split-dynreloc.lds > +CFLAGS-tst-split-dynreloc.c = -O0 -DIS_IN_build shouldn't the source file be compiled w/-fPIE too ? do you need the IS_IN_build define ? i'm not seeing it used. > +tst-split-dynreloc-ENV = LD_BIND_NOW=1 do you need to use the env var ? pass -Wl,-z,now via LDFLAGS instead ? > --- /dev/null > +++ b/elf/tst-split-dynreloc.c > > +/* This test will be used to create an executable with a specific > + section layout in which .rela.dyn and .rela.plt are not contiguous. > + For x86 case, readelf will report something like: > + > + ... > + [10] .rela.dyn RELA > + [11] .bar PROGBITS > + [12] .rela.plt RELA > + ... > + > + This is important as this case was not correctly handled by dynamic > + linker in the bind-now case, and the second section was never > + processed. */ this should be at the top of the file -- move the includes/globals/etc... below > +int > +main (void) > +{ > + printf ("%s\n", foo); > + return 0; > +} please use test-skeleton.c. look at time/tst-ftime.c as a simple example. -mike
-----Original Message----- From: Mike Frysinger [mailto:vapier@gentoo.org] Sent: Monday, March 2, 2015 7:29 AM To: Petar Jovanovic Cc: libc-alpha@sourceware.org; petar.jovanovic@imgtec.com Subject: Re: [PATCH] Fix dynamic linker issue with bind-now >shouldn't the source file be compiled w/-fPIE too ? In the new version of the patch, this has been changed, so no CFLAGS are required. > do you need the IS_IN_build define ? i'm not seeing it used. It was required for the use of "-O0". Now it is not needed anymore in v3. >> +tst-split-dynreloc-ENV = LD_BIND_NOW=1 >do you need to use the env var ? pass -Wl,-z,now via LDFLAGS instead ? Done. > this should be at the top of the file -- move the includes/globals/etc... below Done. > +int > +main (void) > +{ > + printf ("%s\n", foo); > + return 0; > +} > please use test-skeleton.c. look at time/tst-ftime.c as a simple example. Done. Take a look at v3 of the change. Thanks. Regards, Petar
On 02 Mar 2015 17:19, Petar Jovanovic wrote: > From: Mike Frysinger [mailto:vapier@gentoo.org] > > shouldn't the source file be compiled w/-fPIE too ? > > In the new version of the patch, this has been changed, so no CFLAGS are > required. that doesn't really answer my question. why do you need -fPIE at link time ? -mike
-----Original Message----- From: Mike Frysinger [mailto:vapier@gentoo.org] Sent: Monday, March 2, 2015 6:37 PM To: Petar Jovanovic Cc: libc-alpha@sourceware.org; petar.jovanovic@imgtec.com Subject: Re: [PATCH] Fix dynamic linker issue with bind-now On 02 Mar 2015 17:19, Petar Jovanovic wrote: > From: Mike Frysinger [mailto:vapier@gentoo.org] > > shouldn't the source file be compiled w/-fPIE too ? > > In the new version of the patch, this has been changed, so no CFLAGS > are required. > that doesn't really answer my question. why do you need -fPIE at link time ? > -mike This came from the original example, but it is not required. It can be removed. Thanks for noticing this. Regards, Petar
On Mon, 2 Mar 2015, Mike Frysinger wrote: > > +tst-split-dynreloc-ENV = LD_BIND_NOW=1 > > do you need to use the env var ? pass -Wl,-z,now via LDFLAGS instead ? FWIW the two arrangements are not equivalent AFAIK, a binary created with `-Wl,-z,now' may not be the same as one made without that option (beyond the obvious difference coming from the absence or the presence of the DT_BIND_NOW dynamic tag), and consequently different ld.so's execution paths may be exercised respectively. NB this static linker's property may be target specific and I haven't looked into the details of the issue concerned here, so this observation may not be relevant here. I think it's worth noting anyway just in case, as I had unpleasant surprises in the past with it, so it would be good to save other people from falling into this trap too. Maciej
diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h index 7b3e295..d5dea8e 100644 --- a/elf/dynamic-link.h +++ b/elf/dynamic-link.h @@ -133,7 +133,9 @@ elf_machine_lazy_rel (struct link_map *map, \ if (ranges[0].start + ranges[0].size == (start + size)) \ ranges[0].size -= size; \ - if (! ELF_DURING_STARTUP && ((do_lazy) || ranges[0].size == 0)) \ + if (! ELF_DURING_STARTUP \