Re: Rgeression: 2.6.30-rc6-git3 build error - ICE from drivers/char/random.c

[Linus Torvalds <torvalds@linux-foundation.org>]

On Mon, 18 May 2009, Matt Mackall wrote:

> On Mon, May 18, 2009 at 06:29:13AM -0700, Martin Knoblauch wrote:
> > 
> > CC Maintainer.
> 
> Bizarre. I bet Linus has an opinion about your compiler.

I have opinions about a lot of things ;)

> > > drivers/char/random.c: In function `get_random_int':
> > > drivers/char/random.c:1672: error: unrecognizable insn:
> > > (insn 202 148 150 0 /scratch/build/linux-2.6.30-rc6-git3/arch/x86/include/asm/tsc.h:23 (set (reg:SI 0 ax [91])
> > >         (subreg:SI (plus:DI (plus:DI (reg:DI 0 ax [88])
> > >                     (subreg:DI (reg:SI 6 bp) 0))
> > >                 (const_int -4 [0xfffffffffffffffc])) 0)) -1 (nil)
> > >     (nil))
> > > drivers/char/random.c:1672: internal compiler error: in extract_insn, at recog.c:2083

Oh wow. We seldom hit these things. Internal gcc compiler errors do 
happen, but it tends to be _very_ rare. And it's almost always some subtle 
use of inline asm that gets gcc's knickers all twisted up. Often due to 
the double register usage of 64-bit long long arithmetic.

That error report seems to be a bit confused about exactly what triggered 
it: it points to both "get_cycles()" (arch/x86/include/asm/tsc.h:23) and 
to the get_cpu_var() asm code (drivers/char/random.c:1672).

Martin - Is this a 32-bit compile? In particular, does the problem go away 
if you remove the " + get_cycles() "  part (which is three lines down from 
what that error case reports, but with code movement and combining, who 
can tell which one it is?)

Usually the only way to get gcc to calm down when it gets into a tizzy 
like this is to rewrite the thing in some way that gcc has a harder time 
screwing up. In particular, asking gcc to handle a 64-bit value (inside 
get_cycles()) while it's busy doing a lot of other things may have just 
fused the compilers poor little brain.

So the fix _may_ be as simple as just something like the appended. It 
doesn't change anything, but it rewrites things so that it doesn't do that 
"rdtsc" while a lot of other things are also "in flight" at the same time.

		Linus

---
 drivers/char/random.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index b2ced39..68120e5 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1669,11 +1669,14 @@ DEFINE_PER_CPU(__u32 [4], get_random_int_hash);
 unsigned int get_random_int(void)
 {
 	struct keydata *keyptr;
-	__u32 *hash = get_cpu_var(get_random_int_hash);
+	__u32 *hash, garbage;
 	int ret;
 
+	garbage = get_cycles() + (long)&ret;
+	hash = get_cpu_var(get_random_int_hash);
+
 	keyptr = get_keyptr();
-	hash[0] += current->pid + jiffies + get_cycles() + (int)(long)&ret;
+	hash[0] += current->pid + jiffies + garbage;
 
 	ret = half_md4_transform(hash, keyptr->secret);
 	put_cpu_var(get_random_int_hash);