[PATCH] Fix dynamic linker issue with bind-now

[PATCH] Fix dynamic linker issue with bind-now

[Petar Jovanovic]

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(-)

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))                   \
 	  {								      \
 	    ranges[1].start = start;					      \
 	    ranges[1].size = size;					      \
-- 
1.7.9.5

Dynamic linker issue with bind now?

[Petar Jovanovic]

Hello everyone,

Is it possible that someone familiar enough with elf/dynamic-link.h
takes a look at _ELF_DYNAMIC_DO_RELOC macro?
I am under impression that the macro does not correctly handle a case
when DT_REL* and DT_JMPREL are completely separated and there is a gap
between them.
Previously, David S. Miller's patch [1] aimed to fix that case.

Ergo, the code will work in the lazy-bound case, but not in the
bind-now case, and DT_JMPREL section will not be processed. In a
particular case that I have come across, this caused a crash in
_PROCEDURE_LINKAGE_TABLE_, since plt entry for libc_start_main function
did not get updated.

If my assumptions are correct, I would propose a patch that will resolve
the case. Otherwise, I would appreciate explanation how that case gets
resolved correctly.

Thank you.

Regards,
Petar

[1] ld.so regression patch,
https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=d7dd44133f53e8bcc81e18c11694bee985cd86d0

Petar Jovanovic (1):
  Fix dynamic linker issue with bind-now

 elf/dynamic-link.h |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

-- 
1.7.9.5

Re: Dynamic linker issue with bind now?

[Roland McGrath]

You need to supply a test case.

RE: Dynamic linker issue with bind now?

[Petar Jovanovic]

Hi Roland,

Building/running content_shell for MIPS32 from Chromium ToT will trigger
this
issue.

Here is the output from:

$ mipsel-linux-gnu-readelf -d out/Release/content_shell

Dynamic section at offset 0x1a4 contains 68 entries:
  Tag        Type                         Name/Value
 0x00000001 (NEEDED)                     Shared library: [libfreetype.so.6]
 0x00000001 (NEEDED)                     Shared library: [librt.so.1]
 0x00000001 (NEEDED)                     Shared library: [libdl.so.2]
 0x00000001 (NEEDED)                     Shared library:
[libgmodule-2.0.so.0]
 0x00000001 (NEEDED)                     Shared library:
[libgobject-2.0.so.0]
 0x00000001 (NEEDED)                     Shared library: [libglib-2.0.so.0]
 0x00000001 (NEEDED)                     Shared library: [libnss3.so]
 0x00000001 (NEEDED)                     Shared library: [libnssutil3.so]
 0x00000001 (NEEDED)                     Shared library: [libsmime3.so]
 0x00000001 (NEEDED)                     Shared library: [libplc4.so]
 0x00000001 (NEEDED)                     Shared library: [libnspr4.so]
 0x00000001 (NEEDED)                     Shared library: [libgconf-2.so.4]
 0x00000001 (NEEDED)                     Shared library:
[libfontconfig.so.1]
 0x00000001 (NEEDED)                     Shared library:
[libpangocairo-1.0.so.0]
 0x00000001 (NEEDED)                     Shared library: [libcairo.so.2]
 0x00000001 (NEEDED)                     Shared library: [libpango-1.0.so.0]
 0x00000001 (NEEDED)                     Shared library: [libX11.so.6]
 0x00000001 (NEEDED)                     Shared library: [libXi.so.6]
 0x00000001 (NEEDED)                     Shared library: [libXcursor.so.1]
 0x00000001 (NEEDED)                     Shared library: [libXext.so.6]
 0x00000001 (NEEDED)                     Shared library: [libXfixes.so.3]
 0x00000001 (NEEDED)                     Shared library: [libXrender.so.1]
 0x00000001 (NEEDED)                     Shared library:
[libXcomposite.so.1]
 0x00000001 (NEEDED)                     Shared library: [libasound.so.2]
 0x00000001 (NEEDED)                     Shared library: [libXdamage.so.1]
 0x00000001 (NEEDED)                     Shared library: [libXtst.so.6]
 0x00000001 (NEEDED)                     Shared library: [libXrandr.so.2]
 0x00000001 (NEEDED)                     Shared library: [libexpat.so.1]
 0x00000001 (NEEDED)                     Shared library: [libpthread.so.0]
 0x00000001 (NEEDED)                     Shared library: [libcap.so.2]
 0x00000001 (NEEDED)                     Shared library: [libudev.so.0]
 0x00000001 (NEEDED)                     Shared library: [libdbus-1.so.3]
 0x00000001 (NEEDED)                     Shared library: [libstdc++.so.6]
 0x00000001 (NEEDED)                     Shared library: [libm.so.6]
 0x00000001 (NEEDED)                     Shared library: [libgcc_s.so.1]
 0x00000001 (NEEDED)                     Shared library: [libc.so.6]
 0x00000001 (NEEDED)                     Shared library: [ld.so.1]
 0x0000000f (RPATH)                      Library rpath: [$ORIGIN/lib/]
 0x0000000c (INIT)                       0x7cb45c
 0x0000000d (FINI)                       0x40039b0
 0x00000004 (HASH)                       0x4003ec
 0x00000005 (STRTAB)                     0x4cf098
 0x00000006 (SYMTAB)                     0x431998
 0x0000000a (STRSZ)                      2283769 (bytes)
 0x0000000b (SYMENT)                     16 (bytes)
 0x70000016 (MIPS_RLD_MAP)               0x4dbb498
 0x00000015 (DEBUG)                      0x0
 0x00000003 (PLTGOT)                     0x4dbb4a0
 0x00000011 (REL)                        0x7108e4
 0x00000012 (RELSZ)                      59800 (bytes)
 0x00000013 (RELENT)                     8 (bytes)
 0x70000001 (MIPS_RLD_VERSION)           1
 0x70000005 (MIPS_FLAGS)                 NOTPOT
 0x70000006 (MIPS_BASE_ADDRESS)          0x400000
 0x7000000a (MIPS_LOCAL_GOTNO)           15719
 0x70000011 (MIPS_SYMTABNO)              40304
 0x70000012 (MIPS_UNREFEXTNO)            41
 0x70000013 (MIPS_GOTSYM)                0x97fd
 0x00000014 (PLTREL)                     REL
 0x00000017 (JMPREL)                     0x7cb454
 0x00000002 (PLTRELSZ)                   8 (bytes)
 0x70000032 (MIPS_PLTGOT)                0x4da7ff0
 0x00000018 (BIND_NOW)                   
 0x6ffffffb (FLAGS_1)                    Flags: NOW
 0x6ffffffe (VERNEED)                    0x710474
 0x6fffffff (VERNEEDNUM)                 11
 0x6ffffff0 (VERSYM)                     0x6fc992
 0x00000000 (NULL)                       0x0


Regards,
Petar

-----Original Message-----
From: Roland McGrath [mailto:roland@hack.frob.com] 
Sent: Wednesday, August 20, 2014 7:08 PM
To: Petar Jovanovic
Cc: libc-alpha@sourceware.org; petar.jovanovic@imgtec.com;
davem@davemloft.net
Subject: Re: Dynamic linker issue with bind now?

You need to supply a test case.

Re: Dynamic linker issue with bind now?

[Rich Felker]

On Wed, Aug 20, 2014 at 08:19:10PM +0200, Petar Jovanovic wrote:
> Hi Roland,
> 
> Building/running content_shell for MIPS32 from Chromium ToT will trigger
> this
> issue.

I think Roland was asking for a minimal test-case to reproduce the
issue and demonstrate that the patch fixes it, not a 10GB test case
where you originally experienced the problem... :-)

Rich

RE: Dynamic linker issue with bind now?

[Petar Jovanovic]

Understood, but since Roland is working in Chromium team too, it made sense
to mention that.

Also, minimal case may require some tricks or hacks, since the layout is
different for trivial examples.

Regards,
Petar

-----Original Message-----
From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich Felker
Sent: Wednesday, August 20, 2014 9:00 PM
To: Petar Jovanovic
Cc: 'Roland McGrath'; libc-alpha@sourceware.org; petar.jovanovic@imgtec.com;
davem@davemloft.net
Subject: Re: Dynamic linker issue with bind now?

On Wed, Aug 20, 2014 at 08:19:10PM +0200, Petar Jovanovic wrote:
> Hi Roland,
> 
> Building/running content_shell for MIPS32 from Chromium ToT will 
> trigger this issue.

I think Roland was asking for a minimal test-case to reproduce the issue and
demonstrate that the patch fixes it, not a 10GB test case where you
originally experienced the problem... :-)

Rich

Re: Dynamic linker issue with bind now?

['Rich Felker']

On Wed, Aug 20, 2014 at 09:21:29PM +0200, Petar Jovanovic wrote:
> Understood, but since Roland is working in Chromium team too, it made sense
> to mention that.
> 
> Also, minimal case may require some tricks or hacks, since the layout is
> different for trivial examples.

I'm guessing it should be possible to write a linker script to
generate a minimal binary that exhibits the issue, but I don't know
enough about linker scripts to give you any details on how.

Rich

RE: Dynamic linker issue with bind now?

[Petar Jovanovic]

> I'm guessing it should be possible to write a linker script to
> generate a minimal binary that exhibits the issue,

Yes, that is what I had in mind as well.
Speaking of which, I have just found out that I am not the first one to
report this issue and propose (this) patch.

The issue was reported [1] two years ago, and the issue page has a
small test case. For your convenience, I will copy it here:

$ cat > test.c <<EOF
#include <stdio.h>

static int __attribute__((section(".bar"))) bar = 0x12345678;

static const char foo[] = "foo";

int main() {
  printf("%s\n", foo);
  return 0;
}
EOF

$ cat > script <<EOF
SECTIONS
{
   .bar : { *(.bar) }
}
INSERT AFTER .rela.dyn;
EOF

$ gcc -o test test.c -fPIE -Wl,-T,script
$ LD_BIND_NOW=1 ./test
./test: error while loading shared libraries: unexpected reloc type
0x006008a0

I have just double-checked, and the test case is still valid (and fixed
with the patch). Let me know what you think.

Thank you.

Regards,
Petar

[1] Dynamic linker crash when DT_JMPREL and DT_REL{,A} are not contiguous,
https://sourceware.org/bugzilla/show_bug.cgi?id=14341

RE: [PATCH] Fix dynamic linker issue with bind-now

[Petar Jovanovic]

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(-)

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))
\
 	  {
\
 	    ranges[1].start = start;
\
 	    ranges[1].size = size;
\
-- 
1.7.9.5

RE: [PATCH] Fix dynamic linker issue with bind-now

[Petar Jovanovic]

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(-)

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))
\
 	  {
\
 	    ranges[1].start = start;
\
 	    ranges[1].size = size;
\
--
1.7.9.5

Re: [PATCH] Fix dynamic linker issue with bind-now

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 317 bytes --]

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [PATCH] Fix dynamic linker issue with bind-now

[Petar Jovanovic]

[-- Attachment #1: Type: text/plain, Size: 797 bytes --]

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

[-- Attachment #2: fix_dynamic_link.diff --]
[-- Type: application/octet-stream, Size: 2698 bytes --]

From 08bf60f1bb4d6d9aae63e80d0dfd0884ad812659 Mon Sep 17 00:00:00 2001
From: Petar Jovanovic <petar.jovanovic@rt-rk.com>
Date: Fri, 31 Oct 2014 03:26:48 +0100
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/Makefile             |    7 ++++++-
 elf/dynamic-link.h       |    4 +++-
 elf/tst-dynamic-link.c   |   11 +++++++++++
 elf/tst-dynamic-link.lds |    6 ++++++
 4 files changed, 26 insertions(+), 2 deletions(-)
 create mode 100644 elf/tst-dynamic-link.c
 create mode 100644 elf/tst-dynamic-link.lds

diff --git a/elf/Makefile b/elf/Makefile
index 94074f3..1fb2ce5 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -146,8 +146,13 @@ tests += loadtest restest1 preloadtest loadfail multiload origtest resolvfail \
 	 tst-stackguard1 tst-addr1 tst-thrlock \
 	 tst-unique1 tst-unique2 tst-unique3 tst-unique4 \
 	 tst-initorder tst-initorder2 tst-relsort1 tst-null-argv \
-	 tst-ptrguard1
+	 tst-ptrguard1 tst-dynamic-link
 #	 reldep9
+
+LDFLAGS-tst-dynamic-link = -fPIE -Wl,-T,tst-dynamic-link.lds
+CFLAGS-tst-dynamic-link.c = -O0 -DIS_IN_build
+tst-dynamic-link-ENV = LD_BIND_NOW=1
+
 ifeq ($(build-hardcoded-path-in-tests),yes)
 tests += tst-dlopen-aout
 endif
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))                   \
 	  {								      \
 	    ranges[1].start = start;					      \
 	    ranges[1].size = size;					      \
diff --git a/elf/tst-dynamic-link.c b/elf/tst-dynamic-link.c
new file mode 100644
index 0000000..b9953a2
--- /dev/null
+++ b/elf/tst-dynamic-link.c
@@ -0,0 +1,11 @@
+#include <stdio.h>
+
+static int __attribute__((section(".bar"))) bar = 0x12345678;
+
+static const char foo[] = "foo";
+
+int main() {
+  printf("%s\n", foo);
+  return 0;
+}
+
diff --git a/elf/tst-dynamic-link.lds b/elf/tst-dynamic-link.lds
new file mode 100644
index 0000000..ed0a656
--- /dev/null
+++ b/elf/tst-dynamic-link.lds
@@ -0,0 +1,6 @@
+SECTIONS
+{
+   .bar : { *(.bar) }
+}
+INSERT AFTER .rela.dyn;
+
-- 
1.7.9.5

RE: [PATCH] Fix dynamic linker issue with bind-now

[Petar Jovanovic]

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

RE: [PATCH] Fix dynamic linker issue with bind-now

[Petar Jovanovic]

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

Re: [PATCH] Fix dynamic linker issue with bind-now

[Will Newton]

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

RE: [PATCH] Fix dynamic linker issue with bind-now

[Petar Jovanovic]

[-- Attachment #1: Type: text/plain, Size: 3015 bytes --]

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

[-- Attachment #2: fix_dynamic_link2.diff --]
[-- Type: application/octet-stream, Size: 3258 bytes --]

From 38cdb4d833dba5160edd78689d60fbe28b43c24d Mon Sep 17 00:00:00 2001
From: Petar Jovanovic <petar.jovanovic@rt-rk.com>
Date: Fri, 31 Oct 2014 03:26:48 +0100
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/Makefile               |    7 ++++++-
 elf/dynamic-link.h         |    4 +++-
 elf/tst-split-dynreloc.c   |   27 +++++++++++++++++++++++++++
 elf/tst-split-dynreloc.lds |    6 ++++++
 4 files changed, 42 insertions(+), 2 deletions(-)
 create mode 100644 elf/tst-split-dynreloc.c
 create mode 100644 elf/tst-split-dynreloc.lds

diff --git a/elf/Makefile b/elf/Makefile
index 9e07073..cf8046b 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -146,8 +146,13 @@ tests += loadtest restest1 preloadtest loadfail multiload origtest resolvfail \
 	 tst-stackguard1 tst-addr1 tst-thrlock \
 	 tst-unique1 tst-unique2 tst-unique3 tst-unique4 \
 	 tst-initorder tst-initorder2 tst-relsort1 tst-null-argv \
-	 tst-ptrguard1
+	 tst-ptrguard1 tst-split-dynreloc
 #	 reldep9
+
+LDFLAGS-tst-split-dynreloc = -fPIE -Wl,-T,tst-split-dynreloc.lds
+CFLAGS-tst-split-dynreloc.c = -O0 -DIS_IN_build
+tst-split-dynreloc-ENV = LD_BIND_NOW=1
+
 ifeq ($(build-hardcoded-path-in-tests),yes)
 tests += tst-dlopen-aout
 endif
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))                   \
 	  {								      \
 	    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..32d474f
--- /dev/null
+++ b/elf/tst-split-dynreloc.c
@@ -0,0 +1,27 @@
+#include <stdio.h>
+
+static int __attribute__((section(".bar"))) bar = 0x12345678;
+
+static const char foo[] = "foo";
+
+/*
+ *  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.
+ */
+int main() {
+  printf("%s\n", foo);
+  return 0;
+}
+
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;
+
-- 
1.7.9.5

Re: [PATCH] Fix dynamic linker issue with bind-now

[Andreas Schwab]

"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."

RE: [PATCH] Fix dynamic linker issue with bind-now

[Petar Jovanovic]

[-- Attachment #1: Type: text/plain, Size: 2565 bytes --]

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."

[-- Attachment #2: fix_dynamic_link3.diff --]
[-- Type: application/octet-stream, Size: 3160 bytes --]

From aa779d3eb18a570b2d886c9a49d5022b27895272 Mon Sep 17 00:00:00 2001
From: Petar Jovanovic <petar.jovanovic@rt-rk.com>
Date: Fri, 31 Oct 2014 03:26:48 +0100
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/Makefile               |    7 ++++++-
 elf/dynamic-link.h         |    4 +++-
 elf/tst-split-dynreloc.c   |   27 +++++++++++++++++++++++++++
 elf/tst-split-dynreloc.lds |    6 ++++++
 4 files changed, 42 insertions(+), 2 deletions(-)
 create mode 100644 elf/tst-split-dynreloc.c
 create mode 100644 elf/tst-split-dynreloc.lds

diff --git a/elf/Makefile b/elf/Makefile
index 9e07073..cf8046b 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -146,8 +146,13 @@ tests += loadtest restest1 preloadtest loadfail multiload origtest resolvfail \
 	 tst-stackguard1 tst-addr1 tst-thrlock \
 	 tst-unique1 tst-unique2 tst-unique3 tst-unique4 \
 	 tst-initorder tst-initorder2 tst-relsort1 tst-null-argv \
-	 tst-ptrguard1
+	 tst-ptrguard1 tst-split-dynreloc
 #	 reldep9
+
+LDFLAGS-tst-split-dynreloc = -fPIE -Wl,-T,tst-split-dynreloc.lds
+CFLAGS-tst-split-dynreloc.c = -O0 -DIS_IN_build
+tst-split-dynreloc-ENV = LD_BIND_NOW=1
+
 ifeq ($(build-hardcoded-path-in-tests),yes)
 tests += tst-dlopen-aout
 endif
diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h
index 7b3e295..b8daefe 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))		      \
 	  {								      \
 	    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..16d85cd
--- /dev/null
+++ b/elf/tst-split-dynreloc.c
@@ -0,0 +1,27 @@
+#include <stdio.h>
+
+static int __attribute__ ((section(".bar"))) bar = 0x12345678;
+
+static const char foo[] = "foo";
+
+/* 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.  */
+
+int
+main (void)
+{
+  printf ("%s\n", foo);
+  return 0;
+}
+
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;
+
-- 
1.7.9.5

RE: [PATCH] Fix dynamic linker issue with bind-now

[Petar Jovanovic]

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."

RE: [PATCH] Fix dynamic linker issue with bind-now

[Petar Jovanovic]

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."

RE: [PATCH] Fix dynamic linker issue with bind-now

[Petar Jovanovic]

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."

RE: [PATCH] Fix dynamic linker issue with bind-now

[Petar Jovanovic]

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."

RE: [PATCH] Fix dynamic linker issue with bind-now

[Petar Jovanovic]

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."

Re: [PATCH] Fix dynamic linker issue with bind-now

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]

> --- 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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [PATCH] Fix dynamic linker issue with bind-now

[Petar Jovanovic]

-----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

Re: [PATCH] Fix dynamic linker issue with bind-now

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 336 bytes --]

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [PATCH] Fix dynamic linker issue with bind-now

[Petar Jovanovic]

-----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

Re: [PATCH] Fix dynamic linker issue with bind-now

[Maciej W. Rozycki]

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