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

[Ard Biesheuvel]

On Mon, 5 May 2025 at 13:50, H. Peter Anvin <hpa@xxxxxxxxx> wrote:
>
> On May 5, 2025 3:42:46 AM PDT, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
> >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.
>
> It is documented to produce an (%rip) reference in the gcc documentation, and gcc is the reference for inline assembly.
>

Got a link? The one below documents it as a generic modifier, and does
not mention rIP at all.

https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Generic-Operand-Modifiers

> Furthermore, documentation is not reality. Code is.

Not sure I follow your point here. You claim Clang is buggy because
GCC does not behave according to its own documentation?