From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47474) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UYuSh-0004fV-RL for qemu-devel@nongnu.org; Sun, 05 May 2013 04:38:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UYuSf-0006j8-UJ for qemu-devel@nongnu.org; Sun, 05 May 2013 04:38:27 -0400 Message-ID: <51861A73.20900@reactos.org> Date: Sun, 05 May 2013 10:38:11 +0200 From: =?UTF-8?B?SGVydsOpIFBvdXNzaW5lYXU=?= MIME-Version: 1.0 References: <1367525344-7755-1-git-send-email-hpoussin@reactos.org> <1367525344-7755-3-git-send-email-hpoussin@reactos.org> <5183A39A.3000806@suse.de> In-Reply-To: <5183A39A.3000806@suse.de> Content-Type: multipart/mixed; boundary="------------070502090507090809050904" Subject: Re: [Qemu-devel] [PATCH 2/7] qom: handle registration of new types when initializing the first ones List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= Cc: Paolo Bonzini , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Anthony Liguori This is a multi-part message in MIME format. --------------070502090507090809050904 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Andreas F=C3=A4rber a =C3=A9crit : > Am 02.05.2013 22:08, schrieb Herv=C3=A9 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-initi= alized types, >> merged to the main one before each round of initializations. >> >> Bug has been detected when registering dynamic types containing an int= erface. >> >> Signed-off-by: Herv=C3=A9 Poussineau >> --- >> qom/object.c | 45 +++++++++++++++++++++++++++++++++++++-------- >> 1 file changed, 37 insertions(+), 8 deletions(-) >=20 > 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=20 exposed by whatever interface on whatever device. I saw the problem in=20 patch 5/7 ("add a Nvram interface"). However, the problem doesn't seem to appear on other system emulations=20 like i386. With attached patch, you'll get an assert: qemu-system-ppc: qom/object.c:82: type_table_add: Assertion=20 `!enumerating' failed. Program received signal SIGABRT, Aborted. 0xb7fe1430 in __kernel_vsyscall () (gdb) bt #0 0xb7fe1430 in __kernel_vsyscall () #1 0xb6f27941 in *__GI_raise (sig=3D6) at=20 ../nptl/sysdeps/unix/sysv/linux/raise.c:64 #2 0xb6f2ad72 in *__GI_abort () at abort.c:92 #3 0xb6f20b58 in *__GI___assert_fail=20 (assertion=3Dassertion@entry=3D0x803809f8 "!enumerating", file=3Dfile@entry=3D0x80380adc "qom/object.c", line=3Dline@entry=3D= 82, function=3Dfunction@entry=3D0x80380c6c "type_table_add") at assert.= c:81 #4 0x80197513 in type_table_add (ti=3D0x80b67bd0) at qom/object.c:82 #5 type_register_internal (info=3D0xbfffef0c) at qom/object.c:124 #6 0x8019764c in type_initialize_interface (parent=3D0x80b3ec18=20 "interface", ti=3D, ti=3D) at qom/object.c:218 #7 0x801978fe in type_initialize (ti=3D) at qom/object.= c:271 #8 type_initialize (ti=3D0x80b3eb30) at qom/object.c:229 #9 0x80197dfa in object_class_foreach_tramp (key=3D0x80b3ebf0,=20 value=3D0x80b3eb30, opaque=3D0xbffff03c) at qom/object.c:563 #10 0xb7ef35e2 in g_hash_table_foreach () from=20 /lib/i386-linux-gnu/libglib-2.0.so.0 #11 0x801980b1 in object_class_foreach (fn=3Dfn@entry=3D0x80197180=20 , implements_type=3Dimplements_type@entry=3D0x8039b834 "powerpc-cpu",= =20 include_abstract=3Dinclude_abstract@entry=3Dfalse, opaque=3Dopaque@entry=3D0xbffff078) at qom/object.c:585 #12 0x801981ba in object_class_get_list=20 (implements_type=3Dimplements_type@entry=3D0x8039b834 "powerpc-cpu", include_abstract=3Dinclude_abstract@entry=3Dfalse) at qom/object.c:= 618 #13 0x80328d4e in ppc_cpu_class_by_name (name=3Dname@entry=3D0x8039dc69= "G3") at target-ppc/translate_init.c:8003 #14 0x80328f7a in cpu_ppc_init (cpu_model=3Dcpu_model@entry=3D0x8039dc6= 9 "G3") at target-ppc/translate_init.c:8020 #15 0x80216724 in ppc_heathrow_init (args=3D0xbffff2a8) at=20 hw/ppc/mac_oldworld.c:109 #16 0x80040b81 in main (argc=3D1, argv=3D0xbffff4b4, envp=3D0xbffff4bc)= at=20 vl.c:4304 >=20 >> 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 >> =20 >> static Type type_interface; >> =20 >> +static GHashTable *type_table_to_initialize; >> +static GHashTable *type_table_initialized; >> + >> static GHashTable *type_table_get(void) >> { >> - static GHashTable *type_table; >> - >> - if (type_table =3D=3D NULL) { >> - type_table =3D g_hash_table_new(g_str_hash, g_str_equal); >> + if (!type_table_initialized) { >> + type_table_initialized =3D g_hash_table_new(g_str_hash, g_str= _equal); >> } >> =20 >> - return type_table; >> + return type_table_initialized; >> } >> =20 >> 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 =3D &type_table_initialized; >> + } else { >> + type_table =3D &type_table_to_initialize; >> + } >> + if (!*type_table) { >> + *type_table =3D g_hash_table_new(g_str_hash, g_str_equal); >> + } >> + g_hash_table_insert(*type_table, (void *)ti->name, ti); >> } >> =20 >> static TypeImpl *type_table_lookup(const char *name) >> { >> - return g_hash_table_lookup(type_table_get(), name); >> + TypeImpl *ret =3D g_hash_table_lookup(type_table_get(), name); >> + if (!ret && type_table_to_initialize) { >> + ret =3D g_hash_table_lookup(type_table_to_initialize, name); >> + } >> + return ret; >> } >> =20 >> 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); >> } >> =20 >> +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_a= bstract, >> void *opaque) >> { >> OCFData data =3D { fn, implements_type, include_abstract, opaque = }; >> =20 >> - 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_m= erge, >> + NULL); >> + g_hash_table_destroy(type_table_to_initialize); >> + type_table_to_initialize =3D NULL; >> + >> + g_hash_table_foreach(type_table_get(), object_class_foreach_t= ramp, >> + &data); >> + } >> } >> =20 >> int object_child_foreach(Object *obj, int (*fn)(Object *child, void *= opaque), >> >=20 >=20 --------------070502090507090809050904 Content-Type: text/plain; name="0001-usb-ehci-add-an-empty-interface-to-expose-a-problem-.patch" Content-Transfer-Encoding: base64 Content-Disposition: inline; filename*0="0001-usb-ehci-add-an-empty-interface-to-expose-a-problem-.pa"; filename*1="tch" RnJvbSA0ZGEyMmJlMzFkNWZjOGRmODg3YjhjNzZjNjA5Yjk4NDRiZWJlOWY0IE1vbiBTZXAg MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiA9P1VURi04P3E/SGVydj1DMz1BOT0yMFBvdXNzaW5l YXU/PSA8aHBvdXNzaW5AcmVhY3Rvcy5vcmc+CkRhdGU6IFN1biwgNSBNYXkgMjAxMyAxMDoz MToyNCArMDIwMApTdWJqZWN0OiBbUEFUQ0hdIHVzYi1laGNpOiBhZGQgYW4gZW1wdHkgaW50 ZXJmYWNlIHRvIGV4cG9zZSBhIHByb2JsZW0gaW4gUU9NCgpSdW4gd2l0aCBxZW11LXN5c3Rl bS1wcGMgKG5vIGFyZ3VtZW50cykKCnFlbXUtc3lzdGVtLXBwYzogcW9tL29iamVjdC5jOjgy OiB0eXBlX3RhYmxlX2FkZDogQXNzZXJ0aW9uIGAhZW51bWVyYXRpbmcnIGZhaWxlZC4KClBy b2dyYW0gcmVjZWl2ZWQgc2lnbmFsIFNJR0FCUlQsIEFib3J0ZWQuCjB4YjdmZTE0MzAgaW4g X19rZXJuZWxfdnN5c2NhbGwgKCkKKGdkYikgYnQKICMwICAweGI3ZmUxNDMwIGluIF9fa2Vy bmVsX3ZzeXNjYWxsICgpCiAjMSAgMHhiNmYyNzk0MSBpbiAqX19HSV9yYWlzZSAoc2lnPTYp IGF0IC4uL25wdGwvc3lzZGVwcy91bml4L3N5c3YvbGludXgvcmFpc2UuYzo2NAogIzIgIDB4 YjZmMmFkNzIgaW4gKl9fR0lfYWJvcnQgKCkgYXQgYWJvcnQuYzo5MgogIzMgIDB4YjZmMjBi NTggaW4gKl9fR0lfX19hc3NlcnRfZmFpbCAoYXNzZXJ0aW9uPWFzc2VydGlvbkBlbnRyeT0w eDgwMzgwOWY4ICIhZW51bWVyYXRpbmciLAogICAgIGZpbGU9ZmlsZUBlbnRyeT0weDgwMzgw YWRjICJxb20vb2JqZWN0LmMiLCBsaW5lPWxpbmVAZW50cnk9ODIsCiAgICAgZnVuY3Rpb249 ZnVuY3Rpb25AZW50cnk9MHg4MDM4MGM2YyAidHlwZV90YWJsZV9hZGQiKSBhdCBhc3NlcnQu Yzo4MQogIzQgIDB4ODAxOTc1MTMgaW4gdHlwZV90YWJsZV9hZGQgKHRpPTB4ODBiNjdiZDAp IGF0IHFvbS9vYmplY3QuYzo4MgogIzUgIHR5cGVfcmVnaXN0ZXJfaW50ZXJuYWwgKGluZm89 MHhiZmZmZWYwYykgYXQgcW9tL29iamVjdC5jOjEyNAogIzYgIDB4ODAxOTc2NGMgaW4gdHlw ZV9pbml0aWFsaXplX2ludGVyZmFjZSAocGFyZW50PTB4ODBiM2VjMTggImludGVyZmFjZSIs CiAgICAgdGk9PGVycm9yIHJlYWRpbmcgdmFyaWFibGU6IFVuaGFuZGxlZCBkd2FyZiBleHBy ZXNzaW9uIG9wY29kZSAweGZhPiwKICAgICB0aT08ZXJyb3IgcmVhZGluZyB2YXJpYWJsZTog VW5oYW5kbGVkIGR3YXJmIGV4cHJlc3Npb24gb3Bjb2RlIDB4ZmE+KSBhdCBxb20vb2JqZWN0 LmM6MjE4CiAjNyAgMHg4MDE5NzhmZSBpbiB0eXBlX2luaXRpYWxpemUgKHRpPTxvcHRpbWl6 ZWQgb3V0PikgYXQgcW9tL29iamVjdC5jOjI3MQogIzggIHR5cGVfaW5pdGlhbGl6ZSAodGk9 MHg4MGIzZWIzMCkgYXQgcW9tL29iamVjdC5jOjIyOQogIzkgIDB4ODAxOTdkZmEgaW4gb2Jq ZWN0X2NsYXNzX2ZvcmVhY2hfdHJhbXAgKGtleT0weDgwYjNlYmYwLCB2YWx1ZT0weDgwYjNl YjMwLCBvcGFxdWU9MHhiZmZmZjAzYykKICAgICBhdCBxb20vb2JqZWN0LmM6NTYzCiAjMTAg MHhiN2VmMzVlMiBpbiBnX2hhc2hfdGFibGVfZm9yZWFjaCAoKSBmcm9tIC9saWIvaTM4Ni1s aW51eC1nbnUvbGliZ2xpYi0yLjAuc28uMAogIzExIDB4ODAxOTgwYjEgaW4gb2JqZWN0X2Ns YXNzX2ZvcmVhY2ggKGZuPWZuQGVudHJ5PTB4ODAxOTcxODAgPG9iamVjdF9jbGFzc19nZXRf bGlzdF90cmFtcD4sCiAgICAgaW1wbGVtZW50c190eXBlPWltcGxlbWVudHNfdHlwZUBlbnRy eT0weDgwMzliODM0ICJwb3dlcnBjLWNwdSIsIGluY2x1ZGVfYWJzdHJhY3Q9aW5jbHVkZV9h YnN0cmFjdEBlbnRyeT1mYWxzZSwKICAgICBvcGFxdWU9b3BhcXVlQGVudHJ5PTB4YmZmZmYw NzgpIGF0IHFvbS9vYmplY3QuYzo1ODUKICMxMiAweDgwMTk4MWJhIGluIG9iamVjdF9jbGFz c19nZXRfbGlzdCAoaW1wbGVtZW50c190eXBlPWltcGxlbWVudHNfdHlwZUBlbnRyeT0weDgw MzliODM0ICJwb3dlcnBjLWNwdSIsCiAgICAgaW5jbHVkZV9hYnN0cmFjdD1pbmNsdWRlX2Fi c3RyYWN0QGVudHJ5PWZhbHNlKSBhdCBxb20vb2JqZWN0LmM6NjE4CiAjMTMgMHg4MDMyOGQ0 ZSBpbiBwcGNfY3B1X2NsYXNzX2J5X25hbWUgKG5hbWU9bmFtZUBlbnRyeT0weDgwMzlkYzY5 ICJHMyIpCiAgICAgYXQgdGFyZ2V0LXBwYy90cmFuc2xhdGVfaW5pdC5jOjgwMDMKICMxNCAw eDgwMzI4ZjdhIGluIGNwdV9wcGNfaW5pdCAoY3B1X21vZGVsPWNwdV9tb2RlbEBlbnRyeT0w eDgwMzlkYzY5ICJHMyIpCiAgICAgYXQgdGFyZ2V0LXBwYy90cmFuc2xhdGVfaW5pdC5jOjgw MjAKICMxNSAweDgwMjE2NzI0IGluIHBwY19oZWF0aHJvd19pbml0IChhcmdzPTB4YmZmZmYy YTgpIGF0IGh3L3BwYy9tYWNfb2xkd29ybGQuYzoxMDkKICMxNiAweDgwMDQwYjgxIGluIG1h aW4gKGFyZ2M9MSwgYXJndj0weGJmZmZmNGI0LCBlbnZwPTB4YmZmZmY0YmMpIGF0IHZsLmM6 NDMwNAotLS0KIGh3L3VzYi9oY2QtdWhjaS5jIHwgICAgNCArKysrCiBxb20vb2JqZWN0LmMg ICAgICB8ICAgIDQgKysrKwogMiBmaWxlcyBjaGFuZ2VkLCA4IGluc2VydGlvbnMoKykKCmRp ZmYgLS1naXQgYS9ody91c2IvaGNkLXVoY2kuYyBiL2h3L3VzYi9oY2QtdWhjaS5jCmluZGV4 IGY4YzQyODYuLmE5NWNhMzAgMTAwNjQ0Ci0tLSBhL2h3L3VzYi9oY2QtdWhjaS5jCisrKyBi L2h3L3VzYi9oY2QtdWhjaS5jCkBAIC0xMzg2LDYgKzEzODYsMTAgQEAgc3RhdGljIHZvaWQg dWhjaV9yZWdpc3Rlcl90eXBlcyh2b2lkKQogICAgICAgICAuaW5zdGFuY2Vfc2l6ZSA9IHNp emVvZihVSENJU3RhdGUpLAogICAgICAgICAuY2xhc3Nfc2l6ZSAgICA9IHNpemVvZihVSENJ UENJRGV2aWNlQ2xhc3MpLAogICAgICAgICAuY2xhc3NfaW5pdCAgICA9IHVoY2lfY2xhc3Nf aW5pdCwKKyAgICAgICAgLmludGVyZmFjZXMgPSAoSW50ZXJmYWNlSW5mb1tdKSB7CisgICAg ICAgICAgICB7IFRZUEVfSU5URVJGQUNFIH0sCisgICAgICAgICAgICB7IH0KKyAgICAgICAg fQogICAgIH07CiAgICAgaW50IGk7CiAKZGlmZiAtLWdpdCBhL3FvbS9vYmplY3QuYyBiL3Fv bS9vYmplY3QuYwppbmRleCA3NWU2YWFjLi5iOGU5ZjRmIDEwMDY0NAotLS0gYS9xb20vb2Jq ZWN0LmMKKysrIGIvcW9tL29iamVjdC5jCkBAIC03Niw4ICs3NiwxMCBAQCBzdGF0aWMgR0hh c2hUYWJsZSAqdHlwZV90YWJsZV9nZXQodm9pZCkKICAgICByZXR1cm4gdHlwZV90YWJsZTsK IH0KIAorc3RhdGljIGJvb2wgZW51bWVyYXRpbmcgPSBmYWxzZTsKIHN0YXRpYyB2b2lkIHR5 cGVfdGFibGVfYWRkKFR5cGVJbXBsICp0aSkKIHsKKyAgICBhc3NlcnQoIWVudW1lcmF0aW5n KTsKICAgICBnX2hhc2hfdGFibGVfaW5zZXJ0KHR5cGVfdGFibGVfZ2V0KCksICh2b2lkICop dGktPm5hbWUsIHRpKTsKIH0KIApAQCAtNTc5LDcgKzU4MSw5IEBAIHZvaWQgb2JqZWN0X2Ns YXNzX2ZvcmVhY2godm9pZCAoKmZuKShPYmplY3RDbGFzcyAqa2xhc3MsIHZvaWQgKm9wYXF1 ZSksCiB7CiAgICAgT0NGRGF0YSBkYXRhID0geyBmbiwgaW1wbGVtZW50c190eXBlLCBpbmNs dWRlX2Fic3RyYWN0LCBvcGFxdWUgfTsKIAorICAgIGVudW1lcmF0aW5nID0gdHJ1ZTsKICAg ICBnX2hhc2hfdGFibGVfZm9yZWFjaCh0eXBlX3RhYmxlX2dldCgpLCBvYmplY3RfY2xhc3Nf Zm9yZWFjaF90cmFtcCwgJmRhdGEpOworICAgIGVudW1lcmF0aW5nID0gZmFsc2U7CiB9CiAK IGludCBvYmplY3RfY2hpbGRfZm9yZWFjaChPYmplY3QgKm9iaiwgaW50ICgqZm4pKE9iamVj dCAqY2hpbGQsIHZvaWQgKm9wYXF1ZSksCi0tIAoxLjcuMTAuNAoK --------------070502090507090809050904--