diff mbox

[1/4] Add INTERNAL_SYSCALL_CALL

Message ID c2114f75-0e23-0636-32ed-0e879337a27d@linaro.org
State New
Headers show

Commit Message

Adhemerval Zanella Netto Sept. 22, 2016, 1:42 p.m. UTC 
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.

--

Comments

Adhemerval Zanella Netto Sept. 23, 2016, 2:16 p.m. UTC | #1 
On 22/09/2016 17:34, Florian Weimer wrote:
> * Adhemerval Zanella:

> 

>> 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.

> 

> Okay, it looks like this is the better way for now.

> 

>> 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.

> 

> Did you see object code changes from that?


I haven't checked all of the, but at least x86_64, i386, aarch64, and powerpc64le
do not change.  I presume it is ok to push upstream, correct?
diff mbox

Patch

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;