[PATCH] D56828: [ELF] Simplify RelRo, TLS, NOBITS section ranks and make RW PT_LOAD start with RelRo

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D56828#1416503 <https://reviews.llvm.org/D56828#1416503>, @pcc wrote:

> > I agree that in -z relro mode (default), when either .data or .bss exists (almost always true), the contents of .bss.rel.ro are embedded (as if it is .data.rel.ro; this is used by newer ld.bfd/gold). However, since .bss.rel.ro is small, this doesn't matter.
>
> Not necessarily. As I mentioned in my earlier comment, you could have a second PT_LOAD for relro.
>
> e.g. with the following layout:
>
>   .data.rel.ro (vaddr 0x3000 size 0x1000)
>   .bss.rel.ro (vaddr 0x4000 size 0x1000)
>   .data (vaddr 0x5000 size 0x1000)
>   .bss (vaddr 0x6000 size 0x1000)
>
>
> You could lay out `.data` at file offset 0x4000 and then have the following program headers:
>
>   PT_LOAD rw p_offset = 0x3000 p_vaddr = 0x3000 p_filesz = 0x1000 p_memsz = 0x2000
>   PT_LOAD rw p_offset = 0x4000 p_vaddr = 0x5000 p_filesz = 0x1000 p_memsz = 0x2000
>   PT_GNU_RELRO p_offset = 0x3000 p_vaddr = 0x3000 p_filesz = 0x1000 p_memsz = 0x2000
>
>
> I wouldn't expect `.bss.rel.ro` to always be small in the future. One thing that we could use it for is to compress the storage of relro sections which do not contain data, only relocations (a good example of this is most vtables in position-independent binaries). If a relro section's statically relocated data is all zeros, we could move it into `.bss.rel.ro`. Then the section itself would not take up space in the output file, only its relocations would.


Thanks for the suggestion and pointing out the improvement direction of `.bss.rel.ro` ("If a relro section's statically relocated data is all zeros, we could move it into `.bss.rel.ro`")! The idea is illustrated below:

Old: PT_LOAD(PT_GNU_RELRO(.data.rel.ro .bss.rel.ro) .data .bss)
New: PT_LOAD(PT_GNU_RELRO(.data.rel.ro .bss.rel.ro)) PT_LOAD(.data. .bss)

I've created https://reviews.llvm.org/D58892 for the idea. This patch alone requires 116 tests. Do you favor the new layout?

If yes, once this is accepted, I can work on D58892 <https://reviews.llvm.org/D58892> and fix another 9 tests.


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56828/new/

https://reviews.llvm.org/D56828