Skip to content

"ID map text too big or misaligned" after LLD commit 86d24193a9eb45d7bf3745fc2de96cd4e197b08a #812

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
nathanchance opened this issue Dec 14, 2019 · 12 comments
Assignees
Labels
[ARCH] arm64 This bug impacts ARCH=arm64 [BUG] llvm (main) A bug in an unreleased version of LLVM (this label is appropriate for regressions) [CONFIG] allyesconfig Issue affects allyesconfig on certain architectures [FIXED][LLVM] main This bug was only present and fixed in an unreleased version of LLVM [FIXED][LLVM] 10 This bug was fixed in LLVM 10.0 [TOOL] lld The issue is relevant to LLD linker

Comments

@nathanchance
Copy link
Member

Building arm64 allyesconfig after llvm/llvm-project@86d2419 results in a failed assertion in the arm64 linking script:

ld.lld: error: ID map text too big or misaligned
ld.lld: error: ID map text too big or misaligned
ld.lld: error: ID map text too big or misaligned
ld.lld: error: ID map text too big or misaligned

https://github.com/torvalds/linux/blob/master/arch/arm64/kernel/vmlinux.lds.S#L265-L266

Bisect log:

# bad: [ad73f656b3c49f3462625ebeaec7a32f63d11d18] gn build: add deps I failed to add in b2508ce85c1
# good: [d4345636e678ccab8a87b09cdad9129e54c23100] [LegalizeTypes] Remove manual worklist management from SoftenFloatRes_FP_EXTEND.
git bisect start 'origin/master' 'origin/master~318'
# bad: [bf13a71095fd95897cca032e194d73b88af22dea] AMDGPU/SILoadStoreOptimizer: Simplify function
git bisect bad bf13a71095fd95897cca032e194d73b88af22dea
# bad: [5882e6f36fd9bfc7382e6763c5591b3497428d83] [analyzer] Escape symbols conjured into specific regions during a conservative EvalCall
git bisect bad 5882e6f36fd9bfc7382e6763c5591b3497428d83
# good: [bb9254c00757ec376cde435676d27b14ee0c582f] Removing an unused selection field from a diagnostic; NFC.
git bisect good bb9254c00757ec376cde435676d27b14ee0c582f
# bad: [5a3a9e9927b714e94e1c1b839e17429806272052] [ELF][AArch64] Rename --force-bti to -z force-bti and --pac-plt to -z pac-plt
git bisect bad 5a3a9e9927b714e94e1c1b839e17429806272052
# bad: [df494f7512b0ecebdf3d7be97695a1b6278c0336] [Support] Add TimeTraceScope constructor without detail arg
git bisect bad df494f7512b0ecebdf3d7be97695a1b6278c0336
# bad: [86d24193a9eb45d7bf3745fc2de96cd4e197b08a] [LLD][ELF][AArch64][ARM] When errata patching, round thunk size to 4KiB.
git bisect bad 86d24193a9eb45d7bf3745fc2de96cd4e197b08a
# good: [c0a3ab365514e126b694e009503d537d0e67eb01] Revert "[AArch64][SVE] Implement intrinsics for non-temporal loads & stores"
git bisect good c0a3ab365514e126b694e009503d537d0e67eb01
# good: [247b2ce11cf0b9efeb3c1b0394dcc87ccab7be41] [LLD][ELF][AArch64][ARM] Add missing classof to patch sections.
git bisect good 247b2ce11cf0b9efeb3c1b0394dcc87ccab7be41
# first bad commit: [86d24193a9eb45d7bf3745fc2de96cd4e197b08a] [LLD][ELF][AArch64][ARM] When errata patching, round thunk size to 4KiB.

I've done no investigation past this. I tested mainline at torvalds@e31736d.

@nathanchance nathanchance added [BUG] Untriaged Something isn't working [ARCH] arm64 This bug impacts ARCH=arm64 [TOOL] lld The issue is relevant to LLD linker labels Dec 14, 2019
@nickdesaulniers
Copy link
Member

cc @MaskRay @smithp35

@smithp35
Copy link

smithp35 commented Dec 16, 2019

Looking at the file:

/*
 * The HYP init code and ID map text can't be longer than a page each,
 * and should not cross a page boundary.
 */
ASSERT(__idmap_text_end - (__idmap_text_start & ~(SZ_4K - 1)) <= SZ_4K,
	"ID map text too big or misaligned")

The commit in question (mine), should round up the thunk section size to 4k when the cortex-a53 or cortex-a8 errata patch is on. This was to prevent LLD not converging when there was a very large binary and the addition of thunks kept generating more patches ad inifinitum. By rounding the size of the thunk section to 4k this prevented the addition of a thunk disturbing the address of following sections modulo 4k (the errata is sensitive to address modulo 4k).

if __idmap_text contains a Thunk then it will exceed 4k and trip that assertion.

At the moment the only way to fix that is to either ensure that the section doesn't contain a thunk, or turn off the errata patches.

In the long term if it is required to have thunks and errata patches and have the total size < 4k, then we'll need to do one of:
1.) Make the errata patch insertion code more sophisticated (it is much simpler than thunks) so that it can have its size increased to 4k rather than thunks.
2.) Only round up to 4k when the size of the OutputSection is large enough to need it.

I'm on vacation right now, but if this is critical I can look at it. Option 2 should be fairly simple to implement.

@nathanchance
Copy link
Member Author

I'm on vacation right now, but if this is critical I can look at it. Option 2 should be fairly simple to implement.

I think this can wait until after your vacation :) enjoy!

@smithp35
Copy link

smithp35 commented Jan 7, 2020

I've submitted https://reviews.llvm.org/D72344 to only turn this on for OutputSections that are larger than the ThunkSectionSpacing which is close to 128 Mb. I'd be grateful if you could give it a try.

I was initially confident that this would work for assertion but looking at the linker script in more detail this might not be enough. I initially thought
#define IDMAP_TEXT
. = ALIGN(SZ_4K);
__idmap_text_start = .;
(.idmap.text)
__idmap_text_end = .;was the whole OutputSection, when in fact it is just part of a larger.text : { /
Real text segment /
_stext = .; /
Text and read-only data */
IRQENTRY_TEXT
SOFTIRQENTRY_TEXT
ENTRY_TEXT
TEXT_TEXT
SCHED_TEXT
CPUIDLE_TEXT
LOCK_TEXT
KPROBES_TEXT
HYPERVISOR_TEXT
IDMAP_TEXT
HIBERNATE_TEXT
TRAMP_TEXT
*(.fixup)
*(.gnu.warning)
. = ALIGN(16);
(.got) / Global offset table */
}`
If the overall size of all of the .text is > 128 Mb then in theory this problem could still trigger, it just so happens that the idmap may need range-extension thunks and the way LLD will place them will insert them into the point between __idmap_text_start and __idmap_text_end. In theory I think you could get this problem with ld.bfd as it also rounds up the size of its stub section when patches are enabled, however the different placement strategy of stubs makes this much less likely to happen.

If the allyesconfig is still failing I'll have to think again. I can't think of many universally good answers here. Thoughts:

  • Make a more expensive check than OutputSection Size. In effect find the position of the ThunkSection in the OutputSection and only round the size up if there is a significant amount of code after the ThunkSection.
  • Check the InputSectionDescription size rather than the OutputSection Size, this will work for Chromium and the kernel, but there are combinations of InputSectionDescription size lower than the limit that may fail in the future. There are probably some heuristics that can be used, for example if the InputSectionDescription is really small then don't round-up the size as the number of Thunks/Patches that can be generated from the range is small.
  • Collect together the Patches inside a single section like thunks, round the size of this up to 4 KiB instead of thunks. It is unlikely that __idmap_text has any patches.
  • Provide a command line option to disable rounding up the size of thunks.

@nathanchance
Copy link
Member Author

Unfortunately, with D72344 applied and building v5.5-rc5 arm64 allyesconfig, I still see this error :(

@MaskRay
Copy link
Member

MaskRay commented Jan 7, 2020

@nathanchance Do you have easy-to-reproduce command(s)?

Edit: I can reproduce. My Linux repository is at ~/Dev/linux, https://github.com/ClangBuiltLinux/continuous-integration is at ~/Dev/continuous-integration. My LLVM build directory is ~/llvm/Release.

make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- CC=~/llvm/Release/bin/clang LD=~/llvm/Release/bin/ld.lld O=out/arm64 KCONFIG_ALLCONFIG=~/Dev/ClangBuiltLinux/tc-build/kernel/le.config allyesconfig Image.gz  -j 30

After I make a change to lld, I can run

make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- CC=~/llvm/Release/bin/clang LD=~/llvm/Release/bin/ld.lld O=out/arm64 KCONFIG_ALLCONFIG=~/Dev/ClangBuiltLinux/tc-build/kernel/le.config Image.gz

Linker output:

  LD      .tmp_vmlinux1
ld.lld: error: ID map text too big or misaligned
ld.lld: error: ID map text too big or misaligned
ld.lld: error: ID map text too big or misaligned
ld.lld: error: ID map text too big or misaligned

@nathanchance
Copy link
Member Author

@MaskRay this is enough for me with 5.5-rc5:

$ make -j$(nproc) -s ARCH=arm64 CC=clang CROSS_COMPILE=aarch64-linux-gnu- LD=ld.lld O=out.aarch64 distclean allyesconfig vmlinux
...
ld.lld: error: ID map text too big or misaligned
ld.lld: error: ID map text too big or misaligned
ld.lld: error: ID map text too big or misaligned
ld.lld: error: ID map text too big or misaligned
...

@smithp35
Copy link

smithp35 commented Jan 8, 2020

Thanks for the reproducible commands. I'll take a look and see what I can come up with.

@smithp35
Copy link

smithp35 commented Jan 8, 2020

Looking at the map file the overall size of the .text OutputSection is indeed > 128 MiB so my check on the OutputSection size won't be enough. The size of the code between __idmap_text_start and __idmap_text_end is < 2 KiB. There looks to be only 1 thunk generated for the InputSectionDescription, but that would be enough to fail the assertion.

I've got one urgent thing I need to do first before I can get back to this, at worst case it might be first thing next week, but hopefully I'll have something before then.

@nathanchance nathanchance added the [CONFIG] allyesconfig Issue affects allyesconfig on certain architectures label Jan 13, 2020
@smithp35
Copy link

Apologies for the delay, I've uploaded a new fix to https://reviews.llvm.org/D72344 I've made it so that the ThunkSection has its size rounded up when:

  • The errata fix is on
  • There may be more code after the ThunkSection
  • The InputSectionDescription the Thunks will be placed in is larger than 4 KiB.
    The last will prevent any expansion within InputSectionDescriptions like:
    . = ALIGN(SZ_4K); __idmap_text_start = .; (.idmap.text) __idmap_text_end = .;

@nathanchance nathanchance added the [PATCH] Submitted A patch has been submitted for review label Jan 15, 2020
@nathanchance
Copy link
Member Author

I can confirm this fixes the build error on my end! I have no other real way to test if this is a correct fix or not, I'll leave that to the patch reviewers :) thanks for the fix!

@nickdesaulniers nickdesaulniers added [BUG] llvm A bug that should be fixed in upstream LLVM [BUG] llvm (main) A bug in an unreleased version of LLVM (this label is appropriate for regressions) and removed [BUG] Untriaged Something isn't working [BUG] llvm A bug that should be fixed in upstream LLVM labels Jan 16, 2020
@nickdesaulniers nickdesaulniers added [FIXED][LLVM] 10 This bug was fixed in LLVM 10.0 [FIXED][LLVM] main This bug was only present and fixed in an unreleased version of LLVM and removed [FIXED][LLVM] 10 This bug was fixed in LLVM 10.0 [PATCH] Submitted A patch has been submitted for review labels Jan 17, 2020
arichardson pushed a commit to arichardson/llvm-project that referenced this issue Jan 20, 2020
In D71281 a fix was put in to round up the size of a ThunkSection to the
nearest 4KiB when performing errata patching. This fixed a problem with a
very large instrumented program that had thunks and patches mutually
trigger each other. Unfortunately it triggers an assertion failure in an
AArch64 allyesconfig build of the kernel. There is a specific assertion
preventing an InputSectionDescription being larger than 4KiB. This will
always trigger if there is at least one Thunk needed in that
InputSectionDescription, which is possible for an allyesconfig build.

Abstractly the problem case is:
.text : {
          *(.text) ;
          ...
          . = ALIGN(SZ_4K);
          __idmap_text_start = .;
          *(.idmap.text)
          __idmap_text_end = .;
          ...
        }
The assertion checks that __idmap_text_end - __idmap_start is < 4 KiB.
Note that there is more than one InputSectionDescription in the
OutputSection so we can't just restrict the fix to OutputSections smaller
than 4 KiB.

The fix presented here limits the D71281 to InputSectionDescriptions that
meet the following conditions:
1.) The OutputSection is bigger than the thunkSectionSpacing so adding
thunks will affect the addresses of following code.
2.) The InputSectionDescription is larger than 4 KiB. This will prevent
any assertion failures that an InputSectionDescription is < 4 KiB
in size.

We do this at ThunkSection creation time as at this point we know that
the addresses are stable and up to date prior to adding the thunks as
assignAddresses() will have been called immediately prior to thunk
generation.

The fix reverts the two tests affected by D71281 to their original state
as they no longer need the 4KiB size roundup. I've added simpler tests to
check for D71281 when the OutputSection size is larger than the ThunkSection
spacing.

Fixes ClangBuiltLinux/linux#812

Differential Revision: https://reviews.llvm.org/D72344
@nathanchance
Copy link
Member Author

nathanchance commented Feb 4, 2020

I've requested that the commit that fixes this be added to LLVM 10 since it missed the branch: https://llvm.org/pr44764

zmodem pushed a commit to llvm/llvm-project that referenced this issue Feb 4, 2020
In D71281 a fix was put in to round up the size of a ThunkSection to the
nearest 4KiB when performing errata patching. This fixed a problem with a
very large instrumented program that had thunks and patches mutually
trigger each other. Unfortunately it triggers an assertion failure in an
AArch64 allyesconfig build of the kernel. There is a specific assertion
preventing an InputSectionDescription being larger than 4KiB. This will
always trigger if there is at least one Thunk needed in that
InputSectionDescription, which is possible for an allyesconfig build.

Abstractly the problem case is:
.text : {
          *(.text) ;
          ...
          . = ALIGN(SZ_4K);
          __idmap_text_start = .;
          *(.idmap.text)
          __idmap_text_end = .;
          ...
        }
The assertion checks that __idmap_text_end - __idmap_start is < 4 KiB.
Note that there is more than one InputSectionDescription in the
OutputSection so we can't just restrict the fix to OutputSections smaller
than 4 KiB.

The fix presented here limits the D71281 to InputSectionDescriptions that
meet the following conditions:
1.) The OutputSection is bigger than the thunkSectionSpacing so adding
thunks will affect the addresses of following code.
2.) The InputSectionDescription is larger than 4 KiB. This will prevent
any assertion failures that an InputSectionDescription is < 4 KiB
in size.

We do this at ThunkSection creation time as at this point we know that
the addresses are stable and up to date prior to adding the thunks as
assignAddresses() will have been called immediately prior to thunk
generation.

The fix reverts the two tests affected by D71281 to their original state
as they no longer need the 4KiB size roundup. I've added simpler tests to
check for D71281 when the OutputSection size is larger than the ThunkSection
spacing.

Fixes ClangBuiltLinux/linux#812

Differential Revision: https://reviews.llvm.org/D72344

(cherry picked from commit 01ad4c8)
@nickdesaulniers nickdesaulniers added the [FIXED][LLVM] 10 This bug was fixed in LLVM 10.0 label Feb 7, 2020
ajohnson-uoregon pushed a commit to ajohnson-uoregon/clang-rewrite-only that referenced this issue Jul 17, 2022
In D71281 a fix was put in to round up the size of a ThunkSection to the
nearest 4KiB when performing errata patching. This fixed a problem with a
very large instrumented program that had thunks and patches mutually
trigger each other. Unfortunately it triggers an assertion failure in an
AArch64 allyesconfig build of the kernel. There is a specific assertion
preventing an InputSectionDescription being larger than 4KiB. This will
always trigger if there is at least one Thunk needed in that
InputSectionDescription, which is possible for an allyesconfig build.

Abstractly the problem case is:
.text : {
          *(.text) ;
          ...
          . = ALIGN(SZ_4K);
          __idmap_text_start = .;
          *(.idmap.text)
          __idmap_text_end = .;
          ...
        }
The assertion checks that __idmap_text_end - __idmap_start is < 4 KiB.
Note that there is more than one InputSectionDescription in the
OutputSection so we can't just restrict the fix to OutputSections smaller
than 4 KiB.

The fix presented here limits the D71281 to InputSectionDescriptions that
meet the following conditions:
1.) The OutputSection is bigger than the thunkSectionSpacing so adding
thunks will affect the addresses of following code.
2.) The InputSectionDescription is larger than 4 KiB. This will prevent
any assertion failures that an InputSectionDescription is < 4 KiB
in size.

We do this at ThunkSection creation time as at this point we know that
the addresses are stable and up to date prior to adding the thunks as
assignAddresses() will have been called immediately prior to thunk
generation.

The fix reverts the two tests affected by D71281 to their original state
as they no longer need the 4KiB size roundup. I've added simpler tests to
check for D71281 when the OutputSection size is larger than the ThunkSection
spacing.

Fixes ClangBuiltLinux/linux#812

Differential Revision: https://reviews.llvm.org/D72344

(cherry picked from commit 501f912)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[ARCH] arm64 This bug impacts ARCH=arm64 [BUG] llvm (main) A bug in an unreleased version of LLVM (this label is appropriate for regressions) [CONFIG] allyesconfig Issue affects allyesconfig on certain architectures [FIXED][LLVM] main This bug was only present and fixed in an unreleased version of LLVM [FIXED][LLVM] 10 This bug was fixed in LLVM 10.0 [TOOL] lld The issue is relevant to LLD linker
Projects
None yet
Development

No branches or pull requests

4 participants