[PATCH] ARM: entry: add .ltorg directive to keep literals in range

[Ard Biesheuvel]

On Tue, 31 May 2022 at 17:29, Nathan Chancellor <nathan at 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 at kernel.org>
> > Cc: Nick Desaulniers <ndesaulniers at google.com>
> > Fixes: 508074607c7b95b2 ("ARM: 9195/1: entry: avoid explicit literal loads")
> > Reported-by: kernel test robot <lkp at intel.com>
> > Link: https://lore.kernel.org/all/202205290805.1vZLAr36-lkp@intel.com/
> > Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
>
> With the problematic configuration, I no longer see issues with either
> clang or GCC:
>
> Tested-by: Nathan Chancellor <nathan at 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
> >