[PATCH] D28272: ELF: Reserve space for copy relocations of read-only symbols in relro.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

In https://reviews.llvm.org/D28272#636529, @ruiu wrote:

> Looks like ld.bfd and ld.gold have something similar already. Are they using the same section name, .bss.rel.ro?


It looks like they are both using the name `.data.rel.ro`.

Relatedly, I discovered a bug in this patch. We can't use SHT_NOBITS for the new section because we need to lay out all NOBITS sections contiguously at the end of the LOAD. RELRO also needs to be contiguous (there can only be one RELRO header), so we can't have another RELRO covering part of the bss.

In principle, we could teach the linker to emit two r/w LOAD headers, one for RELRO and the other for non-RELRO, each with its own bss, but it's not clear that that would be worth it. So we might as well use SHT_PROGBITS and rename the section to `.data.rel.ro` to match the type. I'll upload a new patch that does that.



================
Comment at: lld/ELF/Relocations.cpp:419-426
+  for (const Elf_Phdr &Phdr : check(SS->file()->getObj().program_headers())) {
+    if ((Phdr.p_type == ELF::PT_LOAD || Phdr.p_type == ELF::PT_GNU_RELRO) &&
+        !(Phdr.p_flags & ELF::PF_W) && Value >= Phdr.p_vaddr &&
+        Value < Phdr.p_vaddr + Phdr.p_memsz) {
+      BssSec = Out<ELFT>::BssRelRo;
+      break;
+    }
----------------
ruiu wrote:
> Can you factor the new code as a function taking a SharedSymbol and returning a boolean value?
Will do


https://reviews.llvm.org/D28272