Re: [PATCH v2] RISC-V: vDSO: Correct inline assembly constraints in the getrandom syscall wrapper
From: Alexandre Ghiti
Date: Tue Jun 10 2025 - 04:15:36 EST
Hi,
On 6/10/25 09:11, Thomas Weißschuh wrote:
On Sat, Jun 07, 2025 at 10:16:34PM +0800, Xi Ruoyao wrote:
On Fri, 2025-06-06 at 15:01 -0700, Vineet Gupta wrote:
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@xxxxxxxxxxxxx/
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@xxxxxxxxxxxxx>
Cc: Nathan Chancellor <nathan@xxxxxxxxxx>
Signed-off-by: Xi Ruoyao <xry111@xxxxxxxxxxx>
---
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");
Then it would be better to rename this variable to just "a0". And I
guess Thomas doesn't want a new convention different from all other
syscall wrappers in vDSO...
Indeed. I want to keep it consistent. Especially for a bugfix.
Speaking of which, IMO this patch should have a Fixes tag.
Yes, here it is:
Fixes: ee0d03053e70 ("RISC-V: vDSO: Wire up getrandom() vDSO
implementation")
Then we could start a new discussion about changing it to something else everywhere.
Although I don't think that the single-variable variant is better.
Vineet feel free to propose something for all architectures if you think
that's better.
For now, I'll merge this version for inclusion in -rc2,
Thanks,
Alex
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
--
Xi Ruoyao <xry111@xxxxxxxxxxx>
School of Aerospace Science and Technology, Xidian University
_______________________________________________
linux-riscv mailing list
linux-riscv@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-riscv