re PR libgcj/8995 (race cases in interpreter)

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

        PR libgcj/8895:

        * interpret-run.cc (REWRITE_INSN): Null this macro.

        * 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.

From-SVN: r139492
This commit is contained in:
Andrew Haley 2008-08-22 16:04:29 +00:00 committed by Andrew Haley
parent c9f1fdfe4c
commit e4493315fc
4 changed files with 106 additions and 31 deletions

View File

@ -1,3 +1,16 @@
2008-08-22 Andrew Haley <aph@redhat.com>
PR libgcj/8895:
* interpret-run.cc (REWRITE_INSN): Null this macro.
* 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.
2008-08-07 Andrew Haley <aph@redhat.com>
* testsuite/libjava.lang/StackTrace2.java: Rewrite to prevent

View File

@ -308,6 +308,9 @@ private:
s = signature;
}
static _Jv_Mutex_t resolve_mutex;
static void init (void) __attribute__((constructor));
public:
static bool has_field_p (jclass, _Jv_Utf8Const *);
@ -325,6 +328,27 @@ public:
_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. */

View File

@ -382,12 +382,24 @@ details. */
#else // !DEBUG
#undef NEXT_INSN
#define NEXT_INSN goto *((pc++)->insn)
#define REWRITE_INSN(INSN,SLOT,VALUE) \
do { \
pc[-2].insn = INSN; \
pc[-1].SLOT = VALUE; \
} \
while (0)
// REWRITE_INSN does nothing.
//
// Rewriting a multi-word instruction in the presence of multiple
// threads leads to a data race if a thread reads part of an
// instruction while some other thread is rewriting that instruction.
// For example, an invokespecial instruction may be rewritten to
// invokespecial_resolved and its operand changed from an index to a
// pointer while another thread is executing invokespecial. This
// other thread then reads the pointer that is now the operand of
// invokespecial_resolved and tries to use it as an index.
//
// Fixing this requires either spinlocks, a more elaborate data
// structure, or even per-thread allocated pages. It's clear from the
// locking in meth->compile below that the presence of multiple
// threads was contemplated when this code was written, but the full
// consequences were not fully appreciated.
#define REWRITE_INSN(INSN,SLOT,VALUE)
#undef INTERP_REPORT_EXCEPTION
#define INTERP_REPORT_EXCEPTION(Jthrowable) /* not needed when not debugging */

View File

@ -380,6 +380,19 @@ _Jv_Linker::resolve_method_entry (jclass klass, jclass &found_class,
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)
{
@ -387,6 +400,10 @@ _Jv_Linker::resolve_pool_entry (jclass klass, int index, bool lazy)
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)
@ -402,14 +419,18 @@ _Jv_Linker::resolve_pool_entry (jclass klass, int index, bool lazy)
_Jv_Constants *pool = &klass->constants;
if ((pool->tags[index] & JV_CONSTANT_ResolvedFlag) != 0)
return pool->data[index];
jbyte tags;
_Jv_word data;
tags = read_cpool_entry (&data, pool, index);
switch (pool->tags[index] & ~JV_CONSTANT_LazyFlag)
if ((tags & JV_CONSTANT_ResolvedFlag) != 0)
return data;
switch (tags & ~JV_CONSTANT_LazyFlag)
{
case JV_CONSTANT_Class:
{
_Jv_Utf8Const *name = pool->data[index].utf8;
_Jv_Utf8Const *name = data.utf8;
jclass found;
if (name->first() == '[')
@ -428,8 +449,8 @@ _Jv_Linker::resolve_pool_entry (jclass klass, int index, bool lazy)
{
found = _Jv_NewClass(name, NULL, NULL);
found->state = JV_STATE_PHANTOM;
pool->tags[index] |= JV_CONSTANT_ResolvedFlag;
pool->data[index].clazz = found;
tags |= JV_CONSTANT_ResolvedFlag;
data.clazz = found;
break;
}
else
@ -447,8 +468,8 @@ _Jv_Linker::resolve_pool_entry (jclass klass, int index, bool lazy)
|| (_Jv_ClassNameSamePackage (check->name,
klass->name)))
{
pool->data[index].clazz = found;
pool->tags[index] |= JV_CONSTANT_ResolvedFlag;
data.clazz = found;
tags |= JV_CONSTANT_ResolvedFlag;
}
else
{
@ -464,16 +485,16 @@ _Jv_Linker::resolve_pool_entry (jclass klass, int index, bool lazy)
case JV_CONSTANT_String:
{
jstring str;
str = _Jv_NewStringUtf8Const (pool->data[index].utf8);
pool->data[index].o = str;
pool->tags[index] |= JV_CONSTANT_ResolvedFlag;
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 (&pool->data[index],
_Jv_loadIndexes (&data,
class_index,
name_and_type_index);
jclass owner = (resolve_pool_entry (klass, class_index, true)).clazz;
@ -503,8 +524,8 @@ _Jv_Linker::resolve_pool_entry (jclass klass, int index, bool lazy)
// 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;
data.field = the_field;
tags |= JV_CONSTANT_ResolvedFlag;
}
break;
@ -512,7 +533,7 @@ _Jv_Linker::resolve_pool_entry (jclass klass, int index, bool lazy)
case JV_CONSTANT_InterfaceMethodref:
{
_Jv_ushort class_index, name_and_type_index;
_Jv_loadIndexes (&pool->data[index],
_Jv_loadIndexes (&data,
class_index,
name_and_type_index);
@ -521,18 +542,21 @@ _Jv_Linker::resolve_pool_entry (jclass klass, int index, bool lazy)
the_method = resolve_method_entry (klass, found_class,
class_index, name_and_type_index,
true,
pool->tags[index] == JV_CONSTANT_InterfaceMethodref);
tags == JV_CONSTANT_InterfaceMethodref);
pool->data[index].rmethod
data.rmethod
= klass->engine->resolve_method(the_method,
found_class,
((the_method->accflags
& Modifier::STATIC) != 0));
pool->tags[index] |= JV_CONSTANT_ResolvedFlag;
tags |= JV_CONSTANT_ResolvedFlag;
}
break;
}
return pool->data[index];
write_cpool_entry (data, tags, pool, index);
return data;
}
// This function is used to lazily locate superclasses and
@ -1728,13 +1752,15 @@ _Jv_Linker::ensure_class_linked (jclass klass)
// Resolve the remaining constant pool entries.
for (int index = 1; index < pool->size; ++index)
{
if (pool->tags[index] == JV_CONSTANT_String)
{
jstring str;
jbyte tags;
_Jv_word data;
str = _Jv_NewStringUtf8Const (pool->data[index].utf8);
pool->data[index].o = str;
pool->tags[index] |= JV_CONSTANT_ResolvedFlag;
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);
}
}