1423821 - elfhack busts lld-linked binaries

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Mike Hommey [:glandium]]

(Found while trying to make elfhack -r work with lld-linked binaries)
STR:
- Take one of the .so files attached to bug 1385783
- Run elfhack on it

Observe the ELF program headers before and after:

Before:
  PHDR           0x000040 0x0000000000000040 0x0000000000000040 0x000230 0x000230 R   0x8
  LOAD           0x000000 0x0000000000000000 0x0000000000000000 0x0042e8 0x0042e8 R   0x1000
  LOAD           0x005000 0x0000000000005000 0x0000000000005000 0x003040 0x003040 R E 0x1000
  LOAD           0x009000 0x0000000000009000 0x0000000000009000 0x0021c8 0x003010 RW  0x1000
  TLS            0x00a3a0 0x000000000000a3a0 0x000000000000a3a0 0x000000 0x001010 R   0x10
  DYNAMIC        0x00b010 0x000000000000b010 0x000000000000b010 0x0001a0 0x0001a0 RW  0x8
  GNU_RELRO      0x00b000 0x000000000000b000 0x000000000000b000 0x0001c8 0x001000 R   0x1
  GNU_EH_FRAME   0x004240 0x0000000000004240 0x0000000000004240 0x000024 0x000024 R   0x1
  GNU_STACK      0x000000 0x0000000000000000 0x0000000000000000 0x000000 0x000000 RW  0
  NOTE           0x00046e 0x000000000000046e 0x000000000000046e 0x000018 0x000018 R   0x1

After:
  PHDR           0x000040 0x0000000000000040 0x0000000000000040 0x000268 0x000268 R   0x8
  LOAD           0x000000 0x0000000000000000 0x0000000000000000 0x000810 0x000810 R   0x1000
  LOAD           0x001240 0x0000000000004240 0x0000000000004240 0x0000a8 0x0000a8 R   0x1000
  LOAD           0x0012e8 0x00000000000042e8 0x00000000000042e8 0x003d58 0x003d58 R E 0x1000
  LOAD           0x006000 0x0000000000009000 0x0000000000009000 0x0021c8 0x003010 RW  0x1000
  TLS            0x0073a0 0x000000000000a3a0 0x000000000000a3a0 0x000000 0x001010 R   0x10
  DYNAMIC        0x008010 0x000000000000b010 0x000000000000b010 0x0001a0 0x0001a0 RW  0x8
  GNU_RELRO      0x008000 0x000000000000b000 0x000000000000b000 0x0001c8 0x001000 R   0x1
  GNU_EH_FRAME   0x001240 0x0000000000004240 0x0000000000004240 0x000024 0x000024 R   0x1
  GNU_STACK      0x000000 0x0000000000000000 0x0000000000000000 0x000000 0x000000 RW  0
  LOPROC+0x36968 0x6f00747365740061 0x676e697274730066 0x6600796172726100 0x770065737500726f 0x68666c6500687469     0x73006f74006b6361

The problem is that elfhack is adding a program header item, growing the program header by 56 bytes, but the data section that directly follows that is .rodata, which can't have its address moved (lld is being too smart for its own good).

I don't want to pretend elfhack works properly considering this, so for now, until we can figure a way to make it work, we'll have to actively make this fail.

Comment 1

[Mike Hommey [:glandium]]

Attachment: Bug 1423821 - Add a consistency check for section offsets to elfhack.

lld is being too smart for its own good, and places non-relocatable data
right after the program headers, which prevents the program headers from
growing. But elfhack wasn't checking for that, so happily placed the
non-relocatable data at its non-relocated location, overwriting the last
item of the program headers.

Review commit: https://reviewboard.mozilla.org/r/206174/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/206174/

Comment 2

[Mike Hommey [:glandium]]

Nathan, please review when you're available.

Comment 3

[Nathan Froyd [:froydnj]]

Comment on attachment 8935271 [details]
Bug 1423821 - Add a consistency check for section offsets to elfhack.

https://reviewboard.mozilla.org/r/206174/#review212980

::: build/unix/elfhack/elf.cpp:428
(Diff revision 1)
> +
> +    // Check sections consistency
> +    unsigned int minOffset = 0;
> +    for (ElfSection *section = ehdr; section != nullptr; section = section->getNext()) {
> +        unsigned int offset = section->getOffset();
> +        assert(offset >= minOffset);

I guess we're relying on this assert to fire if things go wrong?  What happens if that assert doesn't exist because we're compiling without optimizations?  I suppose that doesn't typically happen for host code, perhaps?

Maybe we should just explicitly `fprintf` and `exit(-1)` here, rather than relying on `assert`?

Comment 4

[Nathan Froyd [:froydnj]]

Being more explicit about exiting would be preferred, IMHO.

Comment 5

[Mike Hommey [:glandium]]

Comment on attachment 8935271 [details]
Bug 1423821 - Add a consistency check for section offsets to elfhack.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/206174/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9d93751d55cb
Add a consistency check for section offsets to elfhack. r=froydnj

Comment 7

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/9d93751d55cb