Re: [PATCH v2] RISC-V: vDSO: Correct inline assembly constraints in the getrandom syscall wrapper

[Vineet Gupta <vineetg@rivosinc.com>]

On 6/6/25 02:24, Xi Ruoyao wrote:
> As recently pointed out by Thomas, if a register is forced for two
> different register variables, among them one is used as "+" (both input
> and output) and another is only used as input, Clang would treat the
> conflicting input parameters as undefined behaviour and optimize away
> the argument assignment.
>
> Per an example in the GCC documentation, for this purpose we can use "="
> (only output) for the output, and "0" for the input for that we must
> reuse the same register as the output.  And GCC developers have
> confirmed using a simple "r" (that we use for most vDSO implementations)
> instead of "0" is also fine.
>
> Link: https://lore.kernel.org/all/20250603-loongarch-vdso-syscall-v1-1-6d12d6dfbdd0@linutronix.de/
> Link: https://gcc.gnu.org/onlinedocs/gcc-15.1.0/gcc/Local-Register-Variables.html
> Link: https://gcc.gnu.org/pipermail/gcc-help/2025-June/144266.html
> Cc: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> ---
>
> v1 -> v2: Keep using "r" for buffer to follow the existing convention
> (that the GCC developers have confirmed fine).
>
>  arch/riscv/include/asm/vdso/getrandom.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/vdso/getrandom.h b/arch/riscv/include/asm/vdso/getrandom.h
> index 8dc92441702a..c6d66895c1f5 100644
> --- a/arch/riscv/include/asm/vdso/getrandom.h
> +++ b/arch/riscv/include/asm/vdso/getrandom.h
> @@ -18,7 +18,7 @@ static __always_inline ssize_t getrandom_syscall(void *_buffer, size_t _len, uns
>  	register unsigned int flags asm("a2") = _flags;
>  
>  	asm volatile ("ecall\n"
> -		      : "+r" (ret)
> +		      : "=r" (ret)
>  		      : "r" (nr), "r" (buffer), "r" (len), "r" (flags)
>  		      : "memory");

My 2 cents as I've dabbled into this for ARC glibc syscall macros [1] where r0
is both the first syscall/function arg and also the function/syscall return.

The v2 approach still keeps 2 different variables in same local reg which has
potential for any future compiler shenanigans.
Segher's example avoided specifying the same reg.
What about something like the following: seems to generate the right code (with
gcc 15)

       register long ret asm("a0");
       register long nr asm("a7") = __NR_getrandom;
       register size_t len asm("a1") = _len;
       register unsigned int flags asm("a2") = _flags;
       ret = (unsigned long) _buffer;

       asm volatile ("ecall\n"
                     : "+r" (ret)                                 // keep "+r"
for input _buffer / output ret
                     : "r" (nr), "r" (len), "r" (flags)
                     : "memory");

       return ret;

Thx,
-Vineet

[1] https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/arc/sysdep.h