diff mbox series

[03/13] linux-user: Add faccessat2() syscall

Message ID 20220826141853.419564-4-deller@gmx.de (mailing list archive)
State New, archived
Headers show
Series linux-user: Add more syscalls, enhance tracing & logging enhancements | expand

Commit Message

Helge Deller Aug. 26, 2022, 2:18 p.m. UTC 
Add implementation and strace output for faccessat2().

Signed-off-by: Helge Deller <deller@gmx.de>
---
 linux-user/strace.c    |  2 +-
 linux-user/strace.list |  3 +++
 linux-user/syscall.c   | 12 ++++++++++++
 3 files changed, 16 insertions(+), 1 deletion(-)

--
2.37.1

Comments

Richard Henderson Aug. 26, 2022, 3:17 p.m. UTC | #1 
On 8/26/22 07:18, Helge Deller wrote:
> Add implementation and strace output for faccessat2().
> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> ---
>   linux-user/strace.c    |  2 +-
>   linux-user/strace.list |  3 +++
>   linux-user/syscall.c   | 12 ++++++++++++
>   3 files changed, 16 insertions(+), 1 deletion(-)

I've done this one, and cleaned up the code a bit too:

https://lore.kernel.org/qemu-devel/20220729201414.29869-1-richard.henderson@linaro.org/


r~

> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 27309f1106..e8c63aa4c2 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -1962,7 +1962,7 @@ print_execv(CPUArchState *cpu_env, const struct syscallname *name,
>   }
>   #endif
> 
> -#ifdef TARGET_NR_faccessat
> +#if defined(TARGET_NR_faccessat) || defined(TARGET_NR_faccessat2)
>   static void
>   print_faccessat(CPUArchState *cpu_env, const struct syscallname *name,
>                   abi_long arg0, abi_long arg1, abi_long arg2,
> diff --git a/linux-user/strace.list b/linux-user/strace.list
> index a78cdf3cdf..6e88da7fad 100644
> --- a/linux-user/strace.list
> +++ b/linux-user/strace.list
> @@ -177,6 +177,9 @@
>   #ifdef TARGET_NR_faccessat
>   { TARGET_NR_faccessat, "faccessat" , NULL, print_faccessat, NULL },
>   #endif
> +#ifdef TARGET_NR_faccessat2
> +{ TARGET_NR_faccessat2, "faccessat2" , NULL, print_faccessat, NULL },
> +#endif
>   #ifdef TARGET_NR_fadvise64
>   { TARGET_NR_fadvise64, "fadvise64" , NULL, NULL, NULL },
>   #endif
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index f409121202..f51c4fbabd 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -399,6 +399,9 @@ _syscall3(int, ioprio_set, int, which, int, who, int, ioprio)
>   #if defined(TARGET_NR_getrandom) && defined(__NR_getrandom)
>   _syscall3(int, getrandom, void *, buf, size_t, buflen, unsigned int, flags)
>   #endif
> +#if defined(TARGET_NR_faccessat2) && defined(__NR_faccessat2)
> +_syscall4(int, faccessat2, int, dirfd, char *, pathname, int, mode, int, flags)
> +#endif
> 
>   #if defined(TARGET_NR_kcmp) && defined(__NR_kcmp)
>   _syscall5(int, kcmp, pid_t, pid1, pid_t, pid2, int, type,
> @@ -9098,6 +9101,15 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
>           unlock_user(p, arg2, 0);
>           return ret;
>   #endif
> +#if defined(TARGET_NR_faccessat2) && defined(__NR_faccessat2)
> +    case TARGET_NR_faccessat2:
> +        if (!(p = lock_user_string(arg2))) {
> +            return -TARGET_EFAULT;
> +        }
> +        ret = get_errno(faccessat2(arg1, p, arg3, arg4));
> +        unlock_user(p, arg2, 0);
> +        return ret;
> +#endif
>   #ifdef TARGET_NR_nice /* not on alpha */
>       case TARGET_NR_nice:
>           return get_errno(nice(arg1));
> --
> 2.37.1
>
Helge Deller Aug. 26, 2022, 4:18 p.m. UTC | #2 
On 8/26/22 17:17, Richard Henderson wrote:
> On 8/26/22 07:18, Helge Deller wrote:
>> Add implementation and strace output for faccessat2().
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>> ---
>>   linux-user/strace.c    |  2 +-
>>   linux-user/strace.list |  3 +++
>>   linux-user/syscall.c   | 12 ++++++++++++
>>   3 files changed, 16 insertions(+), 1 deletion(-)
>
> I've done this one, and cleaned up the code a bit too:
>
> https://lore.kernel.org/qemu-devel/20220729201414.29869-1-richard.henderson@linaro.org/

Btw, your patch is missing the strace parts.

Furthermore, IMHO I think it's a bad idea to replace userspace-requested
syscalls with other (emulated) syscalls - UNLESS(!!) we have to.

I'm debugging frequenty failures with qemu-user which happen building
debian packages. For that I often compare native strace output with qemu
QEMU_STRACE=1 output. If you replace syscalls with others it makes comparing
differences harder.

And, with your patch, if the target has TARGET_NR_faccessat2, but the host
hasn't (and as such emulate it via the host libc), a configure check in the guest
would return success. I think it's better to let the guest then fall back, not
the host.

I think we should really just emulate the syscalls the host kernel provides.

Anyway, I'm still ok to drop my patch if you want.

Helge

>
>
> r~
>
>>
>> diff --git a/linux-user/strace.c b/linux-user/strace.c
>> index 27309f1106..e8c63aa4c2 100644
>> --- a/linux-user/strace.c
>> +++ b/linux-user/strace.c
>> @@ -1962,7 +1962,7 @@ print_execv(CPUArchState *cpu_env, const struct syscallname *name,
>>   }
>>   #endif
>>
>> -#ifdef TARGET_NR_faccessat
>> +#if defined(TARGET_NR_faccessat) || defined(TARGET_NR_faccessat2)
>>   static void
>>   print_faccessat(CPUArchState *cpu_env, const struct syscallname *name,
>>                   abi_long arg0, abi_long arg1, abi_long arg2,
>> diff --git a/linux-user/strace.list b/linux-user/strace.list
>> index a78cdf3cdf..6e88da7fad 100644
>> --- a/linux-user/strace.list
>> +++ b/linux-user/strace.list
>> @@ -177,6 +177,9 @@
>>   #ifdef TARGET_NR_faccessat
>>   { TARGET_NR_faccessat, "faccessat" , NULL, print_faccessat, NULL },
>>   #endif
>> +#ifdef TARGET_NR_faccessat2
>> +{ TARGET_NR_faccessat2, "faccessat2" , NULL, print_faccessat, NULL },
>> +#endif
>>   #ifdef TARGET_NR_fadvise64
>>   { TARGET_NR_fadvise64, "fadvise64" , NULL, NULL, NULL },
>>   #endif
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index f409121202..f51c4fbabd 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -399,6 +399,9 @@ _syscall3(int, ioprio_set, int, which, int, who, int, ioprio)
>>   #if defined(TARGET_NR_getrandom) && defined(__NR_getrandom)
>>   _syscall3(int, getrandom, void *, buf, size_t, buflen, unsigned int, flags)
>>   #endif
>> +#if defined(TARGET_NR_faccessat2) && defined(__NR_faccessat2)
>> +_syscall4(int, faccessat2, int, dirfd, char *, pathname, int, mode, int, flags)
>> +#endif
>>
>>   #if defined(TARGET_NR_kcmp) && defined(__NR_kcmp)
>>   _syscall5(int, kcmp, pid_t, pid1, pid_t, pid2, int, type,
>> @@ -9098,6 +9101,15 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
>>           unlock_user(p, arg2, 0);
>>           return ret;
>>   #endif
>> +#if defined(TARGET_NR_faccessat2) && defined(__NR_faccessat2)
>> +    case TARGET_NR_faccessat2:
>> +        if (!(p = lock_user_string(arg2))) {
>> +            return -TARGET_EFAULT;
>> +        }
>> +        ret = get_errno(faccessat2(arg1, p, arg3, arg4));
>> +        unlock_user(p, arg2, 0);
>> +        return ret;
>> +#endif
>>   #ifdef TARGET_NR_nice /* not on alpha */
>>       case TARGET_NR_nice:
>>           return get_errno(nice(arg1));
>> --
>> 2.37.1
>>
>
diff mbox series

Patch

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 27309f1106..e8c63aa4c2 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1962,7 +1962,7 @@  print_execv(CPUArchState *cpu_env, const struct syscallname *name,
 }
 #endif

-#ifdef TARGET_NR_faccessat
+#if defined(TARGET_NR_faccessat) || defined(TARGET_NR_faccessat2)
 static void
 print_faccessat(CPUArchState *cpu_env, const struct syscallname *name,
                 abi_long arg0, abi_long arg1, abi_long arg2,
diff --git a/linux-user/strace.list b/linux-user/strace.list
index a78cdf3cdf..6e88da7fad 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -177,6 +177,9 @@ 
 #ifdef TARGET_NR_faccessat
 { TARGET_NR_faccessat, "faccessat" , NULL, print_faccessat, NULL },
 #endif
+#ifdef TARGET_NR_faccessat2
+{ TARGET_NR_faccessat2, "faccessat2" , NULL, print_faccessat, NULL },
+#endif
 #ifdef TARGET_NR_fadvise64
 { TARGET_NR_fadvise64, "fadvise64" , NULL, NULL, NULL },
 #endif
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f409121202..f51c4fbabd 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -399,6 +399,9 @@  _syscall3(int, ioprio_set, int, which, int, who, int, ioprio)
 #if defined(TARGET_NR_getrandom) && defined(__NR_getrandom)
 _syscall3(int, getrandom, void *, buf, size_t, buflen, unsigned int, flags)
 #endif
+#if defined(TARGET_NR_faccessat2) && defined(__NR_faccessat2)
+_syscall4(int, faccessat2, int, dirfd, char *, pathname, int, mode, int, flags)
+#endif

 #if defined(TARGET_NR_kcmp) && defined(__NR_kcmp)
 _syscall5(int, kcmp, pid_t, pid1, pid_t, pid2, int, type,
@@ -9098,6 +9101,15 @@  static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
         unlock_user(p, arg2, 0);
         return ret;
 #endif
+#if defined(TARGET_NR_faccessat2) && defined(__NR_faccessat2)
+    case TARGET_NR_faccessat2:
+        if (!(p = lock_user_string(arg2))) {
+            return -TARGET_EFAULT;
+        }
+        ret = get_errno(faccessat2(arg1, p, arg3, arg4));
+        unlock_user(p, arg2, 0);
+        return ret;
+#endif
 #ifdef TARGET_NR_nice /* not on alpha */
     case TARGET_NR_nice:
         return get_errno(nice(arg1));