* [PATCH] don't wrap __syscall invocation in parenthesis
@ 2015-11-06 23:51 Petr Hosek
2015-11-07 13:29 ` Szabolcs Nagy
0 siblings, 1 reply; 3+ messages in thread
From: Petr Hosek @ 2015-11-06 23:51 UTC (permalink / raw)
To: musl
[-- Attachment #1.1: Type: text/plain, Size: 209 bytes --]
This is a cleanup change, when __syscall is defined as a macro, its
expansion breaks in cases where the invocation is wrapped in parenthesis.
There are 2 such cases in the codebase and this patch fixes those.
[-- Attachment #1.2: Type: text/html, Size: 233 bytes --]
[-- Attachment #2: don-t-wrap-__syscall-invocation-in-parenthesis.patch --]
[-- Type: text/x-patch, Size: 1362 bytes --]
From 21864f367028899b541290258711750313e226f5 Mon Sep 17 00:00:00 2001
From: Petr Hosek <phosek@chromium.org>
Date: Thu, 5 Nov 2015 07:57:23 -0800
Subject: [PATCH] don't wrap __syscall invocation in parenthesis
when __syscall is defined as a macro, its expansion breaks in
cases where the invocation is wrapped in parenthesis.
---
src/fcntl/posix_fadvise.c | 2 +-
src/thread/__syscall_cp.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/fcntl/posix_fadvise.c b/src/fcntl/posix_fadvise.c
index d5360e0..fc1562e 100644
--- a/src/fcntl/posix_fadvise.c
+++ b/src/fcntl/posix_fadvise.c
@@ -4,7 +4,7 @@
int posix_fadvise(int fd, off_t base, off_t len, int advice)
{
- return -(__syscall)(SYS_fadvise, fd, __SYSCALL_LL_O(base),
+ return -__syscall(SYS_fadvise, fd, __SYSCALL_LL_O(base),
__SYSCALL_LL_E(len), advice);
}
diff --git a/src/thread/__syscall_cp.c b/src/thread/__syscall_cp.c
index faf57b1..09878c6 100644
--- a/src/thread/__syscall_cp.c
+++ b/src/thread/__syscall_cp.c
@@ -10,7 +10,7 @@ static long sccp(syscall_arg_t nr,
syscall_arg_t u, syscall_arg_t v, syscall_arg_t w,
syscall_arg_t x, syscall_arg_t y, syscall_arg_t z)
{
- return (__syscall)(nr, u, v, w, x, y, z);
+ return __syscall(nr, u, v, w, x, y, z);
}
weak_alias(sccp, __syscall_cp_c);
--
2.6.0.rc2.230.g3dd15c0
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] don't wrap __syscall invocation in parenthesis
2015-11-06 23:51 [PATCH] don't wrap __syscall invocation in parenthesis Petr Hosek
@ 2015-11-07 13:29 ` Szabolcs Nagy
2015-11-08 0:51 ` Rich Felker
0 siblings, 1 reply; 3+ messages in thread
From: Szabolcs Nagy @ 2015-11-07 13:29 UTC (permalink / raw)
To: musl
* Petr Hosek <phosek@chromium.org> [2015-11-06 23:51:21 +0000]:
> This is a cleanup change, when __syscall is defined as a macro, its
> expansion breaks in cases where the invocation is wrapped in parenthesis.
> There are 2 such cases in the codebase and this patch fixes those.
well that's the point of the parenthesis to stop macro expansion..
i think x32 breaks otherwise (because of the argument cast hacks)
> From 21864f367028899b541290258711750313e226f5 Mon Sep 17 00:00:00 2001
> From: Petr Hosek <phosek@chromium.org>
> Date: Thu, 5 Nov 2015 07:57:23 -0800
> Subject: [PATCH] don't wrap __syscall invocation in parenthesis
>
> when __syscall is defined as a macro, its expansion breaks in
> cases where the invocation is wrapped in parenthesis.
> ---
> src/fcntl/posix_fadvise.c | 2 +-
> src/thread/__syscall_cp.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/fcntl/posix_fadvise.c b/src/fcntl/posix_fadvise.c
> index d5360e0..fc1562e 100644
> --- a/src/fcntl/posix_fadvise.c
> +++ b/src/fcntl/posix_fadvise.c
> @@ -4,7 +4,7 @@
>
> int posix_fadvise(int fd, off_t base, off_t len, int advice)
> {
> - return -(__syscall)(SYS_fadvise, fd, __SYSCALL_LL_O(base),
> + return -__syscall(SYS_fadvise, fd, __SYSCALL_LL_O(base),
> __SYSCALL_LL_E(len), advice);
> }
>
> diff --git a/src/thread/__syscall_cp.c b/src/thread/__syscall_cp.c
> index faf57b1..09878c6 100644
> --- a/src/thread/__syscall_cp.c
> +++ b/src/thread/__syscall_cp.c
> @@ -10,7 +10,7 @@ static long sccp(syscall_arg_t nr,
> syscall_arg_t u, syscall_arg_t v, syscall_arg_t w,
> syscall_arg_t x, syscall_arg_t y, syscall_arg_t z)
> {
> - return (__syscall)(nr, u, v, w, x, y, z);
> + return __syscall(nr, u, v, w, x, y, z);
> }
>
> weak_alias(sccp, __syscall_cp_c);
> --
> 2.6.0.rc2.230.g3dd15c0
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] don't wrap __syscall invocation in parenthesis
2015-11-07 13:29 ` Szabolcs Nagy
@ 2015-11-08 0:51 ` Rich Felker
0 siblings, 0 replies; 3+ messages in thread
From: Rich Felker @ 2015-11-08 0:51 UTC (permalink / raw)
To: musl
On Sat, Nov 07, 2015 at 02:29:43PM +0100, Szabolcs Nagy wrote:
> * Petr Hosek <phosek@chromium.org> [2015-11-06 23:51:21 +0000]:
> > This is a cleanup change, when __syscall is defined as a macro, its
> > expansion breaks in cases where the invocation is wrapped in parenthesis.
> > There are 2 such cases in the codebase and this patch fixes those.
>
> well that's the point of the parenthesis to stop macro expansion..
>
> i think x32 breaks otherwise (because of the argument cast hacks)
No, it's actually for 32-bit archs that require even-slot alignment of
64-bit syscall arguments. At the time it was added this was just arm.
See commit 0b6eb2dfb2e84a8a51906e7634f3d5edc230b058. To get rid of
the need to call the actuall __syscall function here, we would need to
add a __syscall7 form on archs that need it. I think we should review
this anyway since __syscall (the function) on most of these archs is
probably actually lacking support for 7-argument passing, causing the
advice argument passed to the kernel to be random junk...
> > From 21864f367028899b541290258711750313e226f5 Mon Sep 17 00:00:00 2001
> > From: Petr Hosek <phosek@chromium.org>
> > Date: Thu, 5 Nov 2015 07:57:23 -0800
> > Subject: [PATCH] don't wrap __syscall invocation in parenthesis
> >
> > when __syscall is defined as a macro, its expansion breaks in
> > cases where the invocation is wrapped in parenthesis.
> > ---
> > src/fcntl/posix_fadvise.c | 2 +-
> > src/thread/__syscall_cp.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/fcntl/posix_fadvise.c b/src/fcntl/posix_fadvise.c
> > index d5360e0..fc1562e 100644
> > --- a/src/fcntl/posix_fadvise.c
> > +++ b/src/fcntl/posix_fadvise.c
> > @@ -4,7 +4,7 @@
> >
> > int posix_fadvise(int fd, off_t base, off_t len, int advice)
> > {
> > - return -(__syscall)(SYS_fadvise, fd, __SYSCALL_LL_O(base),
> > + return -__syscall(SYS_fadvise, fd, __SYSCALL_LL_O(base),
> > __SYSCALL_LL_E(len), advice);
> > }
> >
> > diff --git a/src/thread/__syscall_cp.c b/src/thread/__syscall_cp.c
> > index faf57b1..09878c6 100644
> > --- a/src/thread/__syscall_cp.c
> > +++ b/src/thread/__syscall_cp.c
> > @@ -10,7 +10,7 @@ static long sccp(syscall_arg_t nr,
> > syscall_arg_t u, syscall_arg_t v, syscall_arg_t w,
> > syscall_arg_t x, syscall_arg_t y, syscall_arg_t z)
> > {
> > - return (__syscall)(nr, u, v, w, x, y, z);
> > + return __syscall(nr, u, v, w, x, y, z);
> > }
> >
> > weak_alias(sccp, __syscall_cp_c);
> > --
In the latter case, the function call is purely an optimization; it
yields a trivial tail-call rather than actually expanding out to a
duplicate of the syscall code.
I think the NaCl port of musl should simply have a working __syscall
function provided by src/internal/[whatever]/syscall.s. (Or .c as it
may be.) This would make it uniform with other archs and would avoid
the need to remove this optimization. There may be other ways to
restructure this code that's cleaner anyway, but I'd rather keep
design changes like this separate from the actual porting.
Rich
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-11-08 0:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-06 23:51 [PATCH] don't wrap __syscall invocation in parenthesis Petr Hosek
2015-11-07 13:29 ` Szabolcs Nagy
2015-11-08 0:51 ` Rich Felker
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).