Re: [UNIKRAFT PATCH v2 2/3] lib/syscall_shim: handler: Take only required number of arguments

[Gaulthier Gain <Gaulthier.Gain@xxxxxxxxx>]

Reviewed-by: Gaulthier Gain <gaulthier.gain@xxxxxxxxx>

> On 21 Apr 2020, at 17:36, Simon Kuenzer <simon.kuenzer@xxxxxxxxx> wrote:
> 
> Improves the `uk_syscall()`,  `uk_syscall_r()`, `uk_vsyscall()`, and
> `uk_vsyscall_r()` implementations in a way that only the number of
> required arguments are taken from the stack.
> 
> Signed-off-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> ---
> lib/syscall_shim/gen_uk_syscall.awk    | 16 ++++--------
> lib/syscall_shim/gen_uk_syscall_r.awk  | 16 ++++--------
> lib/syscall_shim/include/uk/syscall.h  |  8 +++---
> lib/syscall_shim/uk_syscall.c.in_end   | 28 +++++----------------
> lib/syscall_shim/uk_syscall_r.c.in_end | 34 +++++++-------------------
> 5 files changed, 29 insertions(+), 73 deletions(-)
> 
> diff --git a/lib/syscall_shim/gen_uk_syscall.awk 
> b/lib/syscall_shim/gen_uk_syscall.awk
> index 2f2e1fa9..70c4df47 100644
> --- a/lib/syscall_shim/gen_uk_syscall.awk
> +++ b/lib/syscall_shim/gen_uk_syscall.awk
> @@ -5,15 +5,9 @@ BEGIN {
>       print "#include <uk/syscall.h>"
>       print "#include <uk/print.h>\n"
> 
> -     printf "static inline long __uk_syscall(long nr, "
> -     for (i = 1; i < max_args; i++)
> -             printf "long arg%d, ",i
> -     printf "long arg%d)\n{\n", max_args
> -
> -     for (i = 1; i <= max_args; i++)
> -             printf "\t(void) arg%d;\n", i
> -
> -     print "\n\tswitch (nr) {"
> +     print "long uk_vsyscall(long nr, va_list arg)\n{"
> +     print "\t(void) arg;\n"
> +     print "\tswitch (nr) {"
> }
> 
> 
> @@ -24,9 +18,9 @@ BEGIN {
>       printf "\tcase %s:\n", sys_name;
>       printf "\t\treturn uk_syscall_e_%s(", name;
>       for (i = 1; i < args_nr; i++)
> -             printf("arg%d, ", i)
> +             printf("va_arg(arg, long), ")
>       if (args_nr > 0)
> -             printf("arg%d", args_nr)
> +             printf("va_arg(arg, long)")
>       printf(");\n")
> }
> 
> diff --git a/lib/syscall_shim/gen_uk_syscall_r.awk 
> b/lib/syscall_shim/gen_uk_syscall_r.awk
> index c591fc47..74d13241 100644
> --- a/lib/syscall_shim/gen_uk_syscall_r.awk
> +++ b/lib/syscall_shim/gen_uk_syscall_r.awk
> @@ -5,15 +5,9 @@ BEGIN {
>       print "#include <uk/syscall.h>"
>       print "#include <uk/print.h>\n"
> 
> -     printf "static inline long __uk_syscall_r(long nr, "
> -     for (i = 1; i < max_args; i++)
> -             printf "long arg%d, ",i
> -     printf "long arg%d)\n{\n", max_args
> -
> -     for (i = 1; i <= max_args; i++)
> -             printf "\t(void) arg%d;\n", i
> -
> -     print "\n\tswitch (nr) {"
> +     print "long uk_vsyscall_r(long nr, va_list arg)\n{"
> +     print "\t(void) arg;\n"
> +     print "\tswitch (nr) {"
> }
> 
> 
> @@ -25,9 +19,9 @@ BEGIN {
>       printf "\tcase %s:\n", sys_name;
>       printf "\t\treturn %s(", uk_syscall_r;
>       for (i = 1; i < args_nr; i++)
> -             printf("arg%d, ", i)
> +             printf("va_arg(arg, long), ")
>       if (args_nr > 0)
> -             printf("arg%d", args_nr)
> +             printf("va_arg(arg, long)")
>       printf(");\n")
> }
> 
> diff --git a/lib/syscall_shim/include/uk/syscall.h 
> b/lib/syscall_shim/include/uk/syscall.h
> index 078129e5..3d2e4e89 100644
> --- a/lib/syscall_shim/include/uk/syscall.h
> +++ b/lib/syscall_shim/include/uk/syscall.h
> @@ -288,8 +288,8 @@ typedef long uk_syscall_arg_t;
> #include <uk/bits/syscall_stubs.h>
> 
> /* System call, returns -1 and sets errno on errors */
> -long uk_syscall(long n, ...);
> -long uk_vsyscall(long n, va_list arg);
> +long uk_syscall(long nr, ...);
> +long uk_vsyscall(long nr, va_list arg);
> 
> /*
>  * Use this variant instead of `uk_syscall()` whenever the system call number
> @@ -300,8 +300,8 @@ long uk_vsyscall(long n, va_list arg);
>       UK_CONCAT(__uk_syscall, __UK_SYSCALL_NARGS(__VA_ARGS__))(__VA_ARGS__)
> 
> /* Raw system call, returns negative codes on errors */
> -long uk_syscall_r(long n, ...);
> -long uk_vsyscall_r(long n, va_list arg);
> +long uk_syscall_r(long nr, ...);
> +long uk_vsyscall_r(long nr, va_list arg);
> 
> /*
>  * Use this variant instead of `uk_syscall_r()` whenever the system call 
> number
> diff --git a/lib/syscall_shim/uk_syscall.c.in_end 
> b/lib/syscall_shim/uk_syscall.c.in_end
> index 8a5f8e0b..73e74dd0 100644
> --- a/lib/syscall_shim/uk_syscall.c.in_end
> +++ b/lib/syscall_shim/uk_syscall.c.in_end
> @@ -1,27 +1,11 @@
> 
> -long uk_syscall(long n, ...)
> +long uk_syscall(long nr, ...)
> {
> +     long ret;
>       va_list ap;
> -     long a,b,c,d,e,f;
> -     va_start(ap, n);
> -     a=va_arg(ap, long);
> -     b=va_arg(ap, long);
> -     c=va_arg(ap, long);
> -     d=va_arg(ap, long);
> -     e=va_arg(ap, long);
> -     f=va_arg(ap, long);
> -     va_end(ap);
> -     return __uk_syscall(n,a,b,c,d,e,f);
> -}
> 
> -long uk_vsyscall(long n, va_list arg)
> -{
> -     long a,b,c,d,e,f;
> -     a=va_arg(arg, long);
> -     b=va_arg(arg, long);
> -     c=va_arg(arg, long);
> -     d=va_arg(arg, long);
> -     e=va_arg(arg, long);
> -     f=va_arg(arg, long);
> -     return __uk_syscall(n,a,b,c,d,e,f);
> +     va_start(ap, nr);
> +     ret = uk_vsyscall(nr, ap);
> +     va_end(ap);
> +     return ret;
> }
> diff --git a/lib/syscall_shim/uk_syscall_r.c.in_end 
> b/lib/syscall_shim/uk_syscall_r.c.in_end
> index ae74716e..73ff755c 100644
> --- a/lib/syscall_shim/uk_syscall_r.c.in_end
> +++ b/lib/syscall_shim/uk_syscall_r.c.in_end
> @@ -1,29 +1,13 @@
> 
> -long uk_syscall_r(long n, ...)
> +long uk_syscall_r(long nr, ...)
> {
> +     long ret;
>       va_list ap;
> -     long a,b,c,d,e,f;
> -     va_start(ap, n);
> -     a=va_arg(ap, long);
> -     b=va_arg(ap, long);
> -     c=va_arg(ap, long);
> -     d=va_arg(ap, long);
> -     e=va_arg(ap, long);
> -     f=va_arg(ap, long);
> -     va_end(ap);
> -     return __uk_syscall_r(n,a,b,c,d,e,f);
> -}
> 
> -long uk_vsyscall_r(long n, va_list arg)
> -{
> -     long a,b,c,d,e,f;
> -     a=va_arg(arg, long);
> -     b=va_arg(arg, long);
> -     c=va_arg(arg, long);
> -     d=va_arg(arg, long);
> -     e=va_arg(arg, long);
> -     f=va_arg(arg, long);
> -     return __uk_syscall_r(n,a,b,c,d,e,f);
> +     va_start(ap, nr);
> +     ret = uk_vsyscall_r(nr, ap);
> +     va_end(ap);
> +     return ret;
> }
> 
> #if CONFIG_LIBSYSCALL_SHIM_HANDLER
> @@ -38,8 +22,8 @@ void ukplat_syscall_handler(struct __regs *r)
>       uk_pr_debug("Binary system call request \"%s\" (%lu) at ip:%p 
> (arg0=0x%lx, arg1=0x%lx, ...)\n",
>                   uk_syscall_name(r->rsyscall), r->rsyscall,
>                   (void *) r->rip, r->rarg0, r->rarg1);
> -     r->rret0 = __uk_syscall_r(r->rsyscall,
> -                               r->rarg0, r->rarg1, r->rarg2,
> -                               r->rarg3, r->rarg4, r->rarg5);
> +     r->rret0 = uk_syscall_r(r->rsyscall,
> +                             r->rarg0, r->rarg1, r->rarg2,
> +                             r->rarg3, r->rarg4, r->rarg5);
> }
> #endif /* CONFIG_LIBSYSCALL_SHIM_HANDLER */
> -- 
> 2.20.1
> 
>