Re: [PATCH -tip] x86/asm: Use %a instead of %c(%%rip) in rip_rel_ptr()

[Ard Biesheuvel]

On Mon, 5 May 2025 at 08:09, H. Peter Anvin <hpa@xxxxxxxxx> wrote:
>
> On May 4, 2025 10:48:19 PM PDT, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
> >On Mon, 5 May 2025 at 04:59, H. Peter Anvin <hpa@xxxxxxxxx> wrote:
> >>
> >> On 5/4/25 11:43, Uros Bizjak wrote:
> >> > The "a" asm operand modifier substitutes a memory reference, with the
> >> > actual operand treated as address.  For x86_64, when a symbol is
> >> > provided, the "a" modifier emits "sym(%rip)" instead of "sym".
> >> >
> >
> >Clang does not
> >
> >https://godbolt.org/z/5Y58T45f5
> >
> >> > No functional changes intended.
> >> >
> >
> >NAK. There is a functional change with Clang, which does not emit
> >%(rip), and this is the whole point of this thing.
> >
> >> > Signed-off-by: Uros Bizjak <ubizjak@xxxxxxxxx>
> >> > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> >> > Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> >> > Cc: Borislav Petkov <bp@xxxxxxxxx>
> >> > Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> >> > Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
> >> > Cc: Ard Biesheuvel <ardb@xxxxxxxxxx>
> >> > ---
> >> >   arch/x86/include/asm/asm.h | 2 +-
> >> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
> >> > index f963848024a5..d7610b99b8d8 100644
> >> > --- a/arch/x86/include/asm/asm.h
> >> > +++ b/arch/x86/include/asm/asm.h
> >> > @@ -116,7 +116,7 @@
> >> >   #ifndef __ASSEMBLER__
> >> >   static __always_inline __pure void *rip_rel_ptr(void *p)
> >> >   {
> >> > -     asm("leaq %c1(%%rip), %0" : "=r"(p) : "i"(p));
> >> > +     asm("leaq %a1, %0" : "=r"(p) : "i"(p));
> >> >
> >> >       return p;
> >> >   }
> >>
> >> Also, this function really should be __attribute_const__ rather than __pure.
> >>
> >
> >No it should really not.
> >
> >rip_rel_ptr() will yield different results depending on whether it is
> >called [by the same code] from the 1:1 mapping or the ordinary kernel
> >virtual mapping of memory, and this is basically the entire reason we
> >need it in the first place.
> >
> >Lying to the compiler about this is not a good idea imo.
>
> Goddamnit, another clang bug. Someone fix the damned thing, please...

How is this a bug? The %a modifier is documented as producing a memory
reference - the fact that GCC on x86_64 always emits a RIP-relative
reference regardless of whether we are generating PIC code or not is
an implementation detail, and not fundamental to yield correct code.

Clang produces something that matches the constraints that we gave it
- we cannot blame the tools for the mess of our own making where we
call C code from alternative mappings of memory, while we lie to the
compiler that the code is position dependent and non-relocatable.