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

[H. Peter Anvin]

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...