[1/4] Add INTERNAL_SYSCALL_CALL
Commit Message
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:
> 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?
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?
* Adhemerval Zanella:
> 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
Great, thanks.
> do not change. I presume it is ok to push upstream, correct?
I can't really tell you that the patch is totally unproblematic, but
all my concerns have been addressed. Please push.
@@ -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;