Really Horrible GCJ 4.1 Classloading Bug Discovered

["Craig A. Vanderborgh" <craigv at voxware dot com>]

At the bottom of natClassLoader.cc are the following routines, that have 
been there for many years.  This code is from the gcc-4.1-20051125 
snapshot, but is unchanged from what it has been all along:

// These two functions form a stack of classes.   When a class is loaded
// it is pushed onto the stack by the class loader; this is so that
// StackTrace can quickly determine which classes have been loaded.

jclass
_Jv_PopClass (void)
{
 JvSynchronize sync (&java::lang::Class::class$);
 if (stack_head)
   {
     jclass tmp = stack_head;
     stack_head = tmp->chain;
     return tmp;
   }
 return NULL;
}

void
_Jv_PushClass (jclass k)
{
 JvSynchronize sync (&java::lang::Class::class$);
 jclass tmp = stack_head;
 stack_head = k;
 k->chain = tmp;
}

Straightahead stuff, right?  Just a nice clean little linked list 
implementation, that allows a "stack of classes" to be pushed onto a 
stack for someone else to pop off later.

But what would happen if the same class were pushed TWICE?  Well, what 
would happen is that a circular list would be created, because in the 
process of updating "stack_head" its "chain" pointer is updated to refer 
to the next item on the list.  Unfortunately, since this entry is 
actually the very same piece of memory as the tail of the list, this 
makes the tail point to the head.  Instant circular list.

This problem is far from a hypothetical one though.  I found it because 
my GCJ test program was regularly "hanging" in startup, pegging the 
CPU.  The reason is a "while (klass = _Jv_PopClass())" loop in 
natStackTrace.cc.  Once the list is circular, "PopClass()" always 
returns another class pointer.  Thus the infinite loop.  There is 
nothing special about my test program that provokes this, it's just a 
very large and general Java program.

Here is what I did to address this problem.  I extended PushClass a 
little bit so that it does not push the same class on to the list twice:

void
_Jv_PushClass (jclass k)
{
 JvSynchronize sync (&java::lang::Class::class$);

 if (stack_head != NULL) {
   if (k == stack_head) {
     return;
   }

   int i = 1;
   jclass next = stack_head->chain;
   while (next) {
     // Ensure this class is not already on the list
     if (k == next) {
       return;
     }

     next = next->chain;
   }
 }

 jclass tmp = stack_head;
 stack_head = k;
 k->chain = tmp;
}

This solves the problem.  I'm not submitting this as a patch because 
while this solution is effective, I want the maintainers to think about 
whether this kind of thing is appropriate.

Really though, I can't say enough about how great GCJ 4.1 is.  Thanks to 
everyone who made it happen, we absolutely love it.

Happy Holidays, Everybody!
craig vanderborgh
voxware incorporated