Re: [PATCH 1/4] Add INTERNAL_SYSCALL_CALL

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 21/09/2016 16:22, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 20/09/2016 18:36, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> +#define __INTERNAL_SYSCALL0(name, err) \
>>>> +  INTERNAL_SYSCALL (name, err, 0)
>>>> +#define __INTERNAL_SYSCALL1(name, err, a1) \
>>>> +  INTERNAL_SYSCALL (name, err, 1, a1)
>>>> +#define __INTERNAL_SYSCALL2(name, err, a1, a2) \
>>>> +  INTERNAL_SYSCALL (name, err, 2, a1, a2)
>>>> +#define __INTERNAL_SYSCALL3(name, err, a1, a2, a3) \
>>>> +  INTERNAL_SYSCALL (name, err, 3, a1, a2, a3)
>>>> +#define __INTERNAL_SYSCALL4(name, err, a1, a2, a3, a4) \
>>>> +  INTERNAL_SYSCALL (name, err, 4, a1, a2, a3, a4)
>>>> +#define __INTERNAL_SYSCALL5(name, err, a1, a2, a3, a4, a5) \
>>>> +  INTERNAL_SYSCALL (name, err, 5, a1, a2, a3, a4, a5)
>>>> +#define __INTERNAL_SYSCALL6(name, err, a1, a2, a3, a4, a5, a6) \
>>>> +  INTERNAL_SYSCALL (name, err, 6, a1, a2, a3, a4, a5, a6)
>>>> +#define __INTERNAL_SYSCALL7(name, err, a1, a2, a3, a4, a5, a6, a7) \
>>>> +  INTERNAL_SYSCALL (name, err, 7, a1, a2, a3, a4, a5, a6, a7)
>>>
>>> It's not immediately obvious why these definitions are needed.
>>
>> I agree this is not obvious, but it follows the SYSCALL_CANCEL macro logic
>> where __INTERNAL_SYSCALL_DISP will select the correct __INTERNAL_SYSCALL
>> (based on number of arguments).
> 
> Is there anything that overrides inidivdual __INTERNAL_SYSCALLx
> macros?
> 
> What I mean is this:  Why can't this
> 
> +#define __INTERNAL_SYSCALL_DISP(b,err,...) \
> +  __INTERNAL_SYSCALL_CONCAT (b, __SYSCALL_NARGS (__VA_ARGS__)) \
> +			    (err, __VA_ARGS__)
> 
> turn into
> 
> +#define __INTERNAL_SYSCALL_DISP(b,err,...) \
> +  INTERNAL_SYSCALL (b, err, __SYSCALL_NARGS (__VA_ARGS__), __VA_ARGS__)
> 
> ?
> 

We can, at least for x86_64 for instance where it uses another indirection
for INTERNAL_SYSCALL.  However, something similar fails for i386, where
macro substitution for INTERNAL_SYSCALL will try string concatenation and
thus mess with intended behaviour.  Also, _SYSCALL_NARGS macro would be
required to be different to take in consideration the 'err' argument
required for INTERNAL syscall (something I noted I coded wrong).

I think calling the {INLINE,INTERNAL}_SYSCALL directly would be the safer 
and agnostic approach to avoid issues on how they are actually implemented 
by each port.

I tested the following patch with a build for practically all current
supported ports (aarch64, alpha, armeabi, armeaihf, hppa, ia64, i386, m68k,
microblaze, mips{32,64,n64}, nios2, powerpc{32,64,64le}, s390{-32,-64}, sh4,
sparc{64}, tile{pro,x64}, x86_64, and x32) and saw no build issues.  I also
checked on x86_64 and i386.  To actually check INTERNAL_SYSCALL_CALL macro
work I changed sysdeps/unix/sysv/linux/pthread_setaffinity.c to use it.

For below patch I changed it to the INLINE_SYSCALL macros would follow 
INLINE name and fixed the __INTERNAL_SYSCALL_NARGS argument selection
value.

--

diff --git a/sysdeps/unix/sysdep.h b/sysdeps/unix/sysdep.h
index 94a2ce0..dfd3cfd 100644
--- a/sysdeps/unix/sysdep.h
+++ b/sysdeps/unix/sysdep.h
@@ -24,42 +24,79 @@
 #define	SYSCALL__(name, args)	PSEUDO (__##name, name, args)
 #define	SYSCALL(name, args)	PSEUDO (name, name, args)
 
-#define __SYSCALL0(name) \
+#define __SYSCALL_CONCAT_X(a,b)     a##b
+#define __SYSCALL_CONCAT(a,b)       __SYSCALL_CONCAT_X (a, b)
+
+
+#define __INTERNAL_SYSCALL0(name, err) \
+  INTERNAL_SYSCALL (name, err, 0)
+#define __INTERNAL_SYSCALL1(name, err, a1) \
+  INTERNAL_SYSCALL (name, err, 1, a1)
+#define __INTERNAL_SYSCALL2(name, err, a1, a2) \
+  INTERNAL_SYSCALL (name, err, 2, a1, a2)
+#define __INTERNAL_SYSCALL3(name, err, a1, a2, a3) \
+  INTERNAL_SYSCALL (name, err, 3, a1, a2, a3)
+#define __INTERNAL_SYSCALL4(name, err, a1, a2, a3, a4) \
+  INTERNAL_SYSCALL (name, err, 4, a1, a2, a3, a4)
+#define __INTERNAL_SYSCALL5(name, err, a1, a2, a3, a4, a5) \
+  INTERNAL_SYSCALL (name, err, 5, a1, a2, a3, a4, a5)
+#define __INTERNAL_SYSCALL6(name, err, a1, a2, a3, a4, a5, a6) \
+  INTERNAL_SYSCALL (name, err, 6, a1, a2, a3, a4, a5, a6)
+#define __INTERNAL_SYSCALL7(name, err, a1, a2, a3, a4, a5, a6, a7) \
+  INTERNAL_SYSCALL (name, err, 7, a1, a2, a3, a4, a5, a6, a7)
+
+#define __INTERNAL_SYSCALL_NARGS_X(a,b,c,d,e,f,g,h,n,...) n
+#define __INTERNAL_SYSCALL_NARGS(...) \
+  __INTERNAL_SYSCALL_NARGS_X (__VA_ARGS__,6,5,4,3,2,1,0,)
+#define __INTERNAL_SYSCALL_DISP(b,...) \
+  __SYSCALL_CONCAT (b,__INTERNAL_SYSCALL_NARGS(__VA_ARGS__))(__VA_ARGS__)
+
+/* Issue a syscall defined by syscall number plus any other argument required.
+   It is similar to INTERNAL_SYSCALL macro, but without the need to pass the
+   expected argument number as second parameter.  */
+#define INTERNAL_SYSCALL_CALL(...) \
+  __INTERNAL_SYSCALL_DISP (__INTERNAL_SYSCALL, __VA_ARGS__)
+
+#define __INLINE_SYSCALL0(name) \
   INLINE_SYSCALL (name, 0)
-#define __SYSCALL1(name, a1) \
+#define __INLINE_SYSCALL1(name, a1) \
   INLINE_SYSCALL (name, 1, a1)
-#define __SYSCALL2(name, a1, a2) \
+#define __INLINE_SYSCALL2(name, a1, a2) \
   INLINE_SYSCALL (name, 2, a1, a2)
-#define __SYSCALL3(name, a1, a2, a3) \
+#define __INLINE_SYSCALL3(name, a1, a2, a3) \
   INLINE_SYSCALL (name, 3, a1, a2, a3)
-#define __SYSCALL4(name, a1, a2, a3, a4) \
+#define __INLINE_SYSCALL4(name, a1, a2, a3, a4) \
   INLINE_SYSCALL (name, 4, a1, a2, a3, a4)
-#define __SYSCALL5(name, a1, a2, a3, a4, a5) \
+#define __INLINE_SYSCALL5(name, a1, a2, a3, a4, a5) \
   INLINE_SYSCALL (name, 5, a1, a2, a3, a4, a5)
-#define __SYSCALL6(name, a1, a2, a3, a4, a5, a6) \
+#define __INLINE_SYSCALL6(name, a1, a2, a3, a4, a5, a6) \
   INLINE_SYSCALL (name, 6, a1, a2, a3, a4, a5, a6)
-#define __SYSCALL7(name, a1, a2, a3, a4, a5, a6, a7) \
+#define __INLINE_SYSCALL7(name, a1, a2, a3, a4, a5, a6, a7) \
   INLINE_SYSCALL (name, 7, a1, a2, a3, a4, a5, a6, a7)
 
-#define __SYSCALL_NARGS_X(a,b,c,d,e,f,g,h,n,...) n
-#define __SYSCALL_NARGS(...) \
-  __SYSCALL_NARGS_X (__VA_ARGS__,7,6,5,4,3,2,1,0,)
-#define __SYSCALL_CONCAT_X(a,b)     a##b
-#define __SYSCALL_CONCAT(a,b)       __SYSCALL_CONCAT_X (a, b)
-#define __SYSCALL_DISP(b,...) \
-  __SYSCALL_CONCAT (b,__SYSCALL_NARGS(__VA_ARGS__))(__VA_ARGS__)
+#define __INLINE_SYSCALL_NARGS_X(a,b,c,d,e,f,g,h,n,...) n
+#define __INLINE_SYSCALL_NARGS(...) \
+  __INLINE_SYSCALL_NARGS_X (__VA_ARGS__,7,6,5,4,3,2,1,0,)
+#define __INLINE_SYSCALL_DISP(b,...) \
+  __SYSCALL_CONCAT (b,__INLINE_SYSCALL_NARGS(__VA_ARGS__))(__VA_ARGS__)
 
-#define __SYSCALL_CALL(...) __SYSCALL_DISP (__SYSCALL, __VA_ARGS__)
+/* Issue a syscall defined by syscall number plus any other argument
+   required.  Any error will be handled using arch defined macros and errno
+   will be set accordingly.
+   It is similar to INLINE_SYSCALL macro, but without the need to pass the
+   expected argument number as second parameter.  */
+#define INLINE_SYSCALL_CALL(...) \
+  __INLINE_SYSCALL_DISP (__INLINE_SYSCALL, __VA_ARGS__)
 
 #define SYSCALL_CANCEL(...) \
   ({									     \
     long int sc_ret;							     \
     if (SINGLE_THREAD_P) 						     \
-      sc_ret = __SYSCALL_CALL (__VA_ARGS__);   				     \
+      sc_ret = INLINE_SYSCALL_CALL (__VA_ARGS__); 			     \
     else								     \
       {									     \
 	int sc_cancel_oldtype = LIBC_CANCEL_ASYNC ();			     \
-	sc_ret = __SYSCALL_CALL (__VA_ARGS__);				     \
+	sc_ret = INLINE_SYSCALL_CALL (__VA_ARGS__);			     \
         LIBC_CANCEL_RESET (sc_cancel_oldtype);				     \
       }									     \
     sc_ret;