[ELF] - -pie/--pic-executable option implemented

[grimar]

Summary

-pie
--pic-executable

Create a position independent executable.  This is currently only
 supported on ELF platforms.  Position independent executables are
 similar to shared libraries in that they are relocated by the
 dynamic linker to the virtual address the OS chooses for them
 (which can vary between invocations).  Like normal dynamically
 linked executables they can be executed and symbols defined in the
 executable cannot be overridden by shared libraries.

This fixes the https://llvm.org/bugs/show_bug.cgi?id=26923

[ruiu]

ELF/Driver.cpp

150–151

Why is -pie incompatible with -static?

ELF/SymbolTable.cpp

21

isPIO is a single line function. I don't think including the entire header file just for that function is a good idea.

127

What does PIO stand for?

Anyways, I think this function is probably a bit too much. How about this: add Config->Pic and set it to true if either Config->Pie or Config->Shared.

[grimar]

• Addressed review comments.

[grimar]

ELF/Driver.cpp

150–151

It was done in bfd/gold to be consistent with gcc driver:
https://gcc.gnu.org/ml/gcc/2014-01/msg00125.html
My motivation was to be consistent with gold here.

ELF/SymbolTable.cpp

21

I can't agree here, because using of declarations instead of including header in cpp files does not look clean for me.
Fortunately we don't need that after introducing Config->Pic anymore.

127

It was Position Independent Output. I saw that term in gold sources.
Applied your suggestion about Config->Pic, that should work.

[grimar]

• Removed excessive include file

[ruiu]

ELF/Config.h

71

Remove " = false".

ELF/Driver.cpp

150–151

I don't see any reason to reject that, so I believe they are simply wrong. And I don't see a reason to copy a wrong behavior.

ELF/SymbolTable.cpp

120–121

Can you revert this change?

ELF/Target.cpp

339

I like these renamings from Shared to Pic as it conveys more accurate meaning.

[grimar]

• Addressed review comments

[ruiu]

ELF/Driver.cpp

148–149

I think this is also something that we are checking for errors too eagerly. -shared implies -pie, so they are not incompatible. It is better to ignore this combination.

150–151

Please remove this code unless you have a valid reason to do so.

[grimar]

ELF/Config.h

71

Done.

ELF/Driver.cpp

150–151

This patch does not support this.
I did not investigate where lld needs change to support that. Let me leave it as is now ? If someone needs that - it would be his responsibility to tweak lld here.

ELF/SymbolTable.cpp

120–121

Done.

ELF/Target.cpp

339

true.

[grimar]

ELF/Driver.cpp

148–149

-shared implies pic, but not -pie. As pie is Position Independent Executable. So I can't agree here. Gold do the same check.

[ruiu]

ELF/Driver.cpp

150–151

Adding code which you cannot justify is a bad idea. "Leaving it as is" means not adding that code because that code didn't exist before. Add this piece of code when you find you actually need it.

[grimar]

• Addressed review comments

ELF/Driver.cpp

150–151

I removed it just because you insisted. But I can justify that. You`ll never get this using gcc and that is enough IMO to not support that atm.
You often say something like "lets not support that until we need". So why we enable support here for something that even gold does not do ?

[grimar]

?>I respect compatibility as long as it matters and makes sense. In this case, we don't have a good explanation on why this combination of option is banned by gold/bfd. Adding that >error check would increase the complexity of the linker, and it copies the artificial limitation that we can't explain. I don't want to say when someone asked me why this is banned >"because gold did this, so I don't know."

Ok, I respect and understand your point here.

George.

[grimar]

ELF/Options.td

102

btw, not about this patch, but in general.
If we have errors/warnings in lowercase. Do we want HelpText here to be lowercase as well ? I guess yes.

[ruiu]

ELF/Options.td

102

I don't know which is better, but this is where we just want to follow the convention. What do other command do for --help?

[ruiu]

LGTM

test/ELF/undef.s

3–4

Move CHECK after the following RUN. (Basically, we want to keep the source code not being "patchy". New code should look as if it were there from beginning. I don't think you wrote this way if you have written these two RUN lines)

[grimar]

ELF/Options.td

102

Hmm. clang, ld and gold has uppercase for help text. So seems no need to change anything here.

[grimar]

test/ELF/undef.s

3–4

Done.
But I think we sometimes re-use the previously declared CHECKs when adding new RUN lines in tests. Actually If I would wrote that from beginning it would be:

# REQUIRES: x86
# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t

# RUN: not ld.lld %t -o %t2 2>&1 | FileCheck %s
# CHECK: undefined symbol: bar in {{.*}}
# CHECK: undefined symbol: foo in {{.*}}

# RUN: not ld.lld -pie %t -o %t2 2>&1 | FileCheck %s

[grimar]

ruiu added inline comments.

Comment at: ELF/Driver.cpp:150-151
@@ +149,4 @@
+ error("-shared and -pie may not be used together");
+ if (Config->Static)
+ error("-static and -pie may not be used together");

+ }

Why is -pie incompatible with -static?

It is not fundamentally so. A static program can be self relocating
and openbsd has static pie binaries.

Hmm after that I answered a question to myself - what linker is used to produce that binaries.
I knew that gold does not support mix of -pie and -static. That was the reason I added this lines initially.
I just for some reason was sure that bfd also restricts it.

I checked it for bfd and found that options are working fine together. So now I think that was correct to remove
that check from final patch.

Cheers,
Rafael

George.

ELF/Config.h

   bool Mips64EL = false;
   bool NoUndefined;
   bool NoinhibitExec;
+  bool Pic = false;
+  bool Pie;
   bool PrintGcSections;
   bool Rela;
   bool Relocatable;

ELF/Driver.cpp

   if (Config->EMachine == EM_AMDGPU && !Config->Entry.empty())
     error("-e option is not valid for AMDGPU.");
 
+  if (Config->Pie) {
+    if (Config->Shared)
+      error("-shared and -pie may not be used together");
+    if (Config->Static)
+      error("-static and -pie may not be used together");
+  }
+
   if (!Config->Relocatable)
     return;
 

     error("-r and --gc-sections may not be used together");
   if (Config->ICF)
     error("-r and --icf may not be used together");
+  if (Config->Pie)
+    error("-r and -pie may not be used together");
 }
 
 static StringRef

   Config->ICF = Args.hasArg(OPT_icf);
   Config->NoUndefined = Args.hasArg(OPT_no_undefined);
   Config->NoinhibitExec = Args.hasArg(OPT_noinhibit_exec);
+  Config->Pie = Args.hasArg(OPT_pie);
   Config->PrintGcSections = Args.hasArg(OPT_print_gc_sections);
   Config->Relocatable = Args.hasArg(OPT_relocatable);
   Config->SaveTemps = Args.hasArg(OPT_save_temps);

   Config->ZOrigin = hasZOption(Args, "origin");
   Config->ZRelro = !hasZOption(Args, "norelro");
 
+  Config->Pic = Config->Pie || Config->Shared;
+
   if (Config->Relocatable)
     Config->StripAll = false;
 

ELF/Options.td

 def o : JoinedOrSeparate<["-"], "o">, MetaVarName<"<path>">,
   HelpText<"Path to file to write output">;
 
+def pie : Flag<["-"], "pie">,
+  HelpText<"Create a position independent executable">;
+
 def print_gc_sections: Flag<["--"], "print-gc-sections">,
   HelpText<"List removed unused sections">;
 

 def alias_init_init : Joined<["-"], "init=">, Alias<init>;
 def alias_l__library : Joined<["--"], "library=">, Alias<l>;
 def alias_o_output : Joined<["--"], "output=">, Alias<o>;
+def alias_pie_pic_executable: Flag<["--"], "pic-executable">, Alias<pie>;
 def alias_rpath_rpath : Joined<["-"], "rpath=">, Alias<rpath>;
 def alias_relocatable_r : Flag<["-"], "r">, Alias<relocatable>;
 def alias_shared_Bshareable : Flag<["-"], "Bshareable">, Alias<shared>;

ELF/SymbolTable.cpp

   // might be a cross linker.
 
   std::string ErrMsg;
-  const Target *TheTarget = TargetRegistry::lookupTarget(TripleStr, ErrMsg);
+  const llvm::Target *TheTarget =
+      TargetRegistry::lookupTarget(TripleStr, ErrMsg);
   if (!TheTarget)
     fatal("target not found: " + ErrMsg);
 
   TargetOptions Options;
-  Reloc::Model R = Config->Shared ? Reloc::PIC_ : Reloc::Static;
+  Reloc::Model R = Config->Pic ? Reloc::PIC_ : Reloc::Static;
   std::unique_ptr<TargetMachine> TM(
       TheTarget->createTargetMachine(TripleStr, "", "", Options, R));
 

ELF/Target.cpp

   return false;
 }
 
-uint64_t TargetInfo::getVAStart() const { return Config->Shared ? 0 : VAStart; }
+uint64_t TargetInfo::getVAStart() const { return Config->Pic ? 0 : VAStart; }
 
 bool TargetInfo::needsCopyRelImpl(uint32_t Type) const { return false; }
 

   // plt. That is identified by special relocation types (R_X86_64_JUMP_SLOT,
   // R_386_JMP_SLOT, etc).
   if (auto *SS = dyn_cast<SharedSymbol<ELFT>>(&S))
-    if (!Config->Shared && SS->Sym.getType() == STT_FUNC &&
+    if (!Config->Pic && SS->Sym.getType() == STT_FUNC &&
         !refersToGotEntry(Type))
       return Plt_Implicit;
 

 void X86TargetInfo::writePltZero(uint8_t *Buf) const {
   // Executable files and shared object files have
   // separate procedure linkage tables.
-  if (Config->Shared) {
+  if (Config->Pic) {
     const uint8_t V[] = {
         0xff, 0xb3, 0x04, 0x00, 0x00, 0x00, // pushl 4(%ebx)
         0xff, 0xa3, 0x08, 0x00, 0x00, 0x00, // jmp   *8(%ebx)

   memcpy(Buf, Inst, sizeof(Inst));
 
   // jmp *foo@GOT(%ebx) or jmp *foo_in_GOT
-  Buf[1] = Config->Shared ? 0xa3 : 0x25;
+  Buf[1] = Config->Pic ? 0xa3 : 0x25;
   uint32_t Got = UseLazyBinding ? Out<ELF32LE>::GotPlt->getVA()
                                 : Out<ELF32LE>::Got->getVA();
   write32le(Buf + 2, Config->Shared ? GotEntryAddr - Got : GotEntryAddr);

ELF/Writer.cpp

     return !Symtab.getSharedFiles().empty() && !Config->DynamicLinker.empty();
   }
   bool isOutputDynamic() const {
-    return !Symtab.getSharedFiles().empty() || Config->Shared;
+    return !Symtab.getSharedFiles().empty() || Config->Pic;
   }
 
   void ensureBss();

         continue;
       }
 
-      bool Dynrel = Config->Shared && !Target->isRelRelative(Type) &&
+      bool Dynrel = Config->Pic && !Target->isRelRelative(Type) &&
                     !Target->isSizeRel(Type);
       if (Preemptible || Dynrel) {
         uint32_t DynType;

     // We can however do better than just copying the incoming relocation. We
     // can process some of it and and just ask the dynamic linker to add the
     // load address.
-    if (!Config->Shared || Target->isRelRelative(Type) ||
-        Target->isSizeRel(Type))
+    if (!Config->Pic || Target->isRelRelative(Type) || Target->isSizeRel(Type))
       continue;
 
     uintX_t Addend = getAddend<ELFT>(RI);

 }
 
 static uint16_t getELFType() {
-  if (Config->Shared)
+  if (Config->Pic)
     return ET_DYN;
   if (Config->Relocatable)
     return ET_REL;

test/ELF/driver.test

 # RUN: not ld.lld -r --icf=all %t -o %tfail 2>&1 | FileCheck -check-prefix=ERR4 %s
 # ERR4: -r and --icf may not be used together
 
+## Attempt to use -r and -pie together
+# RUN: not ld.lld -r -pie %t -o %tfail 2>&1 | FileCheck -check-prefix=ERR5 %s
+# ERR5: -r and -pie may not be used together
+
+## Attempt to use -shared and -pie together
+# RUN: not ld.lld -shared -pie %t -o %tfail 2>&1 | FileCheck -check-prefix=ERR6 %s
+# ERR6: -shared and -pie may not be used together
+
+## Attempt to use -static and -pie together
+# RUN: not ld.lld -static -pie %t -o %tfail 2>&1 | FileCheck -check-prefix=ERR7 %s
+# ERR7: -static and -pie may not be used together
+
 .globl _start
 _start:
   nop

test/ELF/dynsym-pie.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t
+# RUN: ld.lld -pie %t -o %t.out
+# RUN: llvm-readobj -t -dyn-symbols %t.out | FileCheck %s
+
+# CHECK:      DynamicSymbols [
+# CHECK-NEXT:  Symbol {
+# CHECK-NEXT:    Name: @
+# CHECK-NEXT:    Value: 0x0
+# CHECK-NEXT:    Size: 0
+# CHECK-NEXT:    Binding: Local
+# CHECK-NEXT:    Type: None
+# CHECK-NEXT:    Other: 0
+# CHECK-NEXT:    Section: Undefined
+# CHECK-NEXT:  }
+# CHECK-NEXT: ]
+
+.text
+.globl _start
+_start:
+
+.global default
+default:
+
+.global protected
+protected:
+
+.global hidden
+hidden:
+
+.global internal
+internal:
+
+.global protected_with_hidden
+.protected
+protected_with_hidden:

test/ELF/local-got-pie.s

+// RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+// RUN: ld.lld %t.o -o %t -pie
+// RUN: llvm-readobj -s -r %t | FileCheck %s
+// RUN: llvm-objdump -d %t | FileCheck --check-prefix=DISASM %s
+
+.globl _start
+_start:
+ call foo@gotpcrel
+
+ .hidden foo
+ .global foo
+foo:
+ nop
+
+// 0x20A0 - 1001 - 5 = 4250
+// DISASM:      Disassembly of section .text:
+// DISASM-NEXT: _start:
+// DISASM-NEXT:   1000: {{.*}} callq 4251
+// DISASM:      foo:
+// DISASM-NEXT:   1005: {{.*}} nop
+
+// CHECK:      Name: .got
+// CHECK-NEXT: Type: SHT_PROGBITS
+// CHECK-NEXT: Flags [
+// CHECK-NEXT:   SHF_ALLOC
+// CHECK-NEXT:   SHF_WRITE
+// CHECK-NEXT: ]
+// CHECK-NEXT: Address: 0x20A0
+// CHECK-NEXT: Offset:
+// CHECK-NEXT: Size: 8
+
+// CHECK:      Relocations [
+// CHECK-NEXT:   Section ({{.*}}) .rela.dyn {
+// CHECK-NEXT:     0x20A0 R_X86_64_RELATIVE - 0x1005
+// CHECK-NEXT:   }
+// CHECK-NEXT: ]

test/ELF/noplt-pie.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t1.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/shared.s -o %t2.o
+# RUN: ld.lld -shared %t2.o -o %t2.so
+# RUN: ld.lld %t1.o %t2.so -o %t.out
+# RUN: llvm-readobj -s -r %t.out | FileCheck %s
+
+# CHECK: Section {
+# CHECK-NOT: Name: .plt
+
+# CHECK:      Relocations [
+# CHECK-NEXT:   Section ({{.*}}) .rela.dyn {
+# CHECK-NEXT:     0x120B0 R_X86_64_GLOB_DAT bar 0x0
+# CHECK-NEXT:     0x120B8 R_X86_64_GLOB_DAT zed 0x0
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]
+
+.global _start
+_start:
+ movq bar@GOTPCREL(%rip), %rcx
+ movq zed@GOTPCREL(%rip), %rcx

test/ELF/pie.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t1.o
+# RUN: ld.lld -pie %t1.o -o %t
+# RUN: llvm-readobj -file-headers -sections -program-headers -symbols -r %t | FileCheck %s
+
+## Test --pic-executable alias
+# RUN: ld.lld --pic-executable %t1.o -o %t
+# RUN: llvm-readobj -file-headers -sections -program-headers -symbols -r %t | FileCheck %s
+
+# CHECK:      ElfHeader {
+# CHECK-NEXT:  Ident {
+# CHECK-NEXT:    Magic: (7F 45 4C 46)
+# CHECK-NEXT:    Class: 64-bit
+# CHECK-NEXT:    DataEncoding: LittleEndian
+# CHECK-NEXT:    FileVersion: 1
+# CHECK-NEXT:    OS/ABI: SystemV
+# CHECK-NEXT:    ABIVersion: 0
+# CHECK-NEXT:    Unused: (00 00 00 00 00 00 00)
+# CHECK-NEXT:  }
+# CHECK-NEXT:  Type: SharedObject
+# CHECK-NEXT:  Machine: EM_X86_64
+# CHECK-NEXT:  Version: 1
+# CHECK-NEXT:  Entry: 0x1000
+# CHECK-NEXT:  ProgramHeaderOffset: 0x40
+# CHECK-NEXT:  SectionHeaderOffset: 0x1110
+# CHECK-NEXT:  Flags [
+# CHECK-NEXT:  ]
+# CHECK-NEXT:  HeaderSize: 64
+# CHECK-NEXT:  ProgramHeaderEntrySize: 56
+# CHECK-NEXT:  ProgramHeaderCount: 7
+# CHECK-NEXT:  SectionHeaderEntrySize: 64
+# CHECK-NEXT:  SectionHeaderCount: 9
+# CHECK-NEXT:  StringTableSectionIndex: 7
+# CHECK-NEXT: }
+
+# CHECK:      ProgramHeaders [
+# CHECK-NEXT:  ProgramHeader {
+# CHECK-NEXT:    Type: PT_PHDR
+# CHECK-NEXT:    Offset: 0x40
+# CHECK-NEXT:    VirtualAddress: 0x40
+# CHECK-NEXT:    PhysicalAddress: 0x40
+# CHECK-NEXT:    FileSize: 392
+# CHECK-NEXT:    MemSize: 392
+# CHECK-NEXT:    Flags [
+# CHECK-NEXT:      PF_R
+# CHECK-NEXT:    ]
+# CHECK-NEXT:    Alignment: 8
+# CHECK-NEXT:  }
+# CHECK-NEXT:  ProgramHeader {
+# CHECK-NEXT:    Type: PT_LOAD
+# CHECK-NEXT:    Offset: 0x0
+# CHECK-NEXT:    VirtualAddress: 0x0
+# CHECK-NEXT:    PhysicalAddress: 0x0
+# CHECK-NEXT:    FileSize: 497
+# CHECK-NEXT:    MemSize: 497
+# CHECK-NEXT:    Flags [
+# CHECK-NEXT:      PF_R
+# CHECK-NEXT:    ]
+# CHECK-NEXT:    Alignment: 4096
+# CHECK-NEXT:  }
+# CHECK-NEXT:  ProgramHeader {
+# CHECK-NEXT:    Type: PT_LOAD
+# CHECK-NEXT:    Offset: 0x1000
+# CHECK-NEXT:    VirtualAddress: 0x1000
+# CHECK-NEXT:    PhysicalAddress: 0x1000
+# CHECK-NEXT:    FileSize: 0
+# CHECK-NEXT:    MemSize: 0
+# CHECK-NEXT:    Flags [
+# CHECK-NEXT:      PF_R
+# CHECK-NEXT:      PF_X
+# CHECK-NEXT:    ]
+# CHECK-NEXT:    Alignment: 4096
+# CHECK-NEXT:  }
+# CHECK-NEXT:  ProgramHeader {
+# CHECK-NEXT:    Type: PT_LOAD
+# CHECK-NEXT:    Offset: 0x1000
+# CHECK-NEXT:    VirtualAddress: 0x1000
+# CHECK-NEXT:    PhysicalAddress: 0x1000
+# CHECK-NEXT:    FileSize: 112
+# CHECK-NEXT:    MemSize: 112
+# CHECK-NEXT:    Flags [
+# CHECK-NEXT:      PF_R
+# CHECK-NEXT:      PF_W
+# CHECK-NEXT:    ]
+# CHECK-NEXT:    Alignment: 4096
+# CHECK-NEXT:  }
+# CHECK-NEXT:  ProgramHeader {
+# CHECK-NEXT:    Type: PT_DYNAMIC
+# CHECK-NEXT:    Offset: 0x1000
+# CHECK-NEXT:    VirtualAddress: 0x1000
+# CHECK-NEXT:    PhysicalAddress: 0x1000
+# CHECK-NEXT:    FileSize: 112
+# CHECK-NEXT:    MemSize: 112
+# CHECK-NEXT:    Flags [
+# CHECK-NEXT:      PF_R
+# CHECK-NEXT:      PF_W
+# CHECK-NEXT:    ]
+# CHECK-NEXT:    Alignment: 8
+# CHECK-NEXT:  }
+
+.globl _start
+_start:

test/ELF/plt-i686.s

 // RUN: ld.lld -shared %t.o %t2.so -o %t
 // RUN: llvm-readobj -s -r %t | FileCheck --check-prefix=CHECKSHARED %s
 // RUN: llvm-objdump -d %t | FileCheck --check-prefix=DISASMSHARED %s
-
+// RUN: ld.lld -pie %t.o %t2.so -o %t
+// RUN: llvm-objdump -d %t | FileCheck --check-prefix=DISASMPIE %s
 // REQUIRES: x86
 
 // CHECK:      Name: .plt

 // DISASMSHARED-NEXT:  1046: 68 08 00 00 00     pushl $8
 // DISASMSHARED-NEXT:  104b: e9 d0 ff ff ff     jmp -48 <.plt>
 
+// DISASMPIE:      Disassembly of section .plt:
+// DISASMPIE-NEXT: .plt:
+// DISASMPIE-NEXT:   1020:  ff b3 04 00 00 00 pushl 4(%ebx)
+// DISASMPIE-NEXT:   1026:  ff a3 08 00 00 00 jmpl *8(%ebx)
+// DISASMPIE-NEXT:   102c:  90 nop
+// DISASMPIE-NEXT:   102d:  90 nop
+// DISASMPIE-NEXT:   102e:  90 nop
+// DISASMPIE-NEXT:   102f:  90 nop
+// DISASMPIE-NEXT:   1030:  ff a3 0c 30 00 00 jmpl *12300(%ebx)
+// DISASMPIE-NEXT:   1036:  68 00 00 00 00 pushl $0
+// DISASMPIE-NEXT:   103b:  e9 e0 ff ff ff jmp -32 <.plt>
+// DISASMPIE-NEXT:   1040:  ff a3 10 30 00 00 jmpl *12304(%ebx)
+// DISASMPIE-NEXT:   1046:  68 08 00 00 00 pushl $8
+// DISASMPIE-NEXT:   104b:  e9 d0 ff ff ff jmp -48 <.plt>
+
 local:
 .long 0
 

test/ELF/relative-dynamic-reloc-pie.s

+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: ld.lld -pie %t.o -o %t.pie
+# RUN: llvm-readobj -r -dyn-symbols %t.pie | FileCheck %s
+
+## Test that we create R_X86_64_RELATIVE relocations with -pie.
+# CHECK:      Relocations [
+# CHECK-NEXT:   Section ({{.*}}) .rela.dyn {
+# CHECK-NEXT:     0x1001 R_X86_64_RELATIVE - 0x1001
+# CHECK-NEXT:     0x1009 R_X86_64_RELATIVE - 0x1009
+# CHECK-NEXT:     0x1011 R_X86_64_RELATIVE - 0x100A
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]
+
+.globl _start
+_start:
+nop
+
+foo:
+ .quad foo
+
+.hidden bar
+.global bar
+bar:
+ .quad bar
+ .quad bar + 1

test/ELF/undef.s

 # RUN: not ld.lld %t -o %t2 2>&1 | FileCheck %s
 # CHECK: undefined symbol: bar in {{.*}}
 # CHECK: undefined symbol: foo in {{.*}}
+# RUN: not ld.lld -pie %t -o %t2 2>&1 | FileCheck %s
 # REQUIRES: x86
 
   .globl _start