arm64 defconfig + CONFIG_RANDOMIZE_BASE ld.lld link failure after r363928

@MaskRay

ld.lld: error: relocation R_AARCH64_ABS32 cannot be used against symbol __efistub_stext_offset; recompile with -fPIC
>>> defined in ./arch/arm64/kernel/vmlinux.lds:52
>>> referenced by efi-entry.S:121 (/home/nathan/cbl/linux/arch/arm64/kernel/efi-entry.S:121)
>>>               efi-entry.stub.o:(.init.text+0xC0) in archive arch/arm64/kernel/built-in.a
make[1]: *** [/home/nathan/cbl/linux/Makefile:1053: vmlinux] Error 1
make[1]: Leaving directory '/mnt/build/kernel'
make: *** [Makefile:179: sub-make] Error 2

llvm/llvm-project@249fde8

I suppose this is expected behavior according to the commit message? Is there something that needs to be changed on the kernel side?

Can be reproduced via:

$ echo "CONFIG_RANDOMIZE_BASE=y" >> arch/arm64/configs/defconfig
$ make -j"$(nproc)" ARCH=arm64 CC=clang CROSS_COMPILE=aarch64-linux-gnu- LD=ld.lld defconfig vmlinux

cc @MaskRay

__efistub_stext_offset should be an absolute symbol, but is (incorrectly) considered a non-absolute symbol in lld. In a PIC link (vmlinuz is linked with -shared), R_AARCH64_ABS32 is not allowed to reference a non-absolute symbol, thus the error. D63383 is a red herring. Without it or the previous D63121, things get worse: R_AARCH64_ABS32 (32-bit) would incorrectly cause the creation of a R_AARCH64_RELATIVE (64-bit).

The best fix I can come up with is:

--- i/arch/arm64/kernel/image.h
+++ w/arch/arm64/kernel/image.h
@@ -70 +70 @@
-__efistub_stext_offset = stext - _text;
+__efistub_stext_offset = ABSOLUTE(stext - _text);

The behavior difference is due to the order ld.bfd and lld evaluate symbol assignments. __efistub_stext_offset is defined before _text. It is not clear how ld.bfd handles __efistub_stext_offset.

A simplified reproduce:

# a.s: llvm-mc -filetype=obj -triple=aarch64 a.s -o a.o
.globl stext
ldr w21, =__efistub_stext_offset   # R_AARCH64_ABS32
stext:

// a.x
// When __efistub_stext_offset is evaluated, both stext and _text should be non-absolute
// and the difference should be absolute. However, due to the order lld evaluates symbols,
// _text is considered absolute.
__efistub_stext_offset = stext - _text;
SECTIONS {
  .text : {
    _text = .;
    *(.text)
  }
}

llvm-mc -filetype=obj -triple=aarch64 a.s -o a.o
ld.lld -shared a.o a.x
# error: can't create dynamic relocation R_AARCH64_ABS32 against symbol...

There are several workarounds:

Change it to __efistub_stext_offset = ABSOLUTE(stext - _text);
Move __efistub_stext_offset = stext - _text; below the SECTIONS command

@ardbiesheuvel

cc @ardbiesheuvel , @smithp35

I just sent a patch for enabling CONFIG_RANDOMIZE_BASE by default for arm64, so supporting this config is important.

If it is at all possible to fix this in the linker script by either of MaskRay's workarounds I recommend doing so as I can't see a simple, safe fix in LLD; unlike in D63383. This isn't to say that it shouldn't be fixed in LLD, but it may take a bit of research into the LLD and BFD parsers and expression evaluators.

At the time "__efistub_stext_offset = stext - _text;" is evaluated, stext is known to be section relative, strictly speaking it is unknown whether _text is section relative or not as it is just a forward reference at this point. Unfortunately LLD treats this as an absolute value as absolute is defined as no Section (or forced absolute). A section relative - absolute is section relative.

When "__efistub_stext_offset = stext - _text;" is evaluated after _text is defined as a section relative symbol then (section relative - section relative) evaluates to an absolute value. ABSOLUTE forces the result to be interpreted in the right context.

Some research is needed about the best way to solve this. My intuition is that I think we need more than one pass in processSectionCommands() or some kind of back-patching to account for expressions that change according to forward references later being defined.

If adding ABSOLUTE() makes this problem go away, at least in the short term, I have absolutely no problem with that.

I can distill all of this down and send a patch if nobody objects.

Tentative patch, let me know if the wording or explanation can be improved:

From e987a48a60182a21e6b28cceb5893acbce4c7a30 Mon Sep 17 00:00:00 2001
From: Nathan Chancellor <natechancellor@gmail.com>
Date: Tue, 25 Jun 2019 20:42:15 -0700
Subject: [PATCH] arm64/efi: Mark __efistub_stext_offset as an absolute symbol
 explicitly

After r363059 and r363928 in LLVM, a build using ld.lld as the linker
with CONFIG_RANDOMIZE_BASE enabled fails like so:

ld.lld: error: relocation R_AARCH64_ABS32 cannot be used against symbol
__efistub_stext_offset; recompile with -fPIC

Fangrui and Peter figured out that ld.lld is incorrectly considering
__efistub_stext_offset as a relative symbol because of the order in
which symbols are evaluated. _text is treated as an absolute symbol
and stext is a relative symbol, making __efistub_stext_offset a
relative symbol.

Adding ABSOLUTE will force ld.lld to evalute this expression in the
right context and does not change ld.bfd's behavior. ld.lld will
need to be fixed but the developers do not see a quick or simple fix
without some research (see the linked issue for further explanation).
Add this simple workaround so that ld.lld can continue to link kernels.

Link: https://github.com/ClangBuiltLinux/linux/issues/561
Link: https://github.com/llvm/llvm-project/commit/025a815d75d2356f2944136269aa5874721ec236
Link: https://github.com/llvm/llvm-project/commit/249fde85832c33f8b06c6b4ac65d1c4b96d23b83
Debugged-by: Fangrui Song <maskray@google.com>
Debugged-by: Peter Smith <peter.smith@linaro.org>
Suggested-by: Fangrui Song <maskray@google.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
 arch/arm64/kernel/image.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/image.h b/arch/arm64/kernel/image.h
index 04ca08086d35..9a2d2227907c 100644
--- a/arch/arm64/kernel/image.h
+++ b/arch/arm64/kernel/image.h
@@ -67,7 +67,7 @@

 #ifdef CONFIG_EFI

-__efistub_stext_offset = stext - _text;
+__efistub_stext_offset = ABSOLUTE(stext - _text);

 /*
  * The EFI stub has its own symbol namespace prefixed by __efistub_, to
-- 
2.22.0

The wording looks good to me. Thanks!

Patch sent: https://lore.kernel.org/lkml/20190626042017.54773-1-natechancellor@gmail.com/

arm64/efi: Mark __efistub_stext_offset as an absolute symbol explicitly

Navigation Menu

Repository navigation

arm64 defconfig + CONFIG_RANDOMIZE_BASE ld.lld link failure after r363928 #561

205 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

arm64 defconfig + CONFIG_RANDOMIZE_BASE ld.lld link failure after r363928 #561

Description

Activity

MaskRay commented on Jun 22, 2019

nickdesaulniers commented on Jun 24, 2019

smithp35 commented on Jun 25, 2019

ardbiesheuvel commented on Jun 25, 2019

nathanchance commented on Jun 25, 2019

nathanchance commented on Jun 26, 2019

MaskRay commented on Jun 26, 2019

nathanchance commented on Jun 26, 2019

205 remaining items

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions