From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/8821 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] don't wrap __syscall invocation in parenthesis Date: Sat, 7 Nov 2015 19:51:32 -0500 Message-ID: <20151108005132.GF3818@brightrain.aerifal.cx> References: <20151107132943.GE8500@port70.net> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1446943914 24853 80.91.229.3 (8 Nov 2015 00:51:54 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 8 Nov 2015 00:51:54 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-8834-gllmg-musl=m.gmane.org@lists.openwall.com Sun Nov 08 01:51:49 2015 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1ZvECx-0002Oq-N2 for gllmg-musl@m.gmane.org; Sun, 08 Nov 2015 01:51:47 +0100 Original-Received: (qmail 13493 invoked by uid 550); 8 Nov 2015 00:51:45 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 13471 invoked from network); 8 Nov 2015 00:51:44 -0000 Content-Disposition: inline In-Reply-To: <20151107132943.GE8500@port70.net> User-Agent: Mutt/1.5.21 (2010-09-15) Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:8821 Archived-At: On Sat, Nov 07, 2015 at 02:29:43PM +0100, Szabolcs Nagy wrote: > * Petr Hosek [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 > > 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