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.
Link: https://lore.kernel.org/lkml/20250602102825-42aa84f0-23f1-4d10-89fc-e8bbaffd... Link: https://lore.kernel.org/lkml/20250519082042.742926976@linutronix.de/ Fixes: c6b99bed6b8f ("LoongArch: Add VDSO and VSYSCALL support") Fixes: 18efd0b10e0f ("LoongArch: vDSO: Wire up getrandom() vDSO implementation") Cc: stable@vger.kernel.org Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de --- arch/loongarch/include/asm/vdso/getrandom.h | 2 +- arch/loongarch/include/asm/vdso/gettimeofday.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/loongarch/include/asm/vdso/getrandom.h b/arch/loongarch/include/asm/vdso/getrandom.h index 48c43f55b039b42168698614d0479b7a872d20f3..a81724b69f291ee49dd1f46b12d6893fc18442b8 100644 --- a/arch/loongarch/include/asm/vdso/getrandom.h +++ b/arch/loongarch/include/asm/vdso/getrandom.h @@ -20,7 +20,7 @@ static __always_inline ssize_t getrandom_syscall(void *_buffer, size_t _len, uns
asm volatile( " syscall 0\n" - : "+r" (ret) + : "=r" (ret) : "r" (nr), "r" (buffer), "r" (len), "r" (flags) : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8", "memory"); diff --git a/arch/loongarch/include/asm/vdso/gettimeofday.h b/arch/loongarch/include/asm/vdso/gettimeofday.h index 88cfcf13311630ed5f1a734d23a2bc3f65d79a88..f15503e3336ca1bdc9675ec6e17bbb77abc35ef4 100644 --- a/arch/loongarch/include/asm/vdso/gettimeofday.h +++ b/arch/loongarch/include/asm/vdso/gettimeofday.h @@ -25,7 +25,7 @@ static __always_inline long gettimeofday_fallback(
asm volatile( " syscall 0\n" - : "+r" (ret) + : "=r" (ret) : "r" (nr), "r" (tv), "r" (tz) : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8", "memory"); @@ -44,7 +44,7 @@ static __always_inline long clock_gettime_fallback(
asm volatile( " syscall 0\n" - : "+r" (ret) + : "=r" (ret) : "r" (nr), "r" (clkid), "r" (ts) : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8", "memory"); @@ -63,7 +63,7 @@ static __always_inline int clock_getres_fallback(
asm volatile( " syscall 0\n" - : "+r" (ret) + : "=r" (ret) : "r" (nr), "r" (clkid), "r" (ts) : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8", "memory");
--- base-commit: 546b1c9e93c2bb8cf5ed24e0be1c86bb089b3253 change-id: 20250603-loongarch-vdso-syscall-f585a99bea03
Best regards,
On Tue, Jun 03, 2025 at 01:48:54PM +0200, Thomas Weißschuh 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.
Link: https://lore.kernel.org/lkml/20250602102825-42aa84f0-23f1-4d10-89fc-e8bbaffd... Link: https://lore.kernel.org/lkml/20250519082042.742926976@linutronix.de/ Fixes: c6b99bed6b8f ("LoongArch: Add VDSO and VSYSCALL support") Fixes: 18efd0b10e0f ("LoongArch: vDSO: Wire up getrandom() vDSO implementation") Cc: stable@vger.kernel.org Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de
This is definitely an odd interaction because of the register variables using the same value.
Reviewed-by: Nathan Chancellor nathan@kernel.org
arch/loongarch/include/asm/vdso/getrandom.h | 2 +- arch/loongarch/include/asm/vdso/gettimeofday.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/loongarch/include/asm/vdso/getrandom.h b/arch/loongarch/include/asm/vdso/getrandom.h index 48c43f55b039b42168698614d0479b7a872d20f3..a81724b69f291ee49dd1f46b12d6893fc18442b8 100644 --- a/arch/loongarch/include/asm/vdso/getrandom.h +++ b/arch/loongarch/include/asm/vdso/getrandom.h @@ -20,7 +20,7 @@ static __always_inline ssize_t getrandom_syscall(void *_buffer, size_t _len, uns asm volatile( " syscall 0\n"
- : "+r" (ret)
- : "=r" (ret) : "r" (nr), "r" (buffer), "r" (len), "r" (flags) : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8", "memory");
diff --git a/arch/loongarch/include/asm/vdso/gettimeofday.h b/arch/loongarch/include/asm/vdso/gettimeofday.h index 88cfcf13311630ed5f1a734d23a2bc3f65d79a88..f15503e3336ca1bdc9675ec6e17bbb77abc35ef4 100644 --- a/arch/loongarch/include/asm/vdso/gettimeofday.h +++ b/arch/loongarch/include/asm/vdso/gettimeofday.h @@ -25,7 +25,7 @@ static __always_inline long gettimeofday_fallback( asm volatile( " syscall 0\n"
- : "+r" (ret)
- : "=r" (ret) : "r" (nr), "r" (tv), "r" (tz) : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8", "memory");
@@ -44,7 +44,7 @@ static __always_inline long clock_gettime_fallback( asm volatile( " syscall 0\n"
- : "+r" (ret)
- : "=r" (ret) : "r" (nr), "r" (clkid), "r" (ts) : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8", "memory");
@@ -63,7 +63,7 @@ static __always_inline int clock_getres_fallback( asm volatile( " syscall 0\n"
- : "+r" (ret)
- : "=r" (ret) : "r" (nr), "r" (clkid), "r" (ts) : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8", "memory");
base-commit: 546b1c9e93c2bb8cf5ed24e0be1c86bb089b3253 change-id: 20250603-loongarch-vdso-syscall-f585a99bea03
Best regards,
Thomas Weißschuh thomas.weissschuh@linutronix.de
在 6/3/25 7:48 PM, Thomas WeiÃschuh 写道:
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.
I don't know Clang and GCC, but I think it's great to do so.
Reviewed-by: Yanteng Si si.yanteng@linux.dev
Thanks, Yanteng
Link: https://lore.kernel.org/lkml/20250602102825-42aa84f0-23f1-4d10-89fc-e8bbaffd... Link: https://lore.kernel.org/lkml/20250519082042.742926976@linutronix.de/ Fixes: c6b99bed6b8f ("LoongArch: Add VDSO and VSYSCALL support") Fixes: 18efd0b10e0f ("LoongArch: vDSO: Wire up getrandom() vDSO implementation") Cc: stable@vger.kernel.org Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de
arch/loongarch/include/asm/vdso/getrandom.h | 2 +- arch/loongarch/include/asm/vdso/gettimeofday.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/loongarch/include/asm/vdso/getrandom.h b/arch/loongarch/include/asm/vdso/getrandom.h index 48c43f55b039b42168698614d0479b7a872d20f3..a81724b69f291ee49dd1f46b12d6893fc18442b8 100644 --- a/arch/loongarch/include/asm/vdso/getrandom.h +++ b/arch/loongarch/include/asm/vdso/getrandom.h @@ -20,7 +20,7 @@ static __always_inline ssize_t getrandom_syscall(void *_buffer, size_t _len, uns asm volatile( " syscall 0\n"
- : "+r" (ret)
- : "=r" (ret) : "r" (nr), "r" (buffer), "r" (len), "r" (flags) : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8", "memory");
diff --git a/arch/loongarch/include/asm/vdso/gettimeofday.h b/arch/loongarch/include/asm/vdso/gettimeofday.h index 88cfcf13311630ed5f1a734d23a2bc3f65d79a88..f15503e3336ca1bdc9675ec6e17bbb77abc35ef4 100644 --- a/arch/loongarch/include/asm/vdso/gettimeofday.h +++ b/arch/loongarch/include/asm/vdso/gettimeofday.h @@ -25,7 +25,7 @@ static __always_inline long gettimeofday_fallback( asm volatile( " syscall 0\n"
- : "+r" (ret)
- : "=r" (ret) : "r" (nr), "r" (tv), "r" (tz) : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8", "memory");
@@ -44,7 +44,7 @@ static __always_inline long clock_gettime_fallback( asm volatile( " syscall 0\n"
- : "+r" (ret)
- : "=r" (ret) : "r" (nr), "r" (clkid), "r" (ts) : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8", "memory");
@@ -63,7 +63,7 @@ static __always_inline int clock_getres_fallback( asm volatile( " syscall 0\n"
- : "+r" (ret)
- : "=r" (ret) : "r" (nr), "r" (clkid), "r" (ts) : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8", "memory");
base-commit: 546b1c9e93c2bb8cf5ed24e0be1c86bb089b3253 change-id: 20250603-loongarch-vdso-syscall-f585a99bea03
Best regards,
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...
Huacai
Link: https://lore.kernel.org/lkml/20250602102825-42aa84f0-23f1-4d10-89fc-e8bbaffd... Link: https://lore.kernel.org/lkml/20250519082042.742926976@linutronix.de/ Fixes: c6b99bed6b8f ("LoongArch: Add VDSO and VSYSCALL support") Fixes: 18efd0b10e0f ("LoongArch: vDSO: Wire up getrandom() vDSO implementation") Cc: stable@vger.kernel.org Signed-off-by: Thomas Weißschuh thomas.weissschuh@linutronix.de
arch/loongarch/include/asm/vdso/getrandom.h | 2 +- arch/loongarch/include/asm/vdso/gettimeofday.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/loongarch/include/asm/vdso/getrandom.h b/arch/loongarch/include/asm/vdso/getrandom.h index 48c43f55b039b42168698614d0479b7a872d20f3..a81724b69f291ee49dd1f46b12d6893fc18442b8 100644 --- a/arch/loongarch/include/asm/vdso/getrandom.h +++ b/arch/loongarch/include/asm/vdso/getrandom.h @@ -20,7 +20,7 @@ static __always_inline ssize_t getrandom_syscall(void *_buffer, size_t _len, uns
asm volatile( " syscall 0\n"
: "+r" (ret)
: "=r" (ret) : "r" (nr), "r" (buffer), "r" (len), "r" (flags) : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8", "memory");diff --git a/arch/loongarch/include/asm/vdso/gettimeofday.h b/arch/loongarch/include/asm/vdso/gettimeofday.h index 88cfcf13311630ed5f1a734d23a2bc3f65d79a88..f15503e3336ca1bdc9675ec6e17bbb77abc35ef4 100644 --- a/arch/loongarch/include/asm/vdso/gettimeofday.h +++ b/arch/loongarch/include/asm/vdso/gettimeofday.h @@ -25,7 +25,7 @@ static __always_inline long gettimeofday_fallback(
asm volatile( " syscall 0\n"
: "+r" (ret)
: "=r" (ret) : "r" (nr), "r" (tv), "r" (tz) : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8", "memory");@@ -44,7 +44,7 @@ static __always_inline long clock_gettime_fallback(
asm volatile( " syscall 0\n"
: "+r" (ret)
: "=r" (ret) : "r" (nr), "r" (clkid), "r" (ts) : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8", "memory");@@ -63,7 +63,7 @@ static __always_inline int clock_getres_fallback(
asm volatile( " syscall 0\n"
: "+r" (ret)
: "=r" (ret) : "r" (nr), "r" (clkid), "r" (ts) : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8", "memory");
base-commit: 546b1c9e93c2bb8cf5ed24e0be1c86bb089b3253 change-id: 20250603-loongarch-vdso-syscall-f585a99bea03
Best regards,
Thomas Weißschuh thomas.weissschuh@linutronix.de
On Wed, 2025-06-04 at 22:05 +0800, 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...
Based on the example at https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html:
To force an operand into a register, create a local variable and specify the register name after the variable’s declaration. Then use the local variable for the asm operand and specify any constraint letter that matches the register:
register int *p1 asm ("r0") = …; register int *p2 asm ("r1") = …; register int *result asm ("r0"); asm ("sysint" : "=r" (result) : "0" (p1), "r" (p2));
I think this should actually be written
asm volatile( " syscall 0\n" : "=r" (ret) : "r" (nr), "0" (buffer), "r" (len), "r" (flags) : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8", "memory");
i.e. "=" should be used for the output operand 0, and "0" should be used for the input operand 2 (buffer) to emphasis the same register as operand 0 is used.
On Wed, Jun 04, 2025 at 10:30:55PM +0800, Xi Ruoyao wrote:
On Wed, 2025-06-04 at 22:05 +0800, 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...
Based on the example at https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html:
To force an operand into a register, create a local variable and specify the register name after the variable’s declaration. Then use the local variable for the asm operand and specify any constraint letter that matches the register: register int *p1 asm ("r0") = …; register int *p2 asm ("r1") = …; register int *result asm ("r0"); asm ("sysint" : "=r" (result) : "0" (p1), "r" (p2)); I think this should actually be written
asm volatile( " syscall 0\n" : "=r" (ret) : "r" (nr), "0" (buffer), "r" (len), "r" (flags) : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8", "memory");
i.e. "=" should be used for the output operand 0, and "0" should be used for the input operand 2 (buffer) to emphasis the same register as operand 0 is used.
I would have expected that matching constraints ("0") would only really make sense if the compiler selects the specific register to use. When the register is already selected manually it seems redundant. But my inline ASM knowledge is limited and this is a real example from the GCC docs, so it is probably more correct. On the other hand all the other vDSO implementations use "r" over "0" for the input operand 2 and I'd like to keep them consistent.
Thomas
On Thu, Jun 5, 2025 at 3:37 PM Thomas Weißschuh thomas.weissschuh@linutronix.de wrote:
On Wed, Jun 04, 2025 at 10:30:55PM +0800, Xi Ruoyao wrote:
On Wed, 2025-06-04 at 22:05 +0800, 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...
Based on the example at https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html:
To force an operand into a register, create a local variable and specify the register name after the variable’s declaration. Then use the local variable for the asm operand and specify any constraint letter that matches the register:
register int *p1 asm ("r0") = …; register int *p2 asm ("r1") = …; register int *result asm ("r0"); asm ("sysint" : "=r" (result) : "0" (p1), "r" (p2));
I think this should actually be written
asm volatile( " syscall 0\n" : "=r" (ret) : "r" (nr), "0" (buffer), "r" (len), "r" (flags) : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7","$t8", "memory");
i.e. "=" should be used for the output operand 0, and "0" should be used for the input operand 2 (buffer) to emphasis the same register as operand 0 is used.
I would have expected that matching constraints ("0") would only really make sense if the compiler selects the specific register to use. When the register is already selected manually it seems redundant. But my inline ASM knowledge is limited and this is a real example from the GCC docs, so it is probably more correct. On the other hand all the other vDSO implementations use "r" over "0" for the input operand 2 and I'd like to keep them consistent.
OK, if there are no objections, I will take this patch.
Huacai
Thomas
On Thu, 2025-06-05 at 09:37 +0200, Thomas Weißschuh wrote:
On Wed, Jun 04, 2025 at 10:30:55PM +0800, Xi Ruoyao wrote:
On Wed, 2025-06-04 at 22:05 +0800, 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...
Based on the example at https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html:
To force an operand into a register, create a local variable and specify the register name after the variable’s declaration. Then use the local variable for the asm operand and specify any constraint letter that matches the register: register int *p1 asm ("r0") = …; register int *p2 asm ("r1") = …; register int *result asm ("r0"); asm ("sysint" : "=r" (result) : "0" (p1), "r" (p2)); I think this should actually be written
asm volatile( " syscall 0\n" : "=r" (ret) : "r" (nr), "0" (buffer), "r" (len), "r" (flags) : "$t0", "$t1", "$t2", "$t3", "$t4", "$t5", "$t6", "$t7", "$t8", "memory");
i.e. "=" should be used for the output operand 0, and "0" should be used for the input operand 2 (buffer) to emphasis the same register as operand 0 is used.
I would have expected that matching constraints ("0") would only really make sense if the compiler selects the specific register to use. When the register is already selected manually it seems redundant. But my inline ASM knowledge is limited and this is a real example from the GCC docs, so it is probably more correct. On the other hand all the other vDSO implementations use "r" over "0" for the input operand 2 and I'd like to keep them consistent.
Per https://gcc.gnu.org/pipermail/gcc-help/2025-June/144261.html and https://gcc.gnu.org/pipermail/gcc-help/2025-June/144266.html it should be fine to just use "r".
Reviewed-by: Xi Ruoyao xry111@xry111.site
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!
linux-stable-mirror@lists.linaro.org
-
Huacai Chen -
Nathan Chancellor -
Thomas Weißschuh -
WANG Xuerui -
Xi Ruoyao -
Yanteng Si