[PATCH] D55864: [elfabi] Write program headers, .dynamic, .dynstr, and .shstrtab

Fri May 3 13:07:50 PDT 2019

jakehehrlich added a comment.

In D55864#1490016 <https://reviews.llvm.org/D55864#1490016>, @MaskRay wrote:

> Is llvm-elfabi used to create interface shared objects? Do you use it to create real(runnable) modules?
>
> If not and if section/segment specification is not required, it can be very simple.
>  You just need the symbol table and very little other information.
>  There is one place in lld that checks if the symbol is in a readonly segment (`addCopyRelSymbol`).
>  1 PT_LOAD(RWX) + 1 PT_GNU_RELRO works just fine. You don't even need PT_TLS - `STT_TLS` symbols can be placed in an arbitrary section. Linkers and various binutils don't validate `PT_TLS`.

Yes this is a stub reader/writer (also textual stubs which can't be linked against are a second format that has already been reviewed largely) but there are other concerns than just what works for lld, namely what works for llvm-readobj. I don't think its worth adding the PT_GNU_RELRO segment. What's the advantage? Also I'll get rid of the PT_TLS segment. The layout I have in the latest patch is

1 PT_LOAD (R)

  .dynstr
  .dynsym
  1 PT_DYNAMIC (R)
    .dynamic 
  .def
  .tls

What the flags are don't really matter too much to me. I wasn't aware of the linker checking for a readonly segment but I think that favors just making everything read only. I'll think about removing the .tls section as well.

>> Using a TLS nobits saves space
> 
> The best layout I can think of is:
>  `PT_LOAD(PT_GNU_RELRO(.data.rel.ro PT_TLS(.tdata | .tbss) .bss.rel.ro) | .data .bss)`

What do you need the PT_GNU_RELRO for? What is .tdata or .tbss for? What is .data.rel.ro for?

> Currently lld has (`.tbss` cannot really be SHT_NOBITS due to `.data.rel.ro`):
>  `PT_LOAD(PT_GNU_RELRO(PT_TLS(.tdata .tbss) | .data.rel.ro .bss.rel.ro) | .data .bss)`

I don't follow.

>> (note that .dynamic is sometimes read only and sometimes writeable)
> 
> Do you mean ld.lld -z rodynamic (D33251 <https://reviews.llvm.org/D33251> http://lists.llvm.org/pipermail/llvm-dev/2017-May/113258.html)? (`.dynamic` is in the PT_GNU_RELRO region)

Yep that's what I mean. The .dynamic section is generally relro but can be read only if that flag is used. We shouldn't add a PT_GNU_RELRO just to be consistent with one particular way of doing things. .dynamic is relro because of DT_DEBUG which is not always used. If it isn't used it is possible to make it read only. What add an extra 50 or so bytes when we don't really need to?

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

https://reviews.llvm.org/D55864