[LTO] Don't override relocation model stored in the module

[phosek]

Summary

Currently, lld LTO configuration explicitly sets relocation model
based on the linker flags, effectively overriding whatever model is
stored inside the module. This behavior differs from the behavior
one would get during regular linking or even ThinLTO with explicit
codegen where the linker flags such as -r, -pie or -shared don't
affect codegen. This breaks the case where user wants to deliberately
select different relocation model for codegen and for linking, which
is can be the case in special environments like kernels.

Concretely, we stumbled upon this issue while trying to build Zircon
kernel with ThinLTO but the resulting kernel wouldn't boot. Upon
investigation, we've noticed that the kernel code is non-relocatable
despite it being compiled with -fpie. The problem was that our build
passes --no-pie to the linker. This is on purpose, while we want pie
codegen, we want to link the final binary to be linked as static one
(because there's no dynamic linker in our environment).

This change alters the behavior to use the relocation model stored
in the module which is the default behavior of the LTO backend, and
allows overriding the model via -mllvm flag. This matches the behavior
already used for code model. With this change, our kernel builds and
boots even when built with (Thin)LTO.

[pcc]

I'd like to understand the motivation behind this change a little better because the relaxation of the code model from PIC to static is technically an "optimization" that the linker should have license to perform and in fact is already performing in some ways (e.g. emitting addresses into the binary instead of RELATIVE/RELR relocations for statically known addresses).

[phosek]

In D64711#1584916, @pcc wrote:

I'd like to understand the motivation behind this change a little better because the relaxation of the code model from PIC to static is technically an "optimization" that the linker should have license to perform and in fact is already performing in some ways (e.g. emitting addresses into the binary instead of RELATIVE/RELR relocations for statically known addresses).

In the case of our kernel, we need the generated code to be position independent for our KASLR scheme, but we're not doing traditional PIE link. When compiling individual translation units, we use -fpie, but when linking we use --no-pie. Changing the code model from PIC to static in this case results in an invalid kernel image because the generated code is not relocatable. I think it's a matter of principle that it shouldn't be the linker's responsibility to decide how to combine our code-generation choices with our linking choices. I'm open to suggestions for other ways to solve this issue though as we'd really like to be able to use ThinLTO for our kernel.

[pcc]

In D64711#1586696, @phosek wrote:

In D64711#1584916, @pcc wrote:

I'd like to understand the motivation behind this change a little better because the relaxation of the code model from PIC to static is technically an "optimization" that the linker should have license to perform and in fact is already performing in some ways (e.g. emitting addresses into the binary instead of RELATIVE/RELR relocations for statically known addresses).

In the case of our kernel, we need the generated code to be position independent for our KASLR scheme, but we're not doing traditional PIE link. When compiling individual translation units, we use -fpie, but when linking we use --no-pie. Changing the code model from PIC to static in this case results in an invalid kernel image because the generated code is not relocatable. I think it's a matter of principle that it shouldn't be the linker's responsibility to decide how to combine our code-generation choices with our linking choices. I'm open to suggestions for other ways to solve this issue though as we'd really like to be able to use ThinLTO for our kernel.

Have you considered linking the kernel as a PIE image when KASLR is enabled? This will fix not only this problem but also others (e.g. initializers containing global pointers, which might not just be explicit in the source code but could also be implicitly generated by the compiler, are broken with this scheme).

This is what the Linux kernel does (it uses -shared -Bsymbolic instead of -pie in KASLR mode, but they are almost equivalent):
https://github.com/torvalds/linux/blob/fec88ab0af9706b2201e5daf377c5031c62d11f7/arch/arm64/Makefile#L21

It uses assembly to self relocate to avoid depending on relative relocations during early startup:
https://github.com/torvalds/linux/blob/fec88ab0af9706b2201e5daf377c5031c62d11f7/arch/arm64/kernel/head.S#L822

[phosek]

In D64711#1586738, @pcc wrote:

In D64711#1586696, @phosek wrote:

In D64711#1584916, @pcc wrote:

I'd like to understand the motivation behind this change a little better because the relaxation of the code model from PIC to static is technically an "optimization" that the linker should have license to perform and in fact is already performing in some ways (e.g. emitting addresses into the binary instead of RELATIVE/RELR relocations for statically known addresses).

In the case of our kernel, we need the generated code to be position independent for our KASLR scheme, but we're not doing traditional PIE link. When compiling individual translation units, we use -fpie, but when linking we use --no-pie. Changing the code model from PIC to static in this case results in an invalid kernel image because the generated code is not relocatable. I think it's a matter of principle that it shouldn't be the linker's responsibility to decide how to combine our code-generation choices with our linking choices. I'm open to suggestions for other ways to solve this issue though as we'd really like to be able to use ThinLTO for our kernel.

Have you considered linking the kernel as a PIE image when KASLR is enabled? This will fix not only this problem but also others (e.g. initializers containing global pointers, which might not just be explicit in the source code but could also be implicitly generated by the compiler, are broken with this scheme).

This is what the Linux kernel does (it uses -shared -Bsymbolic instead of -pie in KASLR mode, but they are almost equivalent):
https://github.com/torvalds/linux/blob/fec88ab0af9706b2201e5daf377c5031c62d11f7/arch/arm64/Makefile#L21

It uses assembly to self relocate to avoid depending on relative relocations during early startup:
https://github.com/torvalds/linux/blob/fec88ab0af9706b2201e5daf377c5031c62d11f7/arch/arm64/kernel/head.S#L822

We're considering it but it requires some non-trivial changes and so won't be ready anytime soon.

[pcc]

In D64711#1586814, @phosek wrote:

In D64711#1586738, @pcc wrote:

In D64711#1586696, @phosek wrote:

In D64711#1584916, @pcc wrote:

I'd like to understand the motivation behind this change a little better because the relaxation of the code model from PIC to static is technically an "optimization" that the linker should have license to perform and in fact is already performing in some ways (e.g. emitting addresses into the binary instead of RELATIVE/RELR relocations for statically known addresses).

In the case of our kernel, we need the generated code to be position independent for our KASLR scheme, but we're not doing traditional PIE link. When compiling individual translation units, we use -fpie, but when linking we use --no-pie. Changing the code model from PIC to static in this case results in an invalid kernel image because the generated code is not relocatable. I think it's a matter of principle that it shouldn't be the linker's responsibility to decide how to combine our code-generation choices with our linking choices. I'm open to suggestions for other ways to solve this issue though as we'd really like to be able to use ThinLTO for our kernel.

Have you considered linking the kernel as a PIE image when KASLR is enabled? This will fix not only this problem but also others (e.g. initializers containing global pointers, which might not just be explicit in the source code but could also be implicitly generated by the compiler, are broken with this scheme).

This is what the Linux kernel does (it uses -shared -Bsymbolic instead of -pie in KASLR mode, but they are almost equivalent):
https://github.com/torvalds/linux/blob/fec88ab0af9706b2201e5daf377c5031c62d11f7/arch/arm64/Makefile#L21

It uses assembly to self relocate to avoid depending on relative relocations during early startup:
https://github.com/torvalds/linux/blob/fec88ab0af9706b2201e5daf377c5031c62d11f7/arch/arm64/kernel/head.S#L822

We're considering it but it requires some non-trivial changes and so won't be ready anytime soon.

I guess I'd be fine with making it so that passing -relocation-model explicitly would override lld's decision (see e.g. how we handle function sections/data sections in the gold plugin). Then you can use -mllvm -relocation-model=pic for now.

[phosek]

In D64711#1586864, @pcc wrote:

I guess I'd be fine with making it so that passing -relocation-model explicitly would override lld's decision (see e.g. how we handle function sections/data sections in the gold plugin). Then you can use -mllvm -relocation-model=pic for now.

I'd like to better understand the rationale behind the current implementation: eliding relocations feels qualitatively different because that's just an optimization of the linking work, whereas what's being done today in lld is effectively ignoring/overriding the (per-TU) compile-time decision which can lead to surprising results as in our case where we explicitly rely on the distinction between the compilation and linking stages. The LTO code generator should be allowed to relax when instructed to target non-PIC, but that's orthogonal to the linker's role as a linker.

Ultimately what really matters to us is that there's some combination of switches that does what we say, so if there's a strong desire to preserve the current semantics, I'm happy to implement the option to override the relocation model via -mllvm -relocation-model=pic as suggested.

[pcc]

As I see it it's not too different from e.g. internalizing or hiding symbols or propagating DSO-local, which is, in the end, also overriding a compile-time decision. I think that taking this sort of stance in general makes it hard to say where to draw the line on what kind of optimizations are permitted. If we do switch to the compile-time relocation model, should the backend then be prevented from generating jump tables with absolute addresses when linking without -pie or -shared because it could cause problems if the binary happens to end up being loaded at the wrong address? That doesn't seem like a good idea to me. It seems more practical to say, at least in this case, that the binary that you get from a non-PIC link is, in fact, position-dependent, and it is an error to use it position-independently.

[MaskRay]

I am with Peter. I don't know much about LTO but just wanted to say it is fairly common to compile with the less efficient -fPIC, and then link twice: -no-pie/-pie (executable) and -shared. A lot of linker optimizations (got->pcrel, TLS GD/LD->IE/LE, internalizing, etc) happen when you link -fPIC objects in the -no-pie/-pie way. Passing -mllvm -relocation-model explicitly to achieve your intended effect seems practical. It just should not be the default.

[phosek]

This was addressed in rL366644.

lld/Common/TargetOptionsCommandFlags.cpp

 // Define an externally visible version of
 // initTargetOptionsFromCodeGenFlags, so that its functionality can be
 // used without having to include llvm/CodeGen/CommandFlags.inc, which
 // would lead to multiple definitions of the command line flags.
 llvm::TargetOptions lld::initTargetOptionsFromCodeGenFlags() {
   return ::InitTargetOptionsFromCodeGenFlags();
 }
 
+llvm::Optional<llvm::Reloc::Model> lld::getRelocModelFromCMModel() {
+  return getRelocModel();
+}
+
 llvm::Optional<llvm::CodeModel::Model> lld::getCodeModelFromCMModel() {
   return getCodeModel();
 }
 
 std::string lld::getCPUStr() { return ::getCPUStr(); }
 
 std::vector<std::string> lld::getMAttrs() { return ::MAttrs; }

lld/ELF/LTO.cpp

static lto::Config createConfig() {
   c.Options = initTargetOptionsFromCodeGenFlags();
   c.Options.RelaxELFRelocations = true;
   c.Options.EmitAddrsig = true;
 
   // Always emit a section per function/datum with LTO.
   c.Options.FunctionSections = true;
   c.Options.DataSections = true;
 
-  if (config->relocatable)
-    c.RelocModel = None;
-  else if (config->isPic)
-    c.RelocModel = Reloc::PIC_;
-  else
-    c.RelocModel = Reloc::Static;
-
+  c.RelocModel = getRelocModelFromCMModel();
   c.CodeModel = getCodeModelFromCMModel();
   c.DisableVerify = config->disableVerify;
   c.DiagHandler = diagnosticHandler;
   c.OptLevel = config->ltoo;
   c.CPU = getCPUStr();
   c.MAttrs = getMAttrs();
   c.CGOptLevel = args::getCGOptLevel(config->ltoo);
 

lld/include/lld/Common/TargetOptionsCommandFlags.h

 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Target/TargetOptions.h"
 
 namespace lld {
 llvm::TargetOptions initTargetOptionsFromCodeGenFlags();
+llvm::Optional<llvm::Reloc::Model> getRelocModelFromCMModel();
 llvm::Optional<llvm::CodeModel::Model> getCodeModelFromCMModel();
 std::string getCPUStr();
 std::vector<std::string> getMAttrs();
 }

lld/test/ELF/lto/discard-value-names.ll

 @GlobalValueName = global i32 0
 
 define i32 @foo(i32 %in) {
 somelabel:
   %GV = load i32, i32* @GlobalValueName
   %add = add i32 %in, %GV
   ret i32 %add
 }
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"PIC Level", i32 2}

lld/test/ELF/lto/relax-relocs.ll

 ; REQUIRES: x86
 ; RUN: llvm-as %s -o %t.o
 ; RUN: ld.lld -save-temps -shared %t.o -o %t.so
 ; RUN: llvm-readobj -r %t.so.lto.o | FileCheck %s
 
 ; Test that we produce R_X86_64_REX_GOTPCRELX instead of R_X86_64_GOTPCREL
 ; CHECK: R_X86_64_REX_GOTPCRELX foo
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 @foo = external global i32
 define i32 @bar() {
   %t = load i32, i32* @foo
   ret i32 %t
 }
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"PIC Level", i32 2}

lld/test/ELF/lto/relocation-model.ll

 ; REQUIRES: x86
 
 ; RUN: llvm-as %s -o %t.o
 ; RUN: llvm-as %p/Inputs/relocation-model-pic.ll -o %t.pic.o
 
 ;; Non-PIC source.
 
-; RUN: ld.lld %t.o -o %t-out -save-temps -shared
+; RUN: ld.lld %t.o -o %t-out -save-temps -shared -mllvm -relocation-model=pic
 ; RUN: llvm-readobj -r %t-out.lto.o | FileCheck %s --check-prefix=PIC
 
-; RUN: ld.lld %t.o -o %t-out -save-temps --export-dynamic --noinhibit-exec -pie
+; RUN: ld.lld %t.o -o %t-out -save-temps --export-dynamic --noinhibit-exec -pie -mllvm -relocation-model=pic
 ; RUN: llvm-readobj -r %t-out.lto.o | FileCheck %s --check-prefix=PIC
 
-; RUN: ld.lld %t.o -o %t-out -save-temps --export-dynamic --noinhibit-exec
+; RUN: ld.lld %t.o -o %t-out -save-temps --export-dynamic --noinhibit-exec -mllvm -relocation-model=static
 ; RUN: llvm-readobj -r %t-out.lto.o | FileCheck %s --check-prefix=STATIC
 
 ; RUN: ld.lld %t.o -o %t-out -save-temps -r --export-dynamic
 ; RUN: llvm-readobj -r %t-out.lto.o | FileCheck %s --check-prefix=STATIC
 
 
 ;; PIC source.
 
-; RUN: ld.lld %t.pic.o -o %t-out -save-temps -shared
+; RUN: ld.lld %t.pic.o -o %t-out -save-temps -shared -mllvm -relocation-model=pic
 ; RUN: llvm-readobj -r %t-out.lto.o | FileCheck %s --check-prefix=PIC
 
-; RUN: ld.lld %t.pic.o -o %t-out -save-temps --export-dynamic --noinhibit-exec -pie
+; RUN: ld.lld %t.pic.o -o %t-out -save-temps --export-dynamic --noinhibit-exec -pie -mllvm -relocation-model=pic
 ; RUN: llvm-readobj -r %t-out.lto.o | FileCheck %s --check-prefix=PIC
 
-; RUN: ld.lld %t.pic.o -o %t-out -save-temps --export-dynamic --noinhibit-exec
+; RUN: ld.lld %t.pic.o -o %t-out -save-temps --export-dynamic --noinhibit-exec -mllvm -relocation-model=static
 ; RUN: llvm-readobj -r %t-out.lto.o | FileCheck %s --check-prefix=STATIC
 
 ; RUN: ld.lld %t.pic.o -o %t-out -save-temps -r --export-dynamic
 ; RUN: llvm-readobj -r %t-out.lto.o | FileCheck %s --check-prefix=PIC
 
 
 ; PIC: R_X86_64_REX_GOTPCRELX foo
 ; STATIC: R_X86_64_PC32 foo

lld/test/ELF/lto/section-name.ll

 define hidden i32* @use1() {
   ret i32* @__start_foo_section
 }
 
 define i32* @use2() {
   ret i32* @__stop_bar_section
 }
 
+!llvm.module.flags = !{!0, !1}
+!0 = !{i32 1, !"PIC Level", i32 2}
+!1 = !{i32 1, !"PIE Level", i32 2}
+
 ; CHECK-NOT: zed_section
 ; CHECK:     foo_section PROGBITS
 ; CHECK-NEXT:     bar_section PROGBITS
 ; CHECK-NOT: zed_section
 
 ; GC-NOT: zed_section
 ; GC-NOT: foo_section
 ; GC:     bar_section PROGBITS
 ; GC-NOT: zed_section
 ; GC-NOT: foo_section

lld/test/ELF/lto/unnamed-addr.ll

 ; REQUIRES: x86
 ; RUN: llvm-as %s -o %t.o
 ; RUN: ld.lld %t.o -o %t.so -save-temps -shared
 ; RUN: llvm-dis %t.so.0.4.opt.bc -o - | FileCheck %s
 
 target triple = "x86_64-unknown-linux-gnu"
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 
 @a = internal unnamed_addr constant i8 42
 
 define i8* @f() {
   ret i8* @a
 }
 
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"PIC Level", i32 2}
+
 ; CHECK: @a = internal unnamed_addr constant i8 42

llvm/include/llvm/LTO/Config.h

 /// LTO configuration. A linker can configure LTO by setting fields in this data
 /// structure and passing it to the lto::LTO constructor.
 struct Config {
   // Note: when adding fields here, consider whether they need to be added to
   // computeCacheKey in LTO.cpp.
   std::string CPU;
   TargetOptions Options;
   std::vector<std::string> MAttrs;
-  Optional<Reloc::Model> RelocModel = Reloc::PIC_;
+  Optional<Reloc::Model> RelocModel = None;
   Optional<CodeModel::Model> CodeModel = None;
   CodeGenOpt::Level CGOptLevel = CodeGenOpt::Default;
   TargetMachine::CodeGenFileType CGFileType = TargetMachine::CGFT_ObjectFile;
   unsigned OptLevel = 2;
   bool DisableVerify = false;
 
   /// Use the new pass manager
   bool UseNewPM = false;

llvm/test/LTO/Resolution/X86/not-prevailing-alias.ll

 ; Test to ensure that dead alias are dropped by converting to a declaration
 ; RUN: opt -module-summary %s -o %t1.bc
-; RUN: llvm-lto2 run %t1.bc -r %t1.bc,barAlias,x \
+; RUN: llvm-lto2 run -relocation-model=pic \
+; RUN:   %t1.bc -r %t1.bc,barAlias,x \
 ; RUN:   -r %t1.bc,bar,x -r %t1.bc,zed,px \
 ; RUN:   -r %t1.bc,var,x -r %t1.bc,varAlias,x \
 ; RUN:   -o %t2.o -save-temps
 
 ; Check that bar and barAlias were dropped to declarations
 ; RUN: llvm-dis %t2.o.1.1.promote.bc -o - | FileCheck %s --check-prefix=DROP
 ; DROP-DAG: declare void @bar()
 ; DROP-DAG: declare void @barAlias()

llvm/test/LTO/X86/codemodel-1.ll

 ; RUN: llvm-as %s -o %t.o
-; RUN: llvm-lto2 run -r %t.o,_start,px %t.o -o %t.s
+; RUN: llvm-lto2 run -relocation-model=pic %t.o -r %t.o,_start,px -o %t.s
 ; RUN: llvm-objdump -d %t.s.0 | FileCheck %s --check-prefix=CHECK-SMALL
 
 target triple = "x86_64-unknown-linux-gnu"
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 
 !llvm.module.flags = !{!0, !1}
 
 !0 = !{i32 1, !"wchar_size", i32 4}