On Fri, Aug 7, 2020 at 6:43 PM Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote: > > On Fri, Aug 07, 2020 at 02:54:39PM -0700, Nick Desaulniers wrote: > > On Fri, Aug 7, 2020 at 2:29 PM Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote: > > > > > > On Fri, Aug 07, 2020 at 12:41:00PM -0700, Nick Desaulniers wrote: > > > > A recent change to a default value of configuration variable > > > > (ENABLE_X86_RELAX_RELOCATIONS OFF -> ON) in LLVM now causes Clang's > > > > integrated assembler to emit R_X86_64_GOTPCRELX/R_X86_64_REX_GOTPCRELX > > > > relocations. LLD will relax instructions with these relocations based on > > > > whether the image is being linked as position independent or not. When > > > > not, then LLD will relax these instructions to use absolute addressing > > > > mode (R_RELAX_GOT_PC_NOPIC). This causes kernels built with Clang > > > > and linked with LLD to fail to boot. > > > > > > It could also cause kernels compiled with gcc and linked with LLD to > > > fail in the same way, no? The gcc/gas combination will generate the > > > relaxed relocations from I think gas-2.26 onward. Although the only > > > troublesome symbol in the case of gcc/gas is trampoline_32bit_src, > > > referenced from pgtable_64.c (gcc doesn't use a GOTPC reloc for _pgtable > > > etc). > > > > Thanks for taking a look, and the feedback. I appreciate it! > > > > $ gcc --version | head -n 1 > > gcc (Debian 9.3.0-11) 9.3.0 > > $ make -j71 clean defconfig bzImage > > $ llvm-readelf -r arch/x86/boot/compressed/*.o | grep -e > > R_X86_64_GOTPCRELX -e R_X86_64_REX_GOTPCRELX > > 0000000000000114 000000120000002a R_X86_64_REX_GOTPCRELX > > 0000000000000000 trampoline_32bit_src - 4 > > $ llvm-readelf -r arch/x86/boot/compressed/vmlinux | grep -e > > R_X86_64_GOTPCRELX -e R_X86_64_REX_GOTPCRELX > > $ > > > > So it looks like yes. I guess then we'd need to add a check for > > CONFIG_LD_IS_LLD and CONFIG_CC_IS_GCC and binutils version is 2.26+? > > I don't mind adding support for that combination, but I'd like to skip > > it in this patch for the sake of backporting something small to stable > > to get our CI green ASAP, since CONFIG_LD_IS_LLD probably doesn't > > exist for those stable branches, which will complicate the backport of > > such a patch. So I'd do it in a follow up patch if we're cool with > > that? > > > > What if we did it only if we couldn't enable -pie, like the below patch? > I think this should cover all the cases without needing LD_IS_LLD > checks. > > For BFD, the only case that should change is binutils-2.26, which > supports relaxations but not -z noreloc-overflow, and will now have > relax-relocations disabled. It currently works (with gcc) only because > the relaxation of > movq foo@GOTPCREL(%rip), %reg > to > movq $foo, %reg > in the non-pie case was only added in 2.27, which is also when -z > noreloc-overflow was added, allowing -pie to be enabled. With 2.26, it > only gets relaxed to > leaq foo(%rip), %reg > which is all LLD currently does as well. Sure, that will work, too. If you'd like to send it along, please add my: Reviewed-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx> Tested-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx> > > diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile > index 8abc30b27ba3..d25bb71f195a 100644 > --- a/arch/x86/boot/compressed/Makefile > +++ b/arch/x86/boot/compressed/Makefile > @@ -60,6 +60,13 @@ else > KBUILD_LDFLAGS += $(shell $(LD) --help 2>&1 | grep -q "\-z noreloc-overflow" \ > && echo "-z noreloc-overflow -pie --no-dynamic-linker") > endif > + > +# Disable relocation relaxation if not building as PIE > +ifeq ($(filter -pie,$(KBUILD_LDFLAGS)),) > +KBUILD_CFLAGS += $(call as-option, -Wa$(comma)-mrelax-relocations=no) > +KBUILD_AFLAGS += $(call as-option, -Wa$(comma)-mrelax-relocations=no) > +endif > + > LDFLAGS_vmlinux := -T > > hostprogs := mkpiggy -- Thanks, ~Nick Desaulniers
