[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v5 04/17] common-user: Move syscall error detection into safe

From: Richard Henderson
Subject: Re: [PATCH v5 04/17] common-user: Move syscall error detection into safe_syscall_base
Date: Mon, 22 Nov 2021 13:21:07 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0

On 11/22/21 12:55 PM, Peter Maydell wrote:
-       /* This is the entry point for making a system call. The calling
+       /*
+        * This is the entry point for making a system call. The calling
          * convention here is that of a C varargs function with the
          * first argument an 'int *' to the signal_pending flag, the
          * second one the system call number (as a 'long'), and all further
          * arguments being syscall arguments (also 'long').

This comment text needs updating to mention the new errnop argument.
(Applies to all the similar comments in the files for the other archs.)


+       /*
+         * The syscall calling convention isn't the same as the C one:

Looks like the indent here is wrong ?

Irritatingly, these files are a mix of tabs/spaces.

          * we enter with x0 == *signal_pending
-        *               x1 == syscall number
-        *               x2 ... x7, (stack) == syscall arguments
+        *               x1 == errno

"int* address of errno"

Arg, fixed some of these, but clearly. not all.

         /* code path for having successfully executed the syscall */
+       cmn     x0, #4095
+       b.cs    1f

Shouldn't this be going to label 0f ? We need to do the 'neg',
and unless I'm misreading the diff there's currently no path
of execution that gets to that.

Oops, rebase error, where the fix landed in the next patch.

+        *               r2 == syscall number
+        *               r3, [sp+0] ... [sp+16] == syscall arguments
          *               and return the result in r0

Don't we wind up with a potential issue here with 64-bit arguments
due to the calling convention wanting to put those in aligned
memory/register locations? Previously because we had just two
extra arguments the arguments started at r2 and had the same
alignment behaviour as the syscall wants for them starting at
r0; but now we start at r3 so if for instance the first argument
is 64-bit it will be in [sp+0][sp+4] but should go in r0:r1
I think...

(Stopped reviewing here because if we need to change the
way we call these functions there's no point my reviewing
the fine detail of the asm.)

Oof. I missed that detail. Yes, that is a problem (I think arm is the only such supported host). I think the best solution would be to *not* pass in &errno, but to have the assembly tail-call to

long safe_syscall_errno_tail(int value)
    errno = value;
    return -1;

Which is probably more efficient in any case.  I'll re-work this.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]