[PATCH] D26889: [ELF] - Implemented -no-rosegment.

[Rafael Avila de Espindola via llvm-commits]

LGTM, but in a followup patch please change the if in
compareSectionsNonScript to use SingleRoRx instead of HasSections (and
add a test).


George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

> grimar updated this revision to Diff 79377.
> grimar added a comment.
>
> - Addressed review comments.
>
>
> https://reviews.llvm.org/D26889
>
> Files:
>   ELF/Config.h
>   ELF/Driver.cpp
>   ELF/Options.td
>   ELF/Writer.cpp
>   test/ELF/segments.s
>
> Index: test/ELF/segments.s
> ===================================================================
> --- test/ELF/segments.s
> +++ test/ELF/segments.s
> @@ -0,0 +1,88 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
> +# RUN: ld.lld %t -o %t1
> +# RUN: llvm-readobj --program-headers %t1 | FileCheck --check-prefix=ROSEGMENT %s
> +
> +# ROSEGMENT:      ProgramHeader {
> +# ROSEGMENT:        Type: PT_LOAD
> +# ROSEGMENT-NEXT:    Offset: 0x0
> +# ROSEGMENT-NEXT:    VirtualAddress:
> +# ROSEGMENT-NEXT:    PhysicalAddress:
> +# ROSEGMENT-NEXT:    FileSize:
> +# ROSEGMENT-NEXT:    MemSize:
> +# ROSEGMENT-NEXT:    Flags [
> +# ROSEGMENT-NEXT:      PF_R
> +# ROSEGMENT-NEXT:    ]
> +# ROSEGMENT-NEXT:    Alignment: 4096
> +# ROSEGMENT-NEXT:  }
> +# ROSEGMENT-NEXT:  ProgramHeader {
> +# ROSEGMENT-NEXT:    Type: PT_LOAD
> +# ROSEGMENT-NEXT:    Offset: 0x1000
> +# ROSEGMENT-NEXT:    VirtualAddress:
> +# ROSEGMENT-NEXT:    PhysicalAddress:
> +# ROSEGMENT-NEXT:    FileSize:
> +# ROSEGMENT-NEXT:    MemSize:
> +# ROSEGMENT-NEXT:    Flags [
> +# ROSEGMENT-NEXT:      PF_R
> +# ROSEGMENT-NEXT:      PF_X
> +# ROSEGMENT-NEXT:    ]
> +# ROSEGMENT-NEXT:    Alignment: 4096
> +# ROSEGMENT-NEXT:  }
> +# ROSEGMENT-NEXT:  ProgramHeader {
> +# ROSEGMENT-NEXT:    Type: PT_LOAD
> +# ROSEGMENT-NEXT:    Offset: 0x2000
> +# ROSEGMENT-NEXT:    VirtualAddress:
> +# ROSEGMENT-NEXT:    PhysicalAddress:
> +# ROSEGMENT-NEXT:    FileSize: 1
> +# ROSEGMENT-NEXT:    MemSize: 1
> +# ROSEGMENT-NEXT:    Flags [
> +# ROSEGMENT-NEXT:      PF_R
> +# ROSEGMENT-NEXT:      PF_W
> +# ROSEGMENT-NEXT:    ]
> +# ROSEGMENT-NEXT:    Alignment: 4096
> +# ROSEGMENT-NEXT:  }
> +
> +# RUN: ld.lld -no-rosegment %t -o %t2
> +# RUN: llvm-readobj --program-headers %t2 | FileCheck --check-prefix=NOROSEGMENT %s
> +
> +# NOROSEGMENT:     ProgramHeader {
> +# NOROSEGMENT:       Type: PT_LOAD
> +# NOROSEGMENT-NEXT:   Offset: 0x0
> +# NOROSEGMENT-NEXT:   VirtualAddress:
> +# NOROSEGMENT-NEXT:   PhysicalAddress:
> +# NOROSEGMENT-NEXT:   FileSize:
> +# NOROSEGMENT-NEXT:   MemSize:
> +# NOROSEGMENT-NEXT:   Flags [
> +# NOROSEGMENT-NEXT:     PF_R
> +# NOROSEGMENT-NEXT:     PF_X
> +# NOROSEGMENT-NEXT:   ]
> +# NOROSEGMENT-NEXT:   Alignment: 4096
> +# NOROSEGMENT-NEXT: }
> +# NOROSEGMENT-NEXT: ProgramHeader {
> +# NOROSEGMENT-NEXT:   Type: PT_LOAD
> +# NOROSEGMENT-NEXT:   Offset: 0x1000
> +# NOROSEGMENT-NEXT:   VirtualAddress:
> +# NOROSEGMENT-NEXT:   PhysicalAddress:
> +# NOROSEGMENT-NEXT:   FileSize:
> +# NOROSEGMENT-NEXT:   MemSize:
> +# NOROSEGMENT-NEXT:   Flags [
> +# NOROSEGMENT-NEXT:     PF_R
> +# NOROSEGMENT-NEXT:     PF_W
> +# NOROSEGMENT-NEXT:   ]
> +# NOROSEGMENT-NEXT:   Alignment: 4096
> +# NOROSEGMENT-NEXT: }
> +# NOROSEGMENT-NEXT: ProgramHeader {
> +# NOROSEGMENT-NEXT:   Type: PT_GNU_STACK
> +
> +.global _start
> +_start:
> + nop
> +
> +.section .ro,"a"
> +nop
> +
> +.section .rw,"aw"
> +nop
> +
> +.section .rx,"ax"
> +nop
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -1096,7 +1096,7 @@
>  // cannot create a PT_LOAD there.
>  template <class ELFT>
>  static typename ELFT::uint computeFlags(typename ELFT::uint F) {
> -  if (ScriptConfig->HasSections && !(F & PF_W))
> +  if (Config->SingleRoRx && !(F & PF_W))
>      return F | PF_X;
>    return F;
>  }
> Index: ELF/Options.td
> ===================================================================
> --- ELF/Options.td
> +++ ELF/Options.td
> @@ -147,6 +147,8 @@
>  
>  def nopie: F<"nopie">, HelpText<"Do not create a position independent executable">;
>  
> +def no_rosegment: F<"no-rosegment">, HelpText<"Do not put read-only non-executable sections in their own segment">;
> +
>  def no_undefined: F<"no-undefined">,
>    HelpText<"Report unresolved symbols even if the linker is creating a shared library">;
>  
> Index: ELF/Driver.cpp
> ===================================================================
> --- ELF/Driver.cpp
> +++ ELF/Driver.cpp
> @@ -696,6 +696,12 @@
>      }
>    }
>  
> +  // -no-rosegment is used to avoid placing read only non-executable sections in
> +  // their own segment. We do the same if SECTIONS command is present in linker
> +  // script. See comment for computeFlags().
> +  Config->SingleRoRx =
> +      Args.hasArg(OPT_no_rosegment) || ScriptConfig->HasSections;
> +
>    if (Files.empty() && ErrorCount == 0)
>      error("no input files");
>  }
> Index: ELF/Config.h
> ===================================================================
> --- ELF/Config.h
> +++ ELF/Config.h
> @@ -120,6 +120,7 @@
>    bool Rela;
>    bool Relocatable;
>    bool SaveTemps;
> +  bool SingleRoRx;
>    bool Shared;
>    bool Static = false;
>    bool SysvHash = true;