code review 4808043: ld: remove overlap of ELF sections on dynamic binaries

ld: remove overlap of ELF sections on dynamic binaries

The dynamic ELF sections were pointing to the proper data,
but that data was already owned by the rodata and text sections.
Some ELF references explicitly prohibit multiple sections from
owning the same data, and strip behaves accordingly.

The data for these sections was moved out and their ranges are
now owned by their respective sections.  This change makes strip
happy both with and without -s being provided at link time.

A test was added in debug/elf to ensure there are no regressions
on this area in the future.

Fixes issue 1242.
Fixes issue 2022.

NOTE: Tested on Linux amd64/386/arm only.

[niemeyer, 2011-07-20 13:22:04]

Hello rsc@golang.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[rsc, 2011-07-20 14:51:36]

http://codereview.appspot.com/4808043/diff/6001/src/cmd/5l/asm.c
File src/cmd/5l/asm.c (left):

http://codereview.appspot.com/4808043/diff/6001/src/cmd/5l/asm.c#oldcode638
src/cmd/5l/asm.c:638: diag("ELFRESERVE too small: %d > %d", a, ELFRESERVE);
Why is this check gone?
Should still have if(a > ELFRESERVE)

http://codereview.appspot.com/4808043/diff/6001/src/cmd/5l/asm.c
File src/cmd/5l/asm.c (right):

http://codereview.appspot.com/4808043/diff/6001/src/cmd/5l/asm.c#newcode228
src/cmd/5l/asm.c:228: s->type = SELFSECT;
preserve comment
// writable

http://codereview.appspot.com/4808043/diff/6001/src/cmd/5l/asm.c#newcode280
src/cmd/5l/asm.c:280: vlong addr = symaddr(s);
separate decl + init

vlong addr;

addr = symaddr(s);

http://codereview.appspot.com/4808043/diff/6001/src/cmd/6l/asm.c
File src/cmd/6l/asm.c (left):

http://codereview.appspot.com/4808043/diff/6001/src/cmd/6l/asm.c#oldcode1072
src/cmd/6l/asm.c:1072: if(a+elfwriteinterp() > ELFRESERVE)
same

http://codereview.appspot.com/4808043/diff/6001/src/cmd/6l/asm.c
File src/cmd/6l/asm.c (right):

http://codereview.appspot.com/4808043/diff/6001/src/cmd/6l/asm.c#newcode641
src/cmd/6l/asm.c:641: s->type = SELFSECT;
same

http://codereview.appspot.com/4808043/diff/6001/src/cmd/6l/asm.c#newcode692
src/cmd/6l/asm.c:692: vlong addr = symaddr(s);
same

http://codereview.appspot.com/4808043/diff/6001/src/cmd/8l/asm.c
File src/cmd/8l/asm.c (left):

http://codereview.appspot.com/4808043/diff/6001/src/cmd/8l/asm.c#oldcode1147
src/cmd/8l/asm.c:1147: if(a+elfwriteinterp() > ELFRESERVE)
same

http://codereview.appspot.com/4808043/diff/6001/src/cmd/8l/asm.c
File src/cmd/8l/asm.c (right):

http://codereview.appspot.com/4808043/diff/6001/src/cmd/8l/asm.c#newcode602
src/cmd/8l/asm.c:602: s->type = SELFSECT;
same

http://codereview.appspot.com/4808043/diff/6001/src/cmd/8l/asm.c#newcode652
src/cmd/8l/asm.c:652: vlong addr = symaddr(s);
same

[niemeyer, 2011-07-20 15:42:25]

Hello rsc@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[niemeyer, 2011-07-20 15:43:12]

All addressed, and also fixed a bug in 5l which had .got as read-only.

[rsc, 2011-07-20 15:43:54]

LGTM

[niemeyer, 2011-07-20 15:47:08]

*** Submitted as http://code.google.com/p/go/source/detail?r=5fbd69feecd3 ***

ld: remove overlap of ELF sections on dynamic binaries

The dynamic ELF sections were pointing to the proper data,
but that data was already owned by the rodata and text sections.
Some ELF references explicitly prohibit multiple sections from
owning the same data, and strip behaves accordingly.

The data for these sections was moved out and their ranges are
now owned by their respective sections.  This change makes strip
happy both with and without -s being provided at link time.

A test was added in debug/elf to ensure there are no regressions
on this area in the future.

Fixes issue 1242.
Fixes issue 2022.

NOTE: Tested on Linux amd64/386/arm only.

R=rsc
CC=golang-dev
http://codereview.appspot.com/4808043

[gustavo_niemeyer.net, 2011-07-20 16:54:35]

> gopack grc _test/crypto/tls.a _gotest_.6
> gotest: ./6.out failed to start: fork/exec ./6.out: exec format error

Sorry about that.  If no one gets there before I do, I'll look for a
testing environment somewhere online later today to debug this.

-- 
Gustavo Niemeyer
http://niemeyer.net
http://niemeyer.net/plus
http://niemeyer.net/twitter
http://niemeyer.net/blog

-- I never filed a patent.

[r, 2011-07-21 01:29:26]

It's buggering the headers if the program links in cgo code. It looks
like the dynamic loader/interpreter stuff is bungled.  I don't know
much about this but I'm 99% sure that's where the problem lies.

From a modest sample of trials it looks like all non-cgo tests are
fine; all cgo tests generate bad 6.outs.

Here's a readelf -l output from before the change:


$ readelf -l GOOD6.out

Elf file type is EXEC (Executable file)
Entry point 0x4113be
There are 7 program headers, starting at offset 64

Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  PHDR           0x0000000000000040 0x0000000000400040 0x0000000000400040
                 0x0000000000000188 0x0000000000000188  R E    1000
  INTERP         0x0000000000000beb 0x0000000000400beb 0x0000000000400beb
                 0x0000000000000015 0x0000000000000015  R      1
      [Requesting program interpreter: /libexec/ld-elf.so.1]
  LOAD           0x0000000000000040 0x0000000000400040 0x0000000000400040
                 0x00000000001fd158 0x00000000001fd158  R E    1000
  LOAD           0x00000000001fe000 0x00000000005fe000 0x00000000005fe000
                 0x0000000000002790 0x0000000002025bf0  RW     1000
  DYNAMIC        0x00000000000c52e8 0x00000000004c52e8 0x00000000004c52e8
                 0x0000000000000120 0x0000000000000120  RW     8
  TLS            0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x0000000000000000 0x0000000000000010  R      8
  STACK          0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x0000000000000000 0x0000000000000000  RW     8

 Section to Segment mapping:
  Segment Sections...
   00
   01     .interp
   02     .interp .dynsym .dynstr .gnu.version .gnu.version_r
.rela.plt .plt .hash .rela .dynamic .text .rodata .gosymtab .gopclntab
   03     .got .got.plt .data .bss
   04     .dynamic
   05
   06
$

Here's after:


Elf file type is EXEC (Executable file)
Entry point 0x4113be
There are 7 program headers, starting at offset 64

Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  PHDR           0x0000000000000040 0x0000000000400040 0x0000000000400040
                 0x0000000000000188 0x0000000000000188  R E    1000
  INTERP         0x00000000001faf20 0x00000000005faf20 0x00000000005faf20
                 0x0000000000000015 0x0000000000000015  R      1
      [Requesting program interpreter: /libexec/ld-elf.so.1]
  LOAD           0x0000000000000040 0x0000000000400040 0x0000000000400040
                 0x00000000001fb778 0x00000000001fb778  R E    1000
  LOAD           0x00000000001fc000 0x00000000005fc000 0x00000000005fc000
                 0x0000000000002818 0x0000000002025c78  RW     1000
  DYNAMIC        0x00000000001fb220 0x00000000005fb220 0x00000000005fb220
                 0x0000000000000120 0x0000000000000120  RW     8
  TLS            0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x0000000000000000 0x0000000000000010  R      8
  STACK          0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x0000000000000000 0x0000000000000000  RW     8

 Section to Segment mapping:
  Segment Sections...
   00
   01     .interp
   02     .interp .dynsym .dynstr .gnu.version .gnu.version_r
.rela.plt .plt .hash .rela .dynamic .shstrtab .text .rodata .gosymtab
.gopclntab
   03     .got .got.plt .data .bss
   04     .dynamic
   05
   06
$

Not sure if it's the problem but the interpreter segment (INTERP) has
moved very far into the binary. Wouldn't be surprised if that's
trouble or at least a big clue.

Leaving for someone who understands ELF better.

Please fix this soon.

-rob

[gustavo_niemeyer.net, 2011-07-21 01:56:11]

> It's buggering the headers if the program links in cgo code. It looks
> like the dynamic loader/interpreter stuff is bungled.  I don't know
> much about this but I'm 99% sure that's where the problem lies.

I've been investigating it for the past couple of hours, and I'm just
debugging the case of .shstrtab. .shstrtab is a section that is
generally not allocated, but was injected in .text.  The manual says
sections in the segment are supposed to be contiguous, and they were
because .shstrtab was loaded even though there was no reason to, but
now that it owns its data range that's not the case anymore.  Note how
the bad segment shows .shstrtab interspersed.  This is certainly
bogus, but I'm not yet sure if it's the problem right now.

> Not sure if it's te problem but the interpreter segment (INTERP) has
> moved very far into the binary. Wouldn't be surprised if that's
> trouble or at least a big clue.

Will check this out.

> Leaving for someone who understands ELF better.
>
> Please fix this soon.

I will spend some more time on this now and if I don't get to a
solution before the end of my day will submit a CL reverting the
change so that I can sort it out without affecting others.

Sorry for the trouble.

-- 
Gustavo Niemeyer
http://niemeyer.net
http://niemeyer.net/plus
http://niemeyer.net/twitter
http://niemeyer.net/blog

-- I never filed a patent.

[r2, 2011-07-21 01:58:56]

It could be both, of course. I believe FreeBSD needs more stuff to be in the
first block, and that is likely to include the interp string.

Thanks for digging in.

-rob

[gustavo_niemeyer.net, 2011-07-21 03:33:17]

> It could be both, of course. I believe FreeBSD needs more stuff to be in the
first block, and that is likely to include the interp string.

When I grow up I want to understand that as well. You are right.

I'll leave the .shstrtab alone for now, as it seems to not care
anyway, and will fix interp and test on the *5* platforms (Linux
amd64/386/arm + FreeBSD amd64/386), and push the fix.

I'll continue addressing these other minors later, but might not be
able to do that tomorrow as I have a conference on Friday and have no
slides yet.

Sorry again for the hiccup.

-- 
Gustavo Niemeyer
http://niemeyer.net
http://niemeyer.net/plus
http://niemeyer.net/twitter
http://niemeyer.net/blog

-- I never filed a patent.

[gustavo_niemeyer.net, 2011-07-21 06:04:42]

> I'll leave the .shstrtab alone for now, as it seems to not care
> anyway, and will fix interp and test on the *5* platforms (Linux
> amd64/386/arm + FreeBSD amd64/386), and push the fix.

This is the fix being baked:

    http://codereview.appspot.com/4794046

Works in Linux 386/amd64, and FreeBSD amd64.  Linux arm is in the
queue for testing (takes quite some time), and I'm having issues with
FreeBSD 9.0 386.  That said, the issue I have with FreeBSD 9.0 386
(which is the one I have access to) seems to be reproducible even with
revision 9183:fd042c825740 right before the change.

So, if arm confirms to pass, and the FreeBSD issue is indeed something
else that was already broken, this should fix the build and preserve
the removal of section overlaps.

-- 
Gustavo Niemeyer
http://niemeyer.net
http://niemeyer.net/plus
http://niemeyer.net/twitter
http://niemeyer.net/blog

-- I never filed a patent.

[r2, 2011-07-21 06:14:43]

seems to fix it for amd64 freebsd 8.0

-rob