Per the GCC docs, local `register` variables aren't guaranteed to
survive a function call, and you shouldn't put code between the
register assignment and the `asm` block:
https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html
The __nolibc_syscallN() macros put their arguments into local register
variables. syscall() drops caller expressions straight into those
macros, so this:
static char msg[] = "Hello, World!\n";
syscall(__NR_write, 1, msg, strlen(msg));
runs the wrong syscall. On x86-64, GCC doesn't reload %rax, %rdi, and
%rsi after the strlen() call, so only %rdx ends up correct.
Fix it by evaluating the arguments into temporaries first, before they
reach the arch macros.
Before this patch (bug):
```
0000000000401000 <main>:
pushq %rcx
movl $0x403000,%edi
callq 401153 <strlen> # strlen() clobbers %rax, %rdi, %rsi.
movq %rax,%rdx # BUG: %rax, %rdi, %rsi are wrong.
syscall # Only %rdx is correct.
[...]
popq %rdx
retq
```
After this patch (fixed):
```
0000000000401000 <main>:
pushq %rcx
movl $0x403000,%edi
callq 40116c <strlen>
movl $0x1,%edi # %rdi = 1 (stdout)
movl $0x403000,%esi # %rsi = msg
movq %rax,%rdx # %rdx = strlen(msg)
movl $0x1,%eax # %rax = __NR_write
syscall
[...]
popq %rdx
retq
```
Reproduced with gcc on i386 and x86-64 at -O0, -O1, -O2, -O3 and -Os.
Found this bug after a chat with Alviro. I also attempted to report a
similar problem to the GCC bugzilla:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=126290
The solution from GNU developers is: use hard register constraints
(introduced in GCC 16) to reliably force asm operands into specific
registers, though Clang hasn't merged its version yet:
https://gcc.gnu.org/onlinedocs/gcc-16.1.0/gcc/Hard-Register-Constraints.html
https://github.com/llvm/llvm-project/pull/85846
Once hard register constraints are widely available (in the future),
a reliable inline asm for a syscall may look like this:
```
#define syscall6(N, ARG1, ARG2, ARG3, ARG4, ARG5, ARG6) ({ \
long long ret = (N); \
__asm__ volatile( \
"syscall" \
: "+{rax}"(ret) \
: "{rdi}"(ARG1), \
"{rsi}"(ARG2), \
"{rdx}"(ARG3), \
"{r10}"(ARG4), \
"{r8}"(ARG5), \
"{r9}"(ARG6) \
: "rcx", "r11", "memory"); \
(ret); \
})
```
Cc: Yichun Zhang <[email protected]>
Fixes: 53fcfafa8c5c ("tools/nolibc/unistd: add syscall()")
Reported-by: Alviro Iskandar Setiawan <[email protected]>
Signed-off-by: Ammar Faizi <[email protected]>
---
tools/include/nolibc/sys/syscall.h | 72 +++++++++++++++++++++++++++++-
1 file changed, 71 insertions(+), 1 deletion(-)
diff --git a/tools/include/nolibc/sys/syscall.h
b/tools/include/nolibc/sys/syscall.h
index 7f06314fcf0d..b91b82846fe4 100644
--- a/tools/include/nolibc/sys/syscall.h
+++ b/tools/include/nolibc/sys/syscall.h
@@ -10,9 +10,79 @@
#ifndef _NOLIBC_SYS_SYSCALL_H
#define _NOLIBC_SYS_SYSCALL_H
+/*
+ * The __nolibc_syscallN() macros assign their arguments to local register
+ * variables. A compiler only has to keep such a variable in its register
+ * right before the asm statement it feeds, so an argument expression which
+ * contains a function call gets evaluated once the earlier arguments already
+ * sit in their registers, and the call then clobbers them.
+ *
+ * Caller-supplied expressions enter here, so bind them to temporaries first
+ * and only hand plain variables over. __auto_type preserves the original
+ * type, so nothing gets truncated on the way.
+ */
+#define __nolibc_syscall_eval0(_n) \
+({ \
+ __auto_type __sc_n = (_n); \
+ __nolibc_syscall0(__sc_n); \
+})
+#define __nolibc_syscall_eval1(_n, _a1)
\
+({ \
+ __auto_type __sc_n = (_n); \
+ __auto_type __sc_a1 = (_a1); \
+ __nolibc_syscall1(__sc_n, __sc_a1); \
+})
+#define __nolibc_syscall_eval2(_n, _a1, _a2) \
+({ \
+ __auto_type __sc_n = (_n); \
+ __auto_type __sc_a1 = (_a1); \
+ __auto_type __sc_a2 = (_a2); \
+ __nolibc_syscall2(__sc_n, __sc_a1, __sc_a2); \
+})
+#define __nolibc_syscall_eval3(_n, _a1, _a2, _a3) \
+({ \
+ __auto_type __sc_n = (_n); \
+ __auto_type __sc_a1 = (_a1); \
+ __auto_type __sc_a2 = (_a2); \
+ __auto_type __sc_a3 = (_a3); \
+ __nolibc_syscall3(__sc_n, __sc_a1, __sc_a2, __sc_a3); \
+})
+#define __nolibc_syscall_eval4(_n, _a1, _a2, _a3, _a4) \
+({ \
+ __auto_type __sc_n = (_n); \
+ __auto_type __sc_a1 = (_a1); \
+ __auto_type __sc_a2 = (_a2); \
+ __auto_type __sc_a3 = (_a3); \
+ __auto_type __sc_a4 = (_a4); \
+ __nolibc_syscall4(__sc_n, __sc_a1, __sc_a2, __sc_a3, __sc_a4); \
+})
+#define __nolibc_syscall_eval5(_n, _a1, _a2, _a3, _a4, _a5) \
+({ \
+ __auto_type __sc_n = (_n); \
+ __auto_type __sc_a1 = (_a1); \
+ __auto_type __sc_a2 = (_a2); \
+ __auto_type __sc_a3 = (_a3); \
+ __auto_type __sc_a4 = (_a4); \
+ __auto_type __sc_a5 = (_a5); \
+ __nolibc_syscall5(__sc_n, __sc_a1, __sc_a2, __sc_a3, __sc_a4, \
+ __sc_a5); \
+})
+#define __nolibc_syscall_eval6(_n, _a1, _a2, _a3, _a4, _a5, _a6) \
+({ \
+ __auto_type __sc_n = (_n); \
+ __auto_type __sc_a1 = (_a1); \
+ __auto_type __sc_a2 = (_a2); \
+ __auto_type __sc_a3 = (_a3); \
+ __auto_type __sc_a4 = (_a4); \
+ __auto_type __sc_a5 = (_a5); \
+ __auto_type __sc_a6 = (_a6); \
+ __nolibc_syscall6(__sc_n, __sc_a1, __sc_a2, __sc_a3, __sc_a4, \
+ __sc_a5, __sc_a6); \
+})
+
#define ___nolibc_syscall_narg(_0, _1, _2, _3, _4, _5, _6, N, ...) N
#define __nolibc_syscall_narg(...) ___nolibc_syscall_narg(__VA_ARGS__, 6, 5,
4, 3, 2, 1, 0)
-#define __nolibc_syscall(N, ...) __nolibc_syscall##N(__VA_ARGS__)
+#define __nolibc_syscall(N, ...) __nolibc_syscall_eval##N(__VA_ARGS__)
#define __nolibc_syscall_n(N, ...) __nolibc_syscall(N, __VA_ARGS__)
#define _syscall(...) __nolibc_syscall_n(__nolibc_syscall_narg(__VA_ARGS__),
##__VA_ARGS__)
#define syscall(...) __sysret(_syscall(__VA_ARGS__))
--
Ammar Faizi
