diff options
Diffstat (limited to 'libjava')
| -rw-r--r-- | libjava/ChangeLog | 13 | ||||
| -rw-r--r-- | libjava/include/jvm.h | 24 | ||||
| -rw-r--r-- | libjava/interpret-run.cc | 24 | ||||
| -rw-r--r-- | libjava/link.cc | 76 |
4 files changed, 106 insertions, 31 deletions
diff --git a/libjava/ChangeLog b/libjava/ChangeLog index 891b4dca588..8aacf4f11d0 100644 --- a/libjava/ChangeLog +++ b/libjava/ChangeLog @@ -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 diff --git a/libjava/include/jvm.h b/libjava/include/jvm.h index 64cd6b5d7f9..ec74f295a5f 100644 --- a/libjava/include/jvm.h +++ b/libjava/include/jvm.h @@ -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. */ diff --git a/libjava/interpret-run.cc b/libjava/interpret-run.cc index f858c971e0b..2934b9b8956 100644 --- a/libjava/interpret-run.cc +++ b/libjava/interpret-run.cc @@ -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 */ diff --git a/libjava/link.cc b/libjava/link.cc index f995531e813..c07b6e15c1c 100644 --- a/libjava/link.cc +++ b/libjava/link.cc @@ -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); } } |