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

[H. Peter Anvin]

On May 5, 2025 5:01:26 AM PDT, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
>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?

Yes. Because it isn't the documentation that is authoritative... especially not for inline asm (the gcc documentation notably lags behind the code, and it is only recently that it has even begun catching up.)

The same is true for nearly all interfaces. x86, the Linux ABI, you pick it. If the man pages don't match the kernel, that's a bug in the man pages, and if someone tries to build a Linux compatibility layer based on a buggy man page, that's still wrong. 

Understanding this has been a major friction between the kernel and compiler communities for decades, and it seems that when a new compiler group (first clang, later rust) gets in contact with the kernel this is a barrier that needs to be re-overcome every time.