Andreas Färber a écrit : > Am 02.05.2013 22:08, schrieb Hervé Poussineau: >> When initializing all types in object_class_foreach, called by object_class_get_list, >> some new types may be registered. Those will change the type internal hashtable which >> is currently enumerated, and may crash QEMU. >> >> Fix it, by adding a second hash table which contains all the non-initialized types, >> merged to the main one before each round of initializations. >> >> Bug has been detected when registering dynamic types containing an interface. >> >> Signed-off-by: Hervé Poussineau >> --- >> qom/object.c | 45 +++++++++++++++++++++++++++++++++++++-------- >> 1 file changed, 37 insertions(+), 8 deletions(-) > > Could you be more specific about how to reproduce the problem? Is it a > generic issue or specific to some later patch in this series? I find > neither object_class_get_list() nor object_class_for_each() being used > in this series. And registering types during object_class_for_each() > doesn't sound right... CC'ing Anthony and Paolo. Try the attached patch, and run with qemu-system-ppc (no arguments) I added a dummy interface to a random device, but the problem should be exposed by whatever interface on whatever device. I saw the problem in patch 5/7 ("add a Nvram interface"). However, the problem doesn't seem to appear on other system emulations like i386. With attached patch, you'll get an assert: qemu-system-ppc: qom/object.c:82: type_table_add: Assertion `!enumerating' failed. Program received signal SIGABRT, Aborted. 0xb7fe1430 in __kernel_vsyscall () (gdb) bt #0 0xb7fe1430 in __kernel_vsyscall () #1 0xb6f27941 in *__GI_raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 #2 0xb6f2ad72 in *__GI_abort () at abort.c:92 #3 0xb6f20b58 in *__GI___assert_fail (assertion=assertion@entry=0x803809f8 "!enumerating", file=file@entry=0x80380adc "qom/object.c", line=line@entry=82, function=function@entry=0x80380c6c "type_table_add") at assert.c:81 #4 0x80197513 in type_table_add (ti=0x80b67bd0) at qom/object.c:82 #5 type_register_internal (info=0xbfffef0c) at qom/object.c:124 #6 0x8019764c in type_initialize_interface (parent=0x80b3ec18 "interface", ti=, ti=) at qom/object.c:218 #7 0x801978fe in type_initialize (ti=) at qom/object.c:271 #8 type_initialize (ti=0x80b3eb30) at qom/object.c:229 #9 0x80197dfa in object_class_foreach_tramp (key=0x80b3ebf0, value=0x80b3eb30, opaque=0xbffff03c) at qom/object.c:563 #10 0xb7ef35e2 in g_hash_table_foreach () from /lib/i386-linux-gnu/libglib-2.0.so.0 #11 0x801980b1 in object_class_foreach (fn=fn@entry=0x80197180 , implements_type=implements_type@entry=0x8039b834 "powerpc-cpu", include_abstract=include_abstract@entry=false, opaque=opaque@entry=0xbffff078) at qom/object.c:585 #12 0x801981ba in object_class_get_list (implements_type=implements_type@entry=0x8039b834 "powerpc-cpu", include_abstract=include_abstract@entry=false) at qom/object.c:618 #13 0x80328d4e in ppc_cpu_class_by_name (name=name@entry=0x8039dc69 "G3") at target-ppc/translate_init.c:8003 #14 0x80328f7a in cpu_ppc_init (cpu_model=cpu_model@entry=0x8039dc69 "G3") at target-ppc/translate_init.c:8020 #15 0x80216724 in ppc_heathrow_init (args=0xbffff2a8) at hw/ppc/mac_oldworld.c:109 #16 0x80040b81 in main (argc=1, argv=0xbffff4b4, envp=0xbffff4bc) at vl.c:4304 > >> diff --git a/qom/object.c b/qom/object.c >> index 75e6aac..e0a24dc 100644 >> --- a/qom/object.c >> +++ b/qom/object.c >> @@ -65,25 +65,39 @@ struct TypeImpl >> >> static Type type_interface; >> >> +static GHashTable *type_table_to_initialize; >> +static GHashTable *type_table_initialized; >> + >> static GHashTable *type_table_get(void) >> { >> - static GHashTable *type_table; >> - >> - if (type_table == NULL) { >> - type_table = g_hash_table_new(g_str_hash, g_str_equal); >> + if (!type_table_initialized) { >> + type_table_initialized = g_hash_table_new(g_str_hash, g_str_equal); >> } >> >> - return type_table; >> + return type_table_initialized; >> } >> >> static void type_table_add(TypeImpl *ti) >> { >> - g_hash_table_insert(type_table_get(), (void *)ti->name, ti); >> + GHashTable **type_table; >> + if (ti->class) { >> + type_table = &type_table_initialized; >> + } else { >> + type_table = &type_table_to_initialize; >> + } >> + if (!*type_table) { >> + *type_table = g_hash_table_new(g_str_hash, g_str_equal); >> + } >> + g_hash_table_insert(*type_table, (void *)ti->name, ti); >> } >> >> static TypeImpl *type_table_lookup(const char *name) >> { >> - return g_hash_table_lookup(type_table_get(), name); >> + TypeImpl *ret = g_hash_table_lookup(type_table_get(), name); >> + if (!ret && type_table_to_initialize) { >> + ret = g_hash_table_lookup(type_table_to_initialize, name); >> + } >> + return ret; >> } >> >> static TypeImpl *type_register_internal(const TypeInfo *info) >> @@ -573,13 +587,28 @@ static void object_class_foreach_tramp(gpointer key, gpointer value, >> data->fn(k, data->opaque); >> } >> >> +static void object_class_merge(gpointer key, gpointer value, >> + gpointer opaque) >> +{ >> + g_hash_table_insert(type_table_get(), key, value); >> +} >> + >> void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque), >> const char *implements_type, bool include_abstract, >> void *opaque) >> { >> OCFData data = { fn, implements_type, include_abstract, opaque }; >> >> - g_hash_table_foreach(type_table_get(), object_class_foreach_tramp, &data); >> + while (type_table_to_initialize && >> + g_hash_table_size(type_table_to_initialize) > 0) { >> + g_hash_table_foreach(type_table_to_initialize, object_class_merge, >> + NULL); >> + g_hash_table_destroy(type_table_to_initialize); >> + type_table_to_initialize = NULL; >> + >> + g_hash_table_foreach(type_table_get(), object_class_foreach_tramp, >> + &data); >> + } >> } >> >> int object_child_foreach(Object *obj, int (*fn)(Object *child, void *opaque), >> > >