__MIPSEL__ in sys32_rt_sigtimedwait

__MIPSEL__ in sys32_rt_sigtimedwait

[Pavel Kiryukhin]

Hi all,
my question - does endiannes matters in sigset translation in 
sys32_rt_sigtimedwait (arch/mips/signal32.c)?

===================
@@ -827,18 +827,10 @@
         return -EFAULT;
 
     switch (_NSIG_WORDS) {
-#ifdef __MIPSEB__
     case 4: these.sig[3] = these32.sig[6] | (((long)these32.sig[7]) << 32);
     case 3: these.sig[2] = these32.sig[4] | (((long)these32.sig[5]) << 32);
     case 2: these.sig[1] = these32.sig[2] | (((long)these32.sig[3]) << 32);
     case 1: these.sig[0] = these32.sig[0] | (((long)these32.sig[1]) << 32);
-#endif
-#ifdef __MIPSEL__
-    case 4: these.sig[3] = these32.sig[7] | (((long)these32.sig[6]) << 32);
-    case 3: these.sig[2] = these32.sig[5] | (((long)these32.sig[4]) << 32);
-    case 2: these.sig[1] = these32.sig[3] | (((long)these32.sig[2]) << 32);
-    case 1: these.sig[0] = these32.sig[1] | (((long)these32.sig[0]) << 32);
-#endif
     }
 
     /*
===================
Regards,
Pavel Kiryukhin

Re: __MIPSEL__ in sys32_rt_sigtimedwait

[Ralf Baechle]

On Tue, Jan 20, 2004 at 08:42:15PM +0300, Pavel Kiryukhin wrote:

> Hi all,
> my question - does endiannes matters in sigset translation in 
> sys32_rt_sigtimedwait (arch/mips/signal32.c)?

Think about where bit 33 ends for a big endian machine with an without
the conversion.

  Ralf

Re: __MIPSEL__ in sys32_rt_sigtimedwait

[Daniel Jacobowitz]

On Tue, Jan 20, 2004 at 07:31:57PM +0100, Ralf Baechle wrote:
> On Tue, Jan 20, 2004 at 08:42:15PM +0300, Pavel Kiryukhin wrote:
> 
> > Hi all,
> > my question - does endiannes matters in sigset translation in 
> > sys32_rt_sigtimedwait (arch/mips/signal32.c)?
> 
> Think about where bit 33 ends for a big endian machine with an without
> the conversion.

No, I'm pretty sure Pavel's right.

-#ifdef __MIPSEB__
    case 1: these.sig[0] = these32.sig[0] | (((long)these32.sig[1]) << 32);
-#endif
-#ifdef __MIPSEL__
-    case 1: these.sig[0] = these32.sig[1] | (((long)these32.sig[0]) << 32);
-#endif

Consider a 64-bit sigset.  32-bit userland, 64-bit kernel.  Here's a
userland sigset with signal 33 set, only, on a little endian target.
Word 1, least significant bit, right?

byte address in memory
	1	2	3	4	5	6	7	8
val	0	0	0	0	0	0	0	1

Obviously, as a 64-bit integer the sigset looks different.  There it's
supposed to be 1 << (33 - 1).
val	0	0	0	1	0	0	0	0

So the correct algorithm to convert a userspace sigset to a kernel
sigset is to shift the second word left 32 bits, and leave the first
word right aligned, and or them together.  Which is what using the
__MIPSEB__ case does.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: __MIPSEL__ in sys32_rt_sigtimedwait

[Maciej W. Rozycki]

On Tue, 20 Jan 2004, Daniel Jacobowitz wrote:

> No, I'm pretty sure Pavel's right.
> 
> -#ifdef __MIPSEB__
>     case 1: these.sig[0] = these32.sig[0] | (((long)these32.sig[1]) << 32);
> -#endif
> -#ifdef __MIPSEL__
> -    case 1: these.sig[0] = these32.sig[1] | (((long)these32.sig[0]) << 32);
> -#endif
> 
> Consider a 64-bit sigset.  32-bit userland, 64-bit kernel.  Here's a
> userland sigset with signal 33 set, only, on a little endian target.
> Word 1, least significant bit, right?

 Right, but...

> byte address in memory
> 	1	2	3	4	5	6	7	8
> val	0	0	0	0	0	0	0	1

... this is incorrect -- it would be right for big-endian; word #1, bit #1
for little-endian is:

byte address in memory
	1	2	3	4	5	6	7	8
val	0	0	0	0	1	0	0	0


> Obviously, as a 64-bit integer the sigset looks different.  There it's
> supposed to be 1 << (33 - 1).
> val	0	0	0	1	0	0	0	0

 Again, for little-endian it should actually be:

val	0	0	0	0	1	0	0	0

i.e. the whole operation is actually a no-op, except that the 64-bit
vector is assured to be properly aligned for doubleword accesses.

 As a side note -- that's the reason certain C code portability problems
related to the width of the machine word only get actually discovered when
problematic software is run on a big-endian processor.  I've been hit by
this property once -- I was porting a 16-bit program and it appeared to
run just fine on both a 32-bit (i386) and a 64-bit (Alpha) little-endian
CPU, but when run on a 32-bit big-endian one (SPARC) I discovered a few
more bits to be cleaned up.

> So the correct algorithm to convert a userspace sigset to a kernel
> sigset is to shift the second word left 32 bits, and leave the first
> word right aligned, and or them together.  Which is what using the
> __MIPSEB__ case does.

 But this conclusion is of course right.

  Maciej

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: __MIPSEL__ in sys32_rt_sigtimedwait

[Daniel Jacobowitz]

On Wed, Jan 21, 2004 at 02:47:17PM +0100, Maciej W. Rozycki wrote:
> On Tue, 20 Jan 2004, Daniel Jacobowitz wrote:
> 
> > No, I'm pretty sure Pavel's right.
> > 
> > -#ifdef __MIPSEB__
> >     case 1: these.sig[0] = these32.sig[0] | (((long)these32.sig[1]) << 32);
> > -#endif
> > -#ifdef __MIPSEL__
> > -    case 1: these.sig[0] = these32.sig[1] | (((long)these32.sig[0]) << 32);
> > -#endif
> > 
> > Consider a 64-bit sigset.  32-bit userland, 64-bit kernel.  Here's a
> > userland sigset with signal 33 set, only, on a little endian target.
> > Word 1, least significant bit, right?
> 
>  Right, but...
> 
> > byte address in memory
> > 	1	2	3	4	5	6	7	8
> > val	0	0	0	0	0	0	0	1
> 
> ... this is incorrect -- it would be right for big-endian; word #1, bit #1
> for little-endian is:
> 
> byte address in memory
> 	1	2	3	4	5	6	7	8
> val	0	0	0	0	1	0	0	0
> 
> 
> > Obviously, as a 64-bit integer the sigset looks different.  There it's
> > supposed to be 1 << (33 - 1).
> > val	0	0	0	1	0	0	0	0
> 
>  Again, for little-endian it should actually be:
> 
> val	0	0	0	0	1	0	0	0
> 
> i.e. the whole operation is actually a no-op, except that the 64-bit
> vector is assured to be properly aligned for doubleword accesses.

Re-reading what I wrote, the above was actually supposed to be a
big-endian example.  D'oh!  If you pretend I wrote "big endian" up at
the top, then it makes sense.

>  As a side note -- that's the reason certain C code portability problems
> related to the width of the machine word only get actually discovered when
> problematic software is run on a big-endian processor.  I've been hit by
> this property once -- I was porting a 16-bit program and it appeared to
> run just fine on both a 32-bit (i386) and a 64-bit (Alpha) little-endian
> CPU, but when run on a 32-bit big-endian one (SPARC) I discovered a few
> more bits to be cleaned up.
> 
> > So the correct algorithm to convert a userspace sigset to a kernel
> > sigset is to shift the second word left 32 bits, and leave the first
> > word right aligned, and or them together.  Which is what using the
> > __MIPSEB__ case does.
> 
>  But this conclusion is of course right.
> 
>   Maciej
> 
> -- 
> +  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
> +--------------------------------------------------------------+
> +        e-mail: macro@ds2.pg.gda.pl, PGP key available        +
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer