Re: [PATCH] MIPS big endian and *stat syscalls return value

[Rich Felker <dalias@libc.org>]

On Tue, Apr 07, 2015 at 01:58:21PM +0200, Szabolcs Nagy wrote:
> * Eugene <e.yudin@ndmsystems.com> [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