After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 403138 - Invalid read reported by valgrind
Invalid read reported by valgrind
Status: RESOLVED FIXED
Product: atk
Classification: Platform
Component: gail
git master
Other Linux
: Normal critical
: ---
Assigned To: Li Yuan
Li Yuan
: 404664 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2007-02-01 09:50 UTC by Kjartan Maraas
Modified: 2007-02-07 08:32 UTC
See Also:
GNOME target: ---
GNOME version: 2.17/2.18


Attachments
patch to fix the problem (921 bytes, patch)
2007-02-01 09:51 UTC, Kjartan Maraas
none Details | Review
new patch (744 bytes, patch)
2007-02-01 10:08 UTC, Li Yuan
committed Details | Review

Description Kjartan Maraas 2007-02-01 09:50:24 UTC
Valgrind found this:

==3752== Invalid read of size 4
==3752==    at 0x4A06980: g_list_length (glist.c:481)
==3752==    by 0x4F52C50: gail_item_get_name (gailitem.c:292)
==3752==    by 0x47BAAA8: atk_object_get_name (atkobject.c:572)
==3752==    by 0x4F97A94: spi_atk_bridge_property_event_listener (bridge.c:822)
==3752==    by 0x49BD52D: signal_emit_unlocked_R (gsignal.c:2406)
==3752==    by 0x49BEC76: g_signal_emit_valist (gsignal.c:2199)
==3752==    by 0x49BEE38: g_signal_emit (gsignal.c:2243)
==3752==    by 0x47BBA8D: atk_object_notify (atkobject.c:1342)
==3752==    by 0x49B9758: g_cclosure_marshal_VOID__PARAM (gmarshal.c:531)
==3752==    by 0x49AB4F8: g_type_class_meta_marshal (gclosure.c:567)
==3752==    by 0x49ACD0A: g_closure_invoke (gclosure.c:490)
==3752==    by 0x49BDBE9: signal_emit_unlocked_R (gsignal.c:2370)
==3752==    by 0x49BEC76: g_signal_emit_valist (gsignal.c:2199)
==3752==    by 0x49BEE38: g_signal_emit (gsignal.c:2243)
==3752==    by 0x49B1270: g_object_dispatch_properties_changed (gobject.c:563)
==3752==    by 0x49ADACE: g_object_notify_dispatcher (gobject.c:245)
==3752==    by 0x49B1DB1: g_object_notify (gobjectnotifyqueue.c:123)
==3752==    by 0x47BA2DF: atk_object_set_parent (atkobject.c:901)
==3752==    by 0x4F56887: gail_menu_item_real_initialize (gailmenuitem.c:145)
==3752==    by 0x47BA0A0: atk_object_initialize (atkobject.c:1322)
==3752==    by 0x4F5663C: gail_menu_item_new (gailmenuitem.c:180)
==3752==    by 0x4F3B8E5: gail_menu_item_factory_create_accessible (gail.c:73)
==3752==    by 0x47BC1FF: atk_object_factory_create_accessible (atkobjectfactory.c:85)
==3752==    by 0x4529014: gtk_widget_real_get_accessible (gtkwidget.c:7847)
==3752==    by 0x451E609: gtk_widget_get_accessible (gtkwidget.c:7829)
==3752==    by 0x4F56B7A: gail_menu_item_ref_child (gailmenuitem.c:269)
==3752==    by 0x47BA8AF: atk_object_ref_accessible_child (atkobject.c:668)
==3752==    by 0x4F9761B: spi_atk_bridge_signal_listener (bridge.c:1122)
==3752==    by 0x49BD52D: signal_emit_unlocked_R (gsignal.c:2406)
==3752==    by 0x49BEC76: g_signal_emit_valist (gsignal.c:2199)
==3752==  Address 0x5D044EC is 4 bytes inside a block of size 12 free'd
==3752==    at 0x4004FDA: free (vg_replace_malloc.c:233)
==3752==    by 0x4A103B0: g_free (gmem.c:187)
==3752==    by 0x4A204EF: g_slice_free_chain_with_offset (gslice.c:890)
==3752==    by 0x4A07150: g_list_free (glist.c:53)
==3752==    by 0x4F52C3A: gail_item_get_name (gailitem.c:290)
==3752==    by 0x47BAAA8: atk_object_get_name (atkobject.c:572)
==3752==    by 0x4F97A94: spi_atk_bridge_property_event_listener (bridge.c:822)
==3752==    by 0x49BD52D: signal_emit_unlocked_R (gsignal.c:2406)
==3752==    by 0x49BEC76: g_signal_emit_valist (gsignal.c:2199)
==3752==    by 0x49BEE38: g_signal_emit (gsignal.c:2243)
==3752==    by 0x47BBA8D: atk_object_notify (atkobject.c:1342)
==3752==    by 0x49B9758: g_cclosure_marshal_VOID__PARAM (gmarshal.c:531)
==3752==    by 0x49AB4F8: g_type_class_meta_marshal (gclosure.c:567)
==3752==    by 0x49ACD0A: g_closure_invoke (gclosure.c:490)
==3752==    by 0x49BDBE9: signal_emit_unlocked_R (gsignal.c:2370)
==3752==    by 0x49BEC76: g_signal_emit_valist (gsignal.c:2199)
==3752==    by 0x49BEE38: g_signal_emit (gsignal.c:2243)
==3752==    by 0x49B1270: g_object_dispatch_properties_changed (gobject.c:563)
==3752==    by 0x49ADACE: g_object_notify_dispatcher (gobject.c:245)
==3752==    by 0x49B1DB1: g_object_notify (gobjectnotifyqueue.c:123)
==3752==    by 0x47BA2DF: atk_object_set_parent (atkobject.c:901)
==3752==    by 0x4F56887: gail_menu_item_real_initialize (gailmenuitem.c:145)
==3752==    by 0x47BA0A0: atk_object_initialize (atkobject.c:1322)
==3752==    by 0x4F5663C: gail_menu_item_new (gailmenuitem.c:180)
==3752==    by 0x4F3B8E5: gail_menu_item_factory_create_accessible (gail.c:73)
==3752==    by 0x47BC1FF: atk_object_factory_create_accessible (atkobjectfactory.c:85)
==3752==    by 0x4529014: gtk_widget_real_get_accessible (gtkwidget.c:7847)
==3752==    by 0x451E609: gtk_widget_get_accessible (gtkwidget.c:7829)
==3752==    by 0x4F56B7A: gail_menu_item_ref_child (gailmenuitem.c:269)
==3752==    by 0x47BA8AF: atk_object_ref_accessible_child (atkobject.c:668)

Attaching a patch that may be the right solution.
Comment 1 Kjartan Maraas 2007-02-01 09:51:14 UTC
Created attachment 81644 [details] [review]
patch to fix the problem
Comment 2 Li Yuan 2007-02-01 10:08:13 UTC
Oh, thanks for reporting this. Is the new patch OK to you?
Comment 3 Li Yuan 2007-02-01 10:08:41 UTC
Created attachment 81647 [details] [review]
new patch
Comment 4 Kjartan Maraas 2007-02-01 10:34:25 UTC
It should be since list isn't dereferenced below that point.
Comment 5 Kjartan Maraas 2007-02-01 10:35:48 UTC
Btw, I found this leak too in the same file:

==3891== 610 (574 direct, 36 indirect) bytes in 27 blocks are definitely lost in loss record 139 of 204
==3891==    at 0x40053C0: malloc (vg_replace_malloc.c:149)
==3891==    by 0x4B38545: g_malloc (gmem.c:131)
==3891==    by 0x4B4B958: g_strdup (gstrfuncs.c:91)
==3891==    by 0x4AF6FE4: g_value_set_string (gvaluetypes.c:752)
==3891==    by 0x45F8377: _gtk_tree_data_list_node_to_value (gtktreedatalist.c:168)
==3891==    by 0x451DCFD: gtk_list_store_get_value (gtkliststore.c:472)
==3891==    by 0x45FCD74: gtk_tree_model_get_value (gtktreemodel.c:1143)
==3891==    by 0x4F77DA8: gail_item_get_name (gailitem.c:315)
==3891==    by 0x47BCAA8: atk_object_get_name (atkobject.c:572)
==3891==    by 0x4FBCA94: spi_atk_bridge_property_event_listener (bridge.c:822)
==3891==    by 0x4AE152D: signal_emit_unlocked_R (gsignal.c:2406)
==3891==    by 0x4AE2C76: g_signal_emit_valist (gsignal.c:2199)
==3891==    by 0x4AE2E38: g_signal_emit (gsignal.c:2243)
==3891==    by 0x47BDA8D: atk_object_notify (atkobject.c:1342)
==3891==    by 0x4ADD758: g_cclosure_marshal_VOID__PARAM (gmarshal.c:531)
==3891==    by 0x4ACF4F8: g_type_class_meta_marshal (gclosure.c:567)
==3891==    by 0x4AD0D0A: g_closure_invoke (gclosure.c:490)
==3891==    by 0x4AE1BE9: signal_emit_unlocked_R (gsignal.c:2370)
==3891==    by 0x4AE2C76: g_signal_emit_valist (gsignal.c:2199)
==3891==    by 0x4AE2E38: g_signal_emit (gsignal.c:2243)
==3891==    by 0x4AD5270: g_object_dispatch_properties_changed (gobject.c:563)
==3891==    by 0x4AD1ACE: g_object_notify_dispatcher (gobject.c:245)
==3891==    by 0x4AD5DB1: g_object_notify (gobjectnotifyqueue.c:123)
==3891==    by 0x47BC2DF: atk_object_set_parent (atkobject.c:901)
==3891==    by 0x4F7B887: gail_menu_item_real_initialize (gailmenuitem.c:145)
==3891==    by 0x47BC0A0: atk_object_initialize (atkobject.c:1322)
==3891==    by 0x4F7B63C: gail_menu_item_new (gailmenuitem.c:180)
==3891==    by 0x4F608E5: gail_menu_item_factory_create_accessible (gail.c:73)
==3891==    by 0x47BE1FF: atk_object_factory_create_accessible (atkobjectfactory.c:85)
==3891==    by 0x4644014: gtk_widget_real_get_accessible (gtkwidget.c:7847)

Should be fixed by this:

@@ -316,7 +317,8 @@ gail_item_get_name (AtkObject *obj)
                                if (G_VALUE_HOLDS_STRING (&value))
                                  {
                                   g_free (item->text);
-                                   item->text =  (gchar *) g_value_dup_string (&value);
+                                   item->text = g_value_dup_string (&value);
+                                   g_value_unset (&value);
                                    break;
                                  }
                             }
Comment 6 Li Yuan 2007-02-01 10:56:50 UTC
We don't use that list after free it.
Comment 7 Kjartan Maraas 2007-02-01 11:25:37 UTC
Yeah, that's what I meant. It's not used below the place it's freed so your patch should be ok, and probably a lot easier to read too :-) Does the leak fix above look ok to you?
Comment 8 Li Yuan 2007-02-01 12:04:21 UTC
Yes. Committed. Thanks!
Comment 9 Li Yuan 2007-02-07 08:32:17 UTC
*** Bug 404664 has been marked as a duplicate of this bug. ***