Skip to content

--pic-vineer for arm32 lld #286

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nickdesaulniers opened this issue Dec 10, 2018 · 14 comments
Closed

--pic-vineer for arm32 lld #286

nickdesaulniers opened this issue Dec 10, 2018 · 14 comments
Assignees
Labels
[ARCH] arm32 This bug impacts ARCH=arm [BUG] llvm A bug that should be fixed in upstream LLVM feature-request Not a bug per-se [FIXED][LLVM] 8 This bug was fixed in LLVM 8.0 [TOOL] lld The issue is relevant to LLD linker

Comments

@nickdesaulniers
Copy link
Member

https://reviews.llvm.org/D55505

@nickdesaulniers nickdesaulniers added [BUG] llvm A bug that should be fixed in upstream LLVM feature-request Not a bug per-se [TOOL] lld The issue is relevant to LLD linker [ARCH] arm32 This bug impacts ARCH=arm [PATCH] Submitted A patch has been submitted for review labels Dec 10, 2018
@nathanchance
Copy link
Member

nathanchance commented Dec 14, 2018

I tested this patch with defconfig, where it works without any issues. However, with allyesconfig, I get the following errors:

ld.lld: error: relocation R_ARM_MOVT_PREL out of range: -4410603944 is not in [-2147483648, 2147483647]
ld.lld: error: relocation R_ARM_MOVT_PREL out of range: -4411261224 is not in [-2147483648, 2147483647]
ld.lld: error: relocation R_ARM_MOVT_PREL out of range: -4411458860 is not in [-2147483648, 2147483647]
ld.lld: error: relocation R_ARM_MOVT_PREL out of range: -4408657128 is not in [-2147483648, 2147483647]
ld.lld: error: relocation R_ARM_MOVT_PREL out of range: -4392320160 is not in [-2147483648, 2147483647]
ld.lld: error: relocation R_ARM_MOVT_PREL out of range: -4412194940 is not in [-2147483648, 2147483647]
ld.lld: error: relocation R_ARM_MOVT_PREL out of range: -4412002028 is not in [-2147483648, 2147483647]
ld.lld: error: relocation R_ARM_MOVT_PREL out of range: -4410626800 is not in [-2147483648, 2147483647]
ld.lld: error: relocation R_ARM_MOVT_PREL out of range: -4392320280 is not in [-2147483648, 2147483647]
ld.lld: error: relocation R_ARM_MOVT_PREL out of range: -4410135732 is not in [-2147483648, 2147483647]
ld.lld: error: relocation R_ARM_MOVT_PREL out of range: -4410937664 is not in [-2147483648, 2147483647]
ld.lld: error: relocation R_ARM_MOVT_PREL out of range: -4375736156 is not in [-2147483648, 2147483647]
ld.lld: error: relocation R_ARM_MOVT_PREL out of range: -4411188840 is not in [-2147483648, 2147483647]
ld.lld: error: relocation R_ARM_MOVT_PREL out of range: -4412035056 is not in [-2147483648, 2147483647]
ld.lld: error: relocation R_ARM_MOVT_PREL out of range: -4392320928 is not in [-2147483648, 2147483647]
ld.lld: error: relocation R_ARM_MOVT_PREL out of range: -4410970328 is not in [-2147483648, 2147483647]
ld.lld: error: relocation R_ARM_MOVT_PREL out of range: -4375736960 is not in [-2147483648, 2147483647]
ld.lld: error: relocation R_ARM_MOVT_PREL out of range: -4411222636 is not in [-2147483648, 2147483647]
ld.lld: error: relocation R_ARM_MOVT_PREL out of range: -4392320420 is not in [-2147483648, 2147483647]
ld.lld: error: relocation R_ARM_MOVT_PREL out of range: -4411261436 is not in [-2147483648, 2147483647]
ld.lld: error: too many errors emitted, stopping now (use -error-limit=0 to see all errors)

I'm exhausted so debugging probably isn't a great idea at the moment but I should be able to triage further tomorrow (unless someone beats me to it).

Just an FYI, lld without this patch + https://lore.kernel.org/lkml/[email protected]/ links (with a couple of hacks to work around #35 + #287). Here is an mbox that can be applied on top of -next to test/verify: curl https://gist.githubusercontent.com/nathanchance/90b04d2d08d6faff799461d586e6ebcd/raw/29d9209c5eca828277f85b85dd2489556f7e867c/gistfile1.txt | git am

@ardbiesheuvel
Copy link

GCC never uses place-relative movw/movt pairs, and while it seems appealing for position independent code, the fact that ARM uses REL type relocations (which put the addend in the place rather in than in the relocation), and the fact that the addend needs to be duplicated between the two instructions (or you wouldn't be able to handle a carry between the low and the high part ), the addend range is only [-32k, 32k), and I am not surprised this breaks when building an allyesconfig kernel (which is huge)

I know Clang does support -mno-movt, but this gets rid of movw/movt pairs entirely, which would be unfortunate.

@nathanchance
Copy link
Member

Ah, so that particular set of errors is something that needs to fixed/addressed in Clang?

@nickdesaulniers
Copy link
Member Author

cc @smithp35

@smithp35
Copy link

I will take a look when I get back to the office next January. I'm interested to see where the PC-Relative use of movt/movw comes from. They should only be generated from compiled code when position independent, but when -fpic or -fpie is used it looks like care is taken to avoid using movt/movw.

@ardbiesheuvel
Copy link

ardbiesheuvel commented Dec 21, 2018

The implementation of ARMV7PILongThunk emits place-relative movw/movt pairs.

My suspicion is that calls from __init code to ordinary code with static linkage in the same object file is causing the addends to overflow here, since the .init code section is far away from the ordinary code section, and the .text section will be much larger than 32 KB on a allyesconfig kernel.

I think this is a fundamental problem that could occur in any large program where functions in the same object file may be emitted into different ELF sections, e.g., hot/cold partitioning or -ffunction-sections. So perhaps the V7 PI long thunk should be dropped entirely in favor of the v5 one.

@smithp35
Copy link

Thanks for the pointer. I have made an example with a simple reproducer:
.text
.arm
.global _start
.type _start, %function
_start:
bx lr
.global low
.type low, %function
low:
bl high

    .section .text_high, "ax", %progbits
    .global high
    .type high, %function

high: bl low
bx lr

SECTIONS {
. = SIZEOF_HEADERS;
.text_low : { *(.text) }
.text_high 0xffff0000 : { *(.text_high) }
}

ld.lld test.o -o test.axf -fpie --script=test.lds
ld.lld: error: relocation R_ARM_MOVT_PREL out of range: -4294901536 is not in [-2147483648, 2147483647]

This is consistent with the overflow checking for the movt relocation looking for a signed 32-bit integer, and also why we haven't seen this before as LLD has tended to be used on applications with continuous address spaces. I don't think that this overflow behaviour is right (at the stage that it is being evaluated) so I'll have to investigate both the overflow behaviour and what thunks are appropriate next year.

@smithp35
Copy link

smithp35 commented Jan 7, 2019

I've submitted https://reviews.llvm.org/D56396 that should fix the relocation overflow problem. The ELF for the ARM Architecture document states that the relocations shouldn't have overflow checking, and it is possible for LLD to inject the values it needs to into the movt/movw pair so I've done that.

@ardbiesheuvel
Copy link

OK, so the limited range of the addend is not a problem here since we never actually have to deal with the static relocation in the thunk case, right?

@nathanchance
Copy link
Member

nathanchance commented Jan 7, 2019

@smithp35 thank you for the fix, I can link an arm32 allyesconfig kernel with D55505 + D56396.

@smithp35
Copy link

smithp35 commented Jan 7, 2019

Yes, in this particular case we are just reusing the part of the relocation code that encodes the final result so we don't have to deal with the encoding of the addend in the relocation.

@nickdesaulniers nickdesaulniers added [PATCH] Accepted A submitted patch has been accepted upstream and removed [PATCH] Submitted A patch has been submitted for review labels Jan 15, 2019
@nickdesaulniers
Copy link
Member Author

Looks like both are now fixed. @smithp35 , will these been in lld-8, or do we need to ask Hans to pick these up?

@smithp35
Copy link

smithp35 commented Jan 17, 2019

--pic-veneer just missed the branch. I've created https://llvm.org/pr40352 and linked it to the 8.0 release https://llvm.org/pr40331 as a blocker.

@nathanchance
Copy link
Member

Added to the 8.0 branch: llvm/llvm-project@37174c6

Thanks @smithp35!

@nathanchance nathanchance added [FIXED][LLVM] 8 This bug was fixed in LLVM 8.0 and removed [PATCH] Accepted A submitted patch has been accepted upstream labels Jan 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[ARCH] arm32 This bug impacts ARCH=arm [BUG] llvm A bug that should be fixed in upstream LLVM feature-request Not a bug per-se [FIXED][LLVM] 8 This bug was fixed in LLVM 8.0 [TOOL] lld The issue is relevant to LLD linker
Projects
None yet
Development

No branches or pull requests

4 participants