This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v5] Fix dynamic linker issue with bind-now
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Petar Jovanovic <petar dot jovanovic at rt-rk dot com>
- Cc: GNU C Library <libc-alpha at sourceware dot org>, Roland McGrath <roland at hack dot frob dot com>, Mike Frysinger <vapier at gentoo dot org>
- Date: Tue, 14 Jul 2015 11:36:06 -0700
- Subject: Re: [PATCH v5] Fix dynamic linker issue with bind-now
- Authentication-results: sourceware.org; auth=none
- References: <1436883776-79869-1-git-send-email-petar dot jovanovic at rt-rk dot com>
On Tue, Jul 14, 2015 at 7:22 AM, Petar Jovanovic
<petar.jovanovic@rt-rk.com> wrote:
> Fix the bind-now case when DT_REL and DT_JMPREL sections are separate
> and there is a gap between them. This patch fixes bug #14341.
> ---
> v5:
> - Added a ChangeLog entry and bug number into commit message.
>
> v4:
> - Moved the Makefile part into sysdeps/x86_64/Makefile, so the test is
> executed for x86-64 only
>
> v3:
> - addressed comments raised by Mike Frysinger
> - use of test-skeleton.c
> - use -Wl,-z,now instead of LD_BIND_NOW=1
> - moved comments to the start of the test file
>
> v2:
> - addressed all comments raised by Andreas Schwab
>
> ChangeLog | 9 +++++++++
> elf/dynamic-link.h | 4 +++-
> elf/tst-split-dynreloc.c | 28 ++++++++++++++++++++++++++++
> elf/tst-split-dynreloc.lds | 6 ++++++
> sysdeps/x86_64/Makefile | 4 ++++
> 5 files changed, 50 insertions(+), 1 deletion(-)
> create mode 100644 elf/tst-split-dynreloc.c
> create mode 100644 elf/tst-split-dynreloc.lds
>
> diff --git a/ChangeLog b/ChangeLog
> index dfef5e0..594b774 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,12 @@
> +2015-07-14 Petar Jovanovic <petar.jovanovic@rt-rk.com>
> +
> + [BZ #14341]
> + * elf/dynamic-link.h (elf_machine_lazy_rel): Properly handle the
> + case when there is a gap between DT_REL and DT_JMPREL sections.
> + * elf/tst-split-dynreloc.c: New file.
> + * elf/tst-split-dynreloc.lds: New file.
> + * sysdeps/x86_64/Makefile: Add new test.
> +
Please put ChangeLog entry in your commit log, not in ChangeLog
directly. Otherwise, your patch may not apply. If you can't check
it in yourself,. please generate the patch with " gcc format-patch".
> 2015-04-01 Wilco Dijkstra <wdijkstr@arm.com>
>
> * sysdeps/aarch64/fpu/math_private.h
> diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h
> index 8d428e2..83e760b 100644
> --- a/elf/dynamic-link.h
> +++ b/elf/dynamic-link.h
> @@ -135,7 +135,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 \
Please put " ranges[0].size == 0" on separate line.
> + || ranges[0].start + ranges[0].size != start)) \
Please add () around "ranges[0].start + ranges[0].size".
> { \
> ranges[1].start = start; \
> ranges[1].size = size; \
> diff --git a/elf/tst-split-dynreloc.c b/elf/tst-split-dynreloc.c
> new file mode 100644
> index 0000000..bdb6b7c
> --- /dev/null
> +++ b/elf/tst-split-dynreloc.c
> @@ -0,0 +1,28 @@
> +/* 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. */
> +
> +#include <stdio.h>
> +
> +static int __attribute__ ((section(".bar"))) bar = 0x12345678;
Please make "bar" readonly to avoid writable and executable segment.
> +static const char foo[] = "foo";
> +
> +static int
> +do_test (void)
> +{
> + printf ("%s %d\n", foo, bar);
> + return 0;
> +}
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
> diff --git a/elf/tst-split-dynreloc.lds b/elf/tst-split-dynreloc.lds
> new file mode 100644
> index 0000000..ed0a656
> --- /dev/null
> +++ b/elf/tst-split-dynreloc.lds
> @@ -0,0 +1,6 @@
> +SECTIONS
> +{
> + .bar : { *(.bar) }
> +}
> +INSERT AFTER .rela.dyn;
> +
> diff --git a/sysdeps/x86_64/Makefile b/sysdeps/x86_64/Makefile
> index ef70a50..b9d949c 100644
> --- a/sysdeps/x86_64/Makefile
> +++ b/sysdeps/x86_64/Makefile
> @@ -38,6 +38,10 @@ tests += tst-audit3 tst-audit4 tst-audit5 tst-audit10
> ifeq (yes,$(config-cflags-avx))
> tests += tst-audit6 tst-audit7
> endif
> +
> +tests += tst-split-dynreloc
> +LDFLAGS-tst-split-dynreloc = -Wl,-T,tst-split-dynreloc.lds -Wl,-z,now
> +
> modules-names += tst-auditmod3a tst-auditmod3b \
> tst-auditmod4a tst-auditmod4b \
> tst-auditmod5a tst-auditmod5b \
Please verify your testcase with the linker from the current binutils master
branch. The new linker doesn't have DT_JMPREL at all when -z now is
used and the testcase works even without your patch. Please update
the testcase such that it always fails without the fix.
--
H.J.