On 6/4/25 22:05, Huacai Chen wrote:
On Tue, Jun 3, 2025 at 7:49 PM Thomas Weißschuh thomas.weissschuh@linutronix.de wrote:
The syscall wrappers use the "a0" register for two different register variables, both the first argument and the return value. The "ret" variable is used as both input and output while the argument register is only used as input. Clang treats the conflicting input parameters as undefined behaviour and optimizes away the argument assignment.
The code seems to work by chance for the most part today but that may change in the future. Specifically clock_gettime_fallback() fails with clockids from 16 to 23, as implemented by the upcoming auxiliary clocks.
Switch the "ret" register variable to a pure output, similar to the other architectures' vDSO code. This works in both clang and GCC.
Hmmm, at first the constraint is "=r", during the progress of upstream, Xuerui suggested me to use "+r" instead [1]. [1] https://lore.kernel.org/linux-arch/5b14144a-9725-41db-7179-c059c41814cf@xen0...
Oops, I've already completely forgotten! That said...
I didn't notice back then, that `ret` and the first parameter actually shared the same manually allocated register, so I replied as if the two shared one variable. If it were me to write the original code, I would re-used `ret` for arg0 (with a comment explaining the a0 situation) so that "+r" could be properly used there without UB.
As for the current situation -- both this patch's approach or my alternative above are OK to me. Feel free to take either; and have my R-b tag if you send a v2.
Reviewed-by: WANG Xuerui git@xen0n.name
Thanks!