From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/7352 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] MIPS big endian and *stat syscalls return value Date: Tue, 7 Apr 2015 10:34:15 -0400 Message-ID: <20150407143415.GI6817@brightrain.aerifal.cx> References: <5523A299.5050209@ndmsystems.com> <20150407115821.GA31681@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 1428417273 9642 80.91.229.3 (7 Apr 2015 14:34:33 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Tue, 7 Apr 2015 14:34:33 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-7365-gllmg-musl=m.gmane.org@lists.openwall.com Tue Apr 07 16:34:32 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 1YfUaE-0004Wx-Uk for gllmg-musl@m.gmane.org; Tue, 07 Apr 2015 16:34:31 +0200 Original-Received: (qmail 21525 invoked by uid 550); 7 Apr 2015 14:34:28 -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 20478 invoked from network); 7 Apr 2015 14:34:28 -0000 Content-Disposition: inline In-Reply-To: <20150407115821.GA31681@port70.net> User-Agent: Mutt/1.5.21 (2010-09-15) Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:7352 Archived-At: On Tue, Apr 07, 2015 at 01:58:21PM +0200, Szabolcs Nagy wrote: > * Eugene [2015-04-07 12:25:45 +0300]: > > > > The reason of the problem is that the function __stat_fix rewrites register > > $v0, which is not stored in the parent function. > > > > ouch Yes ouch, but I've never seen this trigger in practice. With the 'new' interpretation of __asm__("regname") that clang uses, where it only affects constraints for __asm__ blocks, such an issue should not be possible. With the old interpretation old (and maybe current?) gcc uses, where the variable 'permanently lives' in the register, it certainly could break, but I've never observed such breakage. I'm guessing you need -O0 to trigger it, no? > > static inline long __syscall2(long n, long a, long b) > > { > > + long t; > > register long r4 __asm__("$4") = a; > > register long r5 __asm__("$5") = b; > > register long r7 __asm__("$7"); > > register long r2 __asm__("$2"); > > + > > __asm__ __volatile__ ( > > "addu $2,$0,%2 ; syscall" > > : "=&r"(r2), "=r"(r7) : "ir"(n), "0"(r2), "1"(r7), > > "r"(r4), "r"(r5) > > : "$1", "$3", "$8", "$9", "$10", "$11", "$12", "$13", > > "$14", "$15", "$24", "$25", "hi", "lo", "memory"); > > + > > if (r7) return -r2; > > + t = r2; > > if (n == SYS_stat64 || n == SYS_fstat64 || n == SYS_lstat64) __stat_fix(b); > > - return r2; > > + > > + return t; > > } > > > > looks ok to me > (other than the newline changes) Yeah. I would also slightly prefer a name like 'ret' to 't' but these are purely cosmetic issues. The functional content of the patch looks fully valid. > i wonder if __stat_fix could be inlined in a way that the > compiler knows it shouldnt clobber r2. In practice it always is, at least for me. But relying on that for semantic purposes is not valid. > same for __syscall3 and __syscall4 Yes. Rich