[PATCH] D40365: [ELF] Rename .bss.rel.ro to .data.rel.ro.bss for some Linker Scripts.

[Rafael Avila de Espindola via llvm-commits]

LGTM

Thanks

Peter Smith via Phabricator <reviews at reviews.llvm.org> writes:

> peter.smith created this revision.
> Herald added subscribers: arichardson, emaste.
>
> This is patch 3 of 3 spun out from https://reviews.llvm.org/D40029 covering: Name change when linker scripts are used.
>
> LLD uses .bss.rel.ro for read-only copy relocations whereas the ld.bfd and gold linkers use .data.rel.ro. In some linker scripts including ld.bfd's internal linker script, the relro sections are placed sequentially assuming .data.rel.ro is used. LLD's use of .bss.rel.ro means that the copy relocations get matched into the .bss section causing the relro sections to be non-contiguous.
>
>       
>
> This change checks for a .data.rel.ro OutputSection when a linker script with the SECTIONS command is used. The section will match in the .data.rel.ro output section and will maintain contiguous relro.
>
>
> https://reviews.llvm.org/D40365
>
> Files:
>   ELF/Writer.cpp
>   test/ELF/relro-script.s
>
>
> Index: test/ELF/relro-script.s
> ===================================================================
> --- /dev/null
> +++ test/ELF/relro-script.s
> @@ -0,0 +1,29 @@
> +// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/shared.s -o %t.o
> +// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/copy-in-shared.s -o %t2.o
> +// RUN: ld.lld -shared %t.o %t2.o -o %t.so
> +
> +// ld.bfd and gold use .data.rel.ro rather than .bss.rel.ro. When a linker
> +// script, such as ld.bfd's internal linker script has a .data.rel.ro
> +// OutputSection we rename .bss.rel.ro to .data.rel.ro.bss in order to match in
> +// .data.rel.ro. This keeps the relro sections contiguous.
> +
> +// Use the same sections and ordering as the ld.bfd internal linker script.
> +// RUN: echo "SECTIONS { \
> +// RUN: .data.rel.ro : { *(.data.rel.ro .data.rel.ro.*) } \
> +// RUN: .dynamic : { *(.dynamic) } \
> +// RUN: .got : { *(.got) } \
> +// RUN: .got.plt : { *(.got.plt) } \
> +// RUN: } " > %t.script
> +// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t3.o
> +// RUN: ld.lld %t3.o %t.so -o %t --script=%t.script --print-map | FileCheck %s
> +// REQUIRES: x86
> +
> +// CHECK: .data.rel.ro
> +// CHECK-NEXT: <internal>:(.bss.rel.ro)
> +        .section .text, "ax", @progbits
> +        .global _start
> +        .global bar
> +        .global foo
> +_start:
> +        .quad bar
> +        .quad foo
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -289,7 +289,13 @@
>  
>    InX::Bss = make<BssSection>(".bss", 0, 1);
>    Add(InX::Bss);
> -  InX::BssRelRo = make<BssSection>(".bss.rel.ro", 0, 1);
> +  // If there is a SECTIONS command and a .data.rel.ro section name use name
> +  // .data.rel.ro.bss so that we match in the .data.rel.ro output section.
> +  // This makes sure our relro is contiguous.
> +  bool IsSectionsDataRelRoOS =
> +      Script->HasSectionsCommand && findSection(".data.rel.ro");
> +  InX::BssRelRo = make<BssSection>(
> +      IsSectionsDataRelRoOS ? ".data.rel.ro.bss" : ".bss.rel.ro", 0, 1);
>    Add(InX::BssRelRo);
>  
>    // Add MIPS-specific sections.
>
>
> Index: test/ELF/relro-script.s
> ===================================================================
> --- /dev/null
> +++ test/ELF/relro-script.s
> @@ -0,0 +1,29 @@
> +// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/shared.s -o %t.o
> +// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/copy-in-shared.s -o %t2.o
> +// RUN: ld.lld -shared %t.o %t2.o -o %t.so
> +
> +// ld.bfd and gold use .data.rel.ro rather than .bss.rel.ro. When a linker
> +// script, such as ld.bfd's internal linker script has a .data.rel.ro
> +// OutputSection we rename .bss.rel.ro to .data.rel.ro.bss in order to match in
> +// .data.rel.ro. This keeps the relro sections contiguous.
> +
> +// Use the same sections and ordering as the ld.bfd internal linker script.
> +// RUN: echo "SECTIONS { \
> +// RUN: .data.rel.ro : { *(.data.rel.ro .data.rel.ro.*) } \
> +// RUN: .dynamic : { *(.dynamic) } \
> +// RUN: .got : { *(.got) } \
> +// RUN: .got.plt : { *(.got.plt) } \
> +// RUN: } " > %t.script
> +// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t3.o
> +// RUN: ld.lld %t3.o %t.so -o %t --script=%t.script --print-map | FileCheck %s
> +// REQUIRES: x86
> +
> +// CHECK: .data.rel.ro
> +// CHECK-NEXT: <internal>:(.bss.rel.ro)
> +        .section .text, "ax", @progbits
> +        .global _start
> +        .global bar
> +        .global foo
> +_start:
> +        .quad bar
> +        .quad foo
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -289,7 +289,13 @@
>  
>    InX::Bss = make<BssSection>(".bss", 0, 1);
>    Add(InX::Bss);
> -  InX::BssRelRo = make<BssSection>(".bss.rel.ro", 0, 1);
> +  // If there is a SECTIONS command and a .data.rel.ro section name use name
> +  // .data.rel.ro.bss so that we match in the .data.rel.ro output section.
> +  // This makes sure our relro is contiguous.
> +  bool IsSectionsDataRelRoOS =
> +      Script->HasSectionsCommand && findSection(".data.rel.ro");
> +  InX::BssRelRo = make<BssSection>(
> +      IsSectionsDataRelRoOS ? ".data.rel.ro.bss" : ".bss.rel.ro", 0, 1);
>    Add(InX::BssRelRo);
>  
>    // Add MIPS-specific sections.