| Message ID | 20220530112417.2492510-1-ardb@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: entry: add .ltorg directive to keep literals in range | expand |
On Mon, May 30, 2022 at 01:24:17PM +0200, Ard Biesheuvel wrote: > LKP reports a build issue on Clang, related to a literal load of > __current issued through the ldr_va macro. This turns out to be due to > the fact that group relocations are disabled when CONFIG_COMPILE_TEST=y, > which means that the ldr_va macro resolves to a pair of LDR > instructions, the first one being a literal load issued too far from its > literal pool. For the record, I do see a similar failure with GNU as, not sure if it is worth calling that out in the commit message or not: arch/arm/kernel/entry-common.S: Assembler messages: arch/arm/kernel/entry-common.S:149: Error: invalid literal constant: pool needs to be closer > Due to the introduction of a couple of new uses of this macro in commit > 508074607c7b95b2 ("ARM: 9195/1: entry: avoid explicit literal loads"), > the literal pools end up getting rearranged in a way that causes the > literal for __current to go out of range. Let's fix this up by putting a > .ltorg directive in a suitable place in the code. > > Cc: Nathan Chancellor <nathan@kernel.org> > Cc: Nick Desaulniers <ndesaulniers@google.com> > Fixes: 508074607c7b95b2 ("ARM: 9195/1: entry: avoid explicit literal loads") > Reported-by: kernel test robot <lkp@intel.com> > Link: https://lore.kernel.org/all/202205290805.1vZLAr36-lkp@intel.com/ > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> With the problematic configuration, I no longer see issues with either clang or GCC: Tested-by: Nathan Chancellor <nathan@kernel.org> > --- > arch/arm/kernel/entry-common.S | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S > index 7aa3ded4af92..6a447ac67d80 100644 > --- a/arch/arm/kernel/entry-common.S > +++ b/arch/arm/kernel/entry-common.S > @@ -298,14 +298,15 @@ local_restart: > 9001: > sub lr, saved_pc, #4 > str lr, [sp, #S_PC] > get_thread_info tsk > b ret_fast_syscall > #endif > ENDPROC(vector_swi) > + .ltorg > > /* > * This is the really slow path. We're going to be doing > * context switches, and waiting for our parent to respond. > */ > __sys_trace: > add r0, sp, #S_OFF > -- > 2.30.2 >
On Tue, 31 May 2022 at 17:29, Nathan Chancellor <nathan@kernel.org> wrote: > > On Mon, May 30, 2022 at 01:24:17PM +0200, Ard Biesheuvel wrote: > > LKP reports a build issue on Clang, related to a literal load of > > __current issued through the ldr_va macro. This turns out to be due to > > the fact that group relocations are disabled when CONFIG_COMPILE_TEST=y, > > which means that the ldr_va macro resolves to a pair of LDR > > instructions, the first one being a literal load issued too far from its > > literal pool. > > For the record, I do see a similar failure with GNU as, not sure if it > is worth calling that out in the commit message or not: > > arch/arm/kernel/entry-common.S: Assembler messages: > arch/arm/kernel/entry-common.S:149: Error: invalid literal constant: pool needs to be closer > Correct, the underlying issue is not specific to Clang. I did notice, though, that the Clang assembler does not deduplicate literal pool entries like GAS does, which might make Clang more susceptible than GCC/binutils. In any case, the fix is not Clang-specific. > > Due to the introduction of a couple of new uses of this macro in commit > > 508074607c7b95b2 ("ARM: 9195/1: entry: avoid explicit literal loads"), > > the literal pools end up getting rearranged in a way that causes the > > literal for __current to go out of range. Let's fix this up by putting a > > .ltorg directive in a suitable place in the code. > > > > Cc: Nathan Chancellor <nathan@kernel.org> > > Cc: Nick Desaulniers <ndesaulniers@google.com> > > Fixes: 508074607c7b95b2 ("ARM: 9195/1: entry: avoid explicit literal loads") > > Reported-by: kernel test robot <lkp@intel.com> > > Link: https://lore.kernel.org/all/202205290805.1vZLAr36-lkp@intel.com/ > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > With the problematic configuration, I no longer see issues with either > clang or GCC: > > Tested-by: Nathan Chancellor <nathan@kernel.org> > Thanks. > > --- > > arch/arm/kernel/entry-common.S | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S > > index 7aa3ded4af92..6a447ac67d80 100644 > > --- a/arch/arm/kernel/entry-common.S > > +++ b/arch/arm/kernel/entry-common.S > > @@ -298,14 +298,15 @@ local_restart: > > 9001: > > sub lr, saved_pc, #4 > > str lr, [sp, #S_PC] > > get_thread_info tsk > > b ret_fast_syscall > > #endif > > ENDPROC(vector_swi) > > + .ltorg > > > > /* > > * This is the really slow path. We're going to be doing > > * context switches, and waiting for our parent to respond. > > */ > > __sys_trace: > > add r0, sp, #S_OFF > > -- > > 2.30.2 > >
On Wed, Jun 1, 2022 at 4:36 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > On Tue, 31 May 2022 at 17:29, Nathan Chancellor <nathan@kernel.org> wrote: > > > > On Mon, May 30, 2022 at 01:24:17PM +0200, Ard Biesheuvel wrote: > > > LKP reports a build issue on Clang, related to a literal load of > > > __current issued through the ldr_va macro. This turns out to be due to > > > the fact that group relocations are disabled when CONFIG_COMPILE_TEST=y, > > > which means that the ldr_va macro resolves to a pair of LDR > > > instructions, the first one being a literal load issued too far from its > > > literal pool. > > > > For the record, I do see a similar failure with GNU as, not sure if it > > is worth calling that out in the commit message or not: > > > > arch/arm/kernel/entry-common.S: Assembler messages: > > arch/arm/kernel/entry-common.S:149: Error: invalid literal constant: pool needs to be closer > > > > Correct, the underlying issue is not specific to Clang. I did notice, > though, that the Clang assembler does not deduplicate literal pool > entries like GAS does, which might make Clang more susceptible than > GCC/binutils. Thanks for the report; just circling back on this, but this should be resolved in ToT (clang-15): https://github.com/llvm/llvm-project/issues/55816 > > In any case, the fix is not Clang-specific.
On Wed, 8 Jun 2022 at 20:42, Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Wed, Jun 1, 2022 at 4:36 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > On Tue, 31 May 2022 at 17:29, Nathan Chancellor <nathan@kernel.org> wrote: > > > > > > On Mon, May 30, 2022 at 01:24:17PM +0200, Ard Biesheuvel wrote: > > > > LKP reports a build issue on Clang, related to a literal load of > > > > __current issued through the ldr_va macro. This turns out to be due to > > > > the fact that group relocations are disabled when CONFIG_COMPILE_TEST=y, > > > > which means that the ldr_va macro resolves to a pair of LDR > > > > instructions, the first one being a literal load issued too far from its > > > > literal pool. > > > > > > For the record, I do see a similar failure with GNU as, not sure if it > > > is worth calling that out in the commit message or not: > > > > > > arch/arm/kernel/entry-common.S: Assembler messages: > > > arch/arm/kernel/entry-common.S:149: Error: invalid literal constant: pool needs to be closer > > > > > > > Correct, the underlying issue is not specific to Clang. I did notice, > > though, that the Clang assembler does not deduplicate literal pool > > entries like GAS does, which might make Clang more susceptible than > > GCC/binutils. > > Thanks for the report; just circling back on this, but this should be > resolved in ToT (clang-15): > https://github.com/llvm/llvm-project/issues/55816 > Wow, that was quick!
diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S index 7aa3ded4af92..6a447ac67d80 100644 --- a/arch/arm/kernel/entry-common.S +++ b/arch/arm/kernel/entry-common.S @@ -298,14 +298,15 @@ local_restart: 9001: sub lr, saved_pc, #4 str lr, [sp, #S_PC] get_thread_info tsk b ret_fast_syscall #endif ENDPROC(vector_swi) + .ltorg /* * This is the really slow path. We're going to be doing * context switches, and waiting for our parent to respond. */ __sys_trace: add r0, sp, #S_OFF
LKP reports a build issue on Clang, related to a literal load of __current issued through the ldr_va macro. This turns out to be due to the fact that group relocations are disabled when CONFIG_COMPILE_TEST=y, which means that the ldr_va macro resolves to a pair of LDR instructions, the first one being a literal load issued too far from its literal pool. Due to the introduction of a couple of new uses of this macro in commit 508074607c7b95b2 ("ARM: 9195/1: entry: avoid explicit literal loads"), the literal pools end up getting rearranged in a way that causes the literal for __current to go out of range. Let's fix this up by putting a .ltorg directive in a suitable place in the code. Cc: Nathan Chancellor <nathan@kernel.org> Cc: Nick Desaulniers <ndesaulniers@google.com> Fixes: 508074607c7b95b2 ("ARM: 9195/1: entry: avoid explicit literal loads") Reported-by: kernel test robot <lkp@intel.com> Link: https://lore.kernel.org/all/202205290805.1vZLAr36-lkp@intel.com/ Signed-off-by: Ard Biesheuvel <ardb@kernel.org> --- arch/arm/kernel/entry-common.S | 1 + 1 file changed, 1 insertion(+)