[PATCH] D40029: [ELF] Only add contiguous isRelro sections to PT_GNU_RELRO PHDR

Rafael Avila de Espindola via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 17 10:16:40 PST 2017


Peter Smith <peter.smith at linaro.org> writes:

> I'm not going to get a chance to upload a new patch today, but I've
> run some experiments. It looks like renaming the section to
> .data.rel.ro.bss but making no other changes has the following effect
> - The default ldscript from bfd is happy to match this in
> .data.rel.ro, the whole of the contents of the section is emitted in
> the file. The output at least for my simple examples is working.

That is as good as we can hope for with that linker script I think.

> - The OutputSection name for the non-linker script case is
> .data.rel.ro as this matches the pattern .data.rel.ro.* . If this is
> the only .rel.ro then it will be output as NOBITS (pt_memsz only)  if
> there is any other .data.rel.ro produced by the compiler/assembler
> then our data.rel.ro.bss will be merged with that and be output as
> PROGBITS.

:-(

This suggests that we should not name it .data.rel.ro.bss
unconditionally. We could do it only when there is a SECTIONS directive,
but that would make it impossible to write linker scripts that correctly
handle ro copy relocations. That is probably not too bad as we also
cannot have a script that has 3 (r, rx, rw) PT_LOADs.

> I'm planning on updating the patch on Monday.
>
> There is one more oddity to report, although it isn't related to
> relro. In the linker script after the bss there is the following:
>   .lbss   :
>   {
>     *(.dynlbss)
>     *(.lbss .lbss.* .gnu.linkonce.lb.*)
>     *(LARGE_COMMON)
>   }
>   . = ALIGN(64 / 8);
>   . = SEGMENT_START("ldata-segment", .);
>   .lrodata   ALIGN(CONSTANT (MAXPAGESIZE)) + (. & (CONSTANT
> (MAXPAGESIZE) - 1)) :
>   {
>     *(.lrodata .lrodata.* .gnu.linkonce.lr.*)
>   }
>   .ldata   ALIGN(CONSTANT (MAXPAGESIZE)) + (. & (CONSTANT (MAXPAGESIZE) - 1)) :
>   {
>     *(.ldata .ldata.* .gnu.linkonce.l.*)
>     . = ALIGN(. != 0 ? 64 / 8 : 1);
>   }
>
> We output a 0 sized section for .ldata after the bss, I'm guessing
> because of the . = ALIGN(. != 0 ? 64 / 8 : 1);
> This is sufficient to make all the NOBITS in the image be output as
> PROGBITS as it gets added to the PHDR containing the RW and ZI. I have
> no idea what .ldata is, but it may be worth taking a look at this as
> it could make lld look bad in size comparisons if projects are using
> ld.bfd derived linker scripts.

Maybe adjustSectionsBeforeSorting should propagate the section type.

BTW, what is your objective in using lld with the bfd script?

Given the above results I think that ignoring empty sections in isRelro
and producing an error if they are discontiguous is probably the best we can
do without an explicit command line option or an if on SECTIONS being present.

Cheers,
Rafael


More information about the llvm-commits mailing list