[PATCH] casts in TLB macros

[PATCH] casts in TLB macros

[Andrew Isaacson]

Fix three cases where macro arguments are not parenthesized, leading to
incorrect operator precedence when called with an expression as the
argument.  This causes incorrect evaluation of
    write_c0_entrylo0(pte_val(*ptep++) >> 6)
when pte_t is 64 bits - the pte value is cast to (unsigned int) first,
then the shift is done, losing the top 32 bits.

Also, this does not add an extra set of parentheses surrounding the
(cast)(value) pair, as there's no danger of precedence problems to avoid
given the high precedence of the cast operator and that this is the
terminal macro in this macro trail.

Signed-off-by: Andrew Isaacson <adi@broadcom.com>

 include/asm-mips/pgtable-32.h |    2 +-

Index: linux-2.6.13-rc5/include/asm-mips/mipsregs.h
===================================================================
--- linux-2.6.13-rc5.orig/include/asm-mips/mipsregs.h	2005-07-14 10:47:59.000000000 -0700
+++ linux-2.6.13-rc5/include/asm-mips/mipsregs.h	2005-08-16 19:44:47.000000000 -0700
@@ -693,13 +693,13 @@
 	if (sel == 0)							\
 		__asm__ __volatile__(					\
 			"mtc0\t%z0, " #register "\n\t"			\
-			: : "Jr" ((unsigned int)value));		\
+			: : "Jr" (unsigned int)(value));		\
 	else								\
 		__asm__ __volatile__(					\
 			".set\tmips32\n\t"				\
 			"mtc0\t%z0, " #register ", " #sel "\n\t"	\
 			".set\tmips0"					\
-			: : "Jr" ((unsigned int)value));		\
+			: : "Jr" (unsigned int)(value));		\
 } while (0)
 
 #define __write_64bit_c0_register(register, sel, value)			\
@@ -748,7 +748,7 @@
 do {									\
 	__asm__ __volatile__(						\
 		"ctc0\t%z0, " #register "\n\t"				\
-		: : "Jr" ((unsigned int)value));			\
+		: : "Jr" (unsigned int)(value));			\
 } while (0)
 
 /*

Re: [PATCH] casts in TLB macros

[Maciej W. Rozycki]

On Tue, 16 Aug 2005, Andrew Isaacson wrote:

> @@ -748,7 +748,7 @@
>  do {									\
>  	__asm__ __volatile__(						\
>  		"ctc0\t%z0, " #register "\n\t"				\
> -		: : "Jr" ((unsigned int)value));			\
> +		: : "Jr" (unsigned int)(value));			\
>  } while (0)
>  
>  /*

 I'm surprised it works, but please don't drop the outer brackets in asm 
operands anyway.  Otherwise it's an obvious fix, thanks.

  Maciej

Re: [PATCH] casts in TLB macros

[Ralf Baechle]

On Tue, Aug 16, 2005 at 08:06:08PM -0700, Andrew Isaacson wrote:

> Fix three cases where macro arguments are not parenthesized, leading to
> incorrect operator precedence when called with an expression as the
> argument.  This causes incorrect evaluation of
>     write_c0_entrylo0(pte_val(*ptep++) >> 6)
> when pte_t is 64 bits - the pte value is cast to (unsigned int) first,
> then the shift is done, losing the top 32 bits.
> 
> Also, this does not add an extra set of parentheses surrounding the
> (cast)(value) pair, as there's no danger of precedence problems to avoid
> given the high precedence of the cast operator and that this is the
> terminal macro in this macro trail.

Thanks, applied,

  Ralf

Re: [PATCH] casts in TLB macros

[Andrew Isaacson]

On Wed, Aug 17, 2005 at 11:08:30AM +0100, Ralf Baechle wrote:
> On Tue, Aug 16, 2005 at 08:06:08PM -0700, Andrew Isaacson wrote:
> 
> > Fix three cases where macro arguments are not parenthesized, leading to
> > incorrect operator precedence when called with an expression as the
> > argument.  This causes incorrect evaluation of
> >     write_c0_entrylo0(pte_val(*ptep++) >> 6)
> > when pte_t is 64 bits - the pte value is cast to (unsigned int) first,
> > then the shift is done, losing the top 32 bits.
> > 
> > Also, this does not add an extra set of parentheses surrounding the
> > (cast)(value) pair, as there's no danger of precedence problems to avoid
> > given the high precedence of the cast operator and that this is the
> > terminal macro in this macro trail.
> 
> Thanks, applied,

Thank *you* for being smarter than me about the parenthesis...

(The patch I sent out was lacking the outer parens, and I used the
following plan for world domination:
1. notice problem, cut patch
2. go to dinner
3. test compile something unrelated by accident
4. send patch to mailing list
5. profit!)

-andy

Re: [PATCH] casts in TLB macros

[Ralf Baechle]

On Wed, Aug 17, 2005 at 09:30:02AM -0700, Andrew Isaacson wrote:

> (The patch I sent out was lacking the outer parens, and I used the
> following plan for world domination:
> 1. notice problem, cut patch
> 2. go to dinner
> 3. test compile something unrelated by accident
> 4. send patch to mailing list
> 5. profit!)

5) reminds me of Clausen Profit System Online, see
http://members.optusnet.com.au/clausen/profit/

  Ralf