gij: more thread-safety fixes

[Andrew Haley <aph at redhat dot com>]

There were more problems in gij related to the lack of locking when
resolving constants.  This patch fixes the problems by using
fine-grained locking whenever accessing the constant pool.

This lack of synchronization did not affect precompiled code because
gcj resolves constants by emitting calls _Jv_ResolvePoolEntry, and
that function locks the class whenever updating the constant pool.
While coarse, this does guarantee that the data races in the
interpreter cannot occur, which is why we've never seen this problem
with precompiled code.

In theory the class lock in _Jv_ResolvePoolEntry could now be removed
because the new fine-grained locking makes it unnececessary.  However,
I'm not going to remove it yet.

Andrew.


2008-08-22  Andrew Haley  <aph@redhat.com>

	* include/jvm.h (class _Jv_Linker): Declare resolve_mutex, init.
	(read_cpool_entry, write_cpool_entry): New functions.
	* link.cc (_Jv_Linker::resolve_mutex): new.
	(_Jv_Linker::init): New function.
	(_Jv_Linker::resolve_pool_entry): Use {read,write}_cpool_entry
	to ensure atomic access to constant pool entries.

*** include/jvm.h~	2007-12-07 16:55:41.000000000 +0000
--- include/jvm.h	2008-08-21 18:11:01.000000000 +0100
***************
*** 307,312 ****
--- 307,315 ----
      s = signature;
    }  
  
+   static _Jv_Mutex_t resolve_mutex;
+   static void init (void) __attribute__((constructor));
+ 
  public:
  
    static bool has_field_p (jclass, _Jv_Utf8Const *);
***************
*** 324,329 ****
--- 327,353 ----
  					     _Jv_Utf8Const *,
  					     bool check_perms = true);
    static void layout_vtable_methods(jclass);
+ 
+   static jbyte read_cpool_entry (_Jv_word *data,
+ 				 const _Jv_Constants *const pool,
+ 				 int index)
+   {
+     _Jv_MutexLock (&resolve_mutex);
+     jbyte tags = pool->tags[index];
+     *data = pool->data[index];
+     _Jv_MutexUnlock (&resolve_mutex);
+     return tags;
+   }
+ 
+   static void write_cpool_entry (_Jv_word data, jbyte tags,
+ 				 _Jv_Constants *pool,
+ 				 int index)
+   {
+     _Jv_MutexLock (&resolve_mutex);
+     pool->data[index] = data;
+     pool->tags[index] = tags;
+     _Jv_MutexUnlock (&resolve_mutex);
+   }
  };
  
  /* Type of pointer used as finalizer.  */
*** link.cc~	2008-03-14 17:07:31.000000000 +0000
--- link.cc	2008-08-22 10:55:26.000000000 +0100
***************
*** 362,367 ****
--- 362,380 ----
    return the_method;
  }
  
+ _Jv_Mutex_t _Jv_Linker::resolve_mutex;
+ 
+ void
+ _Jv_Linker::init (void)
+ {
+   _Jv_MutexInit (&_Jv_Linker::resolve_mutex);
+ }
+ 
+ // Locking in resolve_pool_entry is somewhat subtle.  Constant
+ // resolution is idempotent, so it doesn't matter if two threads
+ // resolve the same entry.  However, it is important that we always
+ // write the resolved flag and the data together, atomically.  It is
+ // also important that we read them atomically.
  _Jv_word
  _Jv_Linker::resolve_pool_entry (jclass klass, int index, bool lazy)
  {
***************
*** 369,374 ****
--- 382,391 ----
  
    if (GC_base (klass) && klass->constants.data
        && ! GC_base (klass->constants.data))
+     // If a class is heap-allocated but the constant pool is not this
+     // is a "new ABI" class, i.e. one where the initial constant pool
+     // is in the read-only data section of an object file.  Copy the
+     // initial constant pool from there to a new heap-allocated pool.
      {
        jsize count = klass->constants.size;
        if (count)
***************
*** 384,397 ****
  
    _Jv_Constants *pool = &klass->constants;
  
!   if ((pool->tags[index] & JV_CONSTANT_ResolvedFlag) != 0)
!     return pool->data[index];
  
!   switch (pool->tags[index] & ~JV_CONSTANT_LazyFlag)
      {
      case JV_CONSTANT_Class:
        {
! 	_Jv_Utf8Const *name = pool->data[index].utf8;
  
  	jclass found;
  	if (name->first() == '[')
--- 401,418 ----
  
    _Jv_Constants *pool = &klass->constants;
  
!   jbyte tags;
!   _Jv_word data;
!   tags = read_cpool_entry (&data, pool, index);
  
!   if ((tags & JV_CONSTANT_ResolvedFlag) != 0)
!     return data;
! 
!   switch (tags & ~JV_CONSTANT_LazyFlag)
      {
      case JV_CONSTANT_Class:
        {
! 	_Jv_Utf8Const *name = data.utf8;
  
  	jclass found;
  	if (name->first() == '[')
***************
*** 410,417 ****
  	      {
  		found = _Jv_NewClass(name, NULL, NULL);
  		found->state = JV_STATE_PHANTOM;
! 		pool->tags[index] |= JV_CONSTANT_ResolvedFlag;
! 		pool->data[index].clazz = found;
  		break;
  	      }
  	    else
--- 431,438 ----
  	      {
  		found = _Jv_NewClass(name, NULL, NULL);
  		found->state = JV_STATE_PHANTOM;
! 		tags |= JV_CONSTANT_ResolvedFlag;
! 		data.clazz = found;
  		break;
  	      }
  	    else
***************
*** 429,436 ****
  	    || (_Jv_ClassNameSamePackage (check->name,
  					  klass->name)))
  	  {
! 	    pool->data[index].clazz = found;
! 	    pool->tags[index] |= JV_CONSTANT_ResolvedFlag;
  	  }
  	else
  	  {
--- 450,457 ----
  	    || (_Jv_ClassNameSamePackage (check->name,
  					  klass->name)))
  	  {
! 	    data.clazz = found;
! 	    tags |= JV_CONSTANT_ResolvedFlag;
  	  }
  	else
  	  {
***************
*** 446,461 ****
      case JV_CONSTANT_String:
        {
  	jstring str;
! 	str = _Jv_NewStringUtf8Const (pool->data[index].utf8);
! 	pool->data[index].o = str;
! 	pool->tags[index] |= JV_CONSTANT_ResolvedFlag;
        }
        break;
  
      case JV_CONSTANT_Fieldref:
        {
  	_Jv_ushort class_index, name_and_type_index;
! 	_Jv_loadIndexes (&pool->data[index],
  			 class_index,
  			 name_and_type_index);
  	jclass owner = (resolve_pool_entry (klass, class_index, true)).clazz;
--- 467,482 ----
      case JV_CONSTANT_String:
        {
  	jstring str;
! 	str = _Jv_NewStringUtf8Const (data.utf8);
! 	data.o = str;
! 	tags |= JV_CONSTANT_ResolvedFlag;
        }
        break;
  
      case JV_CONSTANT_Fieldref:
        {
  	_Jv_ushort class_index, name_and_type_index;
! 	_Jv_loadIndexes (&data,
  			 class_index,
  			 name_and_type_index);
  	jclass owner = (resolve_pool_entry (klass, class_index, true)).clazz;
***************
*** 485,492 ****
  	// Initialize the field's declaring class, not its qualifying
  	// class.
  	_Jv_InitClass (found_class);
! 	pool->data[index].field = the_field;
! 	pool->tags[index] |= JV_CONSTANT_ResolvedFlag;
        }
        break;
  
--- 506,513 ----
  	// Initialize the field's declaring class, not its qualifying
  	// class.
  	_Jv_InitClass (found_class);
! 	data.field = the_field;
! 	tags |= JV_CONSTANT_ResolvedFlag;
        }
        break;
  
***************
*** 494,500 ****
      case JV_CONSTANT_InterfaceMethodref:
        {
  	_Jv_ushort class_index, name_and_type_index;
! 	_Jv_loadIndexes (&pool->data[index],
  			 class_index,
  			 name_and_type_index);
  
--- 515,521 ----
      case JV_CONSTANT_InterfaceMethodref:
        {
  	_Jv_ushort class_index, name_and_type_index;
! 	_Jv_loadIndexes (&data,
  			 class_index,
  			 name_and_type_index);
  
***************
*** 503,520 ****
  	the_method = resolve_method_entry (klass, found_class,
  					   class_index, name_and_type_index,
  					   true,
! 					   pool->tags[index] == JV_CONSTANT_InterfaceMethodref);
        
! 	pool->data[index].rmethod
  	  = klass->engine->resolve_method(the_method,
  					  found_class,
  					  ((the_method->accflags
  					    & Modifier::STATIC) != 0));
! 	pool->tags[index] |= JV_CONSTANT_ResolvedFlag;
        }
        break;
      }
!   return pool->data[index];
  }
  
  // This function is used to lazily locate superclasses and
--- 524,544 ----
  	the_method = resolve_method_entry (klass, found_class,
  					   class_index, name_and_type_index,
  					   true,
! 					   tags == JV_CONSTANT_InterfaceMethodref);
        
! 	data.rmethod
  	  = klass->engine->resolve_method(the_method,
  					  found_class,
  					  ((the_method->accflags
  					    & Modifier::STATIC) != 0));
! 	tags |= JV_CONSTANT_ResolvedFlag;
        }
        break;
      }
! 
!   write_cpool_entry (data, tags, pool, index);
! 
!   return data;
  }
  
  // This function is used to lazily locate superclasses and
***************
*** 1701,1713 ****
        // Resolve the remaining constant pool entries.
        for (int index = 1; index < pool->size; ++index)
  	{
! 	  if (pool->tags[index] == JV_CONSTANT_String)
! 	    {
! 	      jstring str;
  
! 	      str = _Jv_NewStringUtf8Const (pool->data[index].utf8);
! 	      pool->data[index].o = str;
! 	      pool->tags[index] |= JV_CONSTANT_ResolvedFlag;
  	    }
  	}
  
--- 1725,1739 ----
        // Resolve the remaining constant pool entries.
        for (int index = 1; index < pool->size; ++index)
  	{
! 	  jbyte tags;
! 	  _Jv_word data;
  
! 	  tags = read_cpool_entry (&data, pool, index);
! 	  if (tags == JV_CONSTANT_String)
! 	    {
! 	      data.o = _Jv_NewStringUtf8Const (data.utf8);
! 	      tags |= JV_CONSTANT_ResolvedFlag;
! 	      write_cpool_entry (data, tags, pool, index);
  	    }
  	}