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

[Peter Smith via llvm-commits]

> 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 not sure I follow about the handling of ro copy relocations. As I
understand it if there is a linker script and we have a SECTIONS
command then our .data.rel.ro.bss gets merged into the .data.rel.ro,
this is still contiguous with the other relro sections. I've included
an extract of the map file output for a simple test case just to make
sure my description of the end result was accurate.

#include <stdio.h>
// x and z will require copy relocations
extern const int x;
extern const int z;
// force creation of .data.rel.ro section, not too concerned about contents
const int foo __attribute__((section(".data.rel.ro"))) = 30;
int rw = 20;
int bss[100000];
const int * const y = &z;
int main() {
    printf("%d %d %d %d %d\n", x, *y, rw, bss[0], foo);
    return 0;
}

With the bfd linker script I get a map file:
0000000000410038 000000000000000c     4 .data.rel.ro
0000000000410038 0000000000000004     4
/tmp/main-a7573f.o:(.data.rel.ro)
0000000000410038 0000000000000000     0                 $d.1
0000000000410038 0000000000000004     0                 foo
000000000041003c 0000000000000004     4         <internal>:(.bss.rel.ro)
0000000000410040 0000000000000004     4         <internal>:(.bss.rel.ro)
0000000000410048 00000000000001d0     8 .dynamic
0000000000410048 00000000000001d0     8         <internal>:(.dynamic)
0000000000410218 0000000000000008     8 .got
0000000000410218 0000000000000008     8         <internal>:(.got)
0000000000411000 0000000000000038     8 .got.plt
0000000000411000 0000000000000038     8         <internal>:(.got.plt)

Without a linker script I get:
0000000000040018 000000000000000c     4 .data.rel.ro
0000000000040018 0000000000000004     4
/tmp/main-26b827.o:(.data.rel.ro)
0000000000040018 0000000000000000     0                 $d.1
0000000000040018 0000000000000004     0                 foo
000000000004001c 0000000000000004     4         <internal>:(.bss.rel.ro)
0000000000040020 0000000000000004     4         <internal>:(.bss.rel.ro)
0000000000040028 00000000000001b0     8 .dynamic
0000000000040028 00000000000001b0     8         <internal>:(.dynamic)
00000000000401d8 0000000000000008     8 .got
00000000000401d8 0000000000000008     8         <internal>:(.got)

If I comment out the line with
__attribute__((section(".data.rel.ro"))) so that there is only linker
generated .data.rel.ro.bss

0000000000040018 00000000000001b0     8 .dynamic
0000000000040018 00000000000001b0     8         <internal>:(.dynamic)
00000000000401c8 0000000000000008     8 .got
00000000000401c8 0000000000000008     8         <internal>:(.got)
00000000000401d0 0000000000000008     4 .data.rel.ro
00000000000401d0 0000000000000004     4         <internal>:(.bss.rel.ro)
00000000000401d4 0000000000000004     4         <internal>:(.bss.rel.ro)
0000000000050000 0000000000061a81     4 .bss

In all cases the copy relocations are part of the PT_GNU_RELO PHDR,
although we have used more space in the file when there is
compiler/assembler provided .data.rel.ro in the non script case.

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

I don't have a specific objective with the bfd linker script. I've
sometimes wanted a linker script that gets close to ld.bfd so I can
more easily compare the output. I've also used ld --verbose to get a
script that I've then added a few Sections to in order to test an
experiment. I thought I may not be the only person that occasionally
does this.

Generalising a bit, I think that it should be possible to write a
linker script that works with both ld.bfd and lld, at least for the
common feature set that could get used in everyday builds.

> 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.
>

Ok, I'll take that into account and see what I can come up with. Will
update the review tomorrow, I didn't get a chance today.

Thanks very much for the comments.

> Cheers,
> Rafael