Re: [bug-gettext] libgettextpo: memory leak with po_file_read/po_file_free

[Alexander Potashev]

2011/4/12 Alexander Potashev <address@hidden>:
> If I call po_file_read and po_file_free multiple times, my program
> consumes more and more memory.

Now I removed the loop and tested the program using Valgrind. Here is
the valgrind log:

~/libgettextpo-test-leak $ valgrind --track-origins=yes --leak-check=full ./test
==19606== Memcheck, a memory error detector
==19606== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==19606== Using Valgrind-3.6.1 and LibVEX; rerun with -h for copyright info
==19606== Command: ./test
==19606==
==19606== Conditional jump or move depends on uninitialised value(s)
==19606==    at 0x4E5E23C: libgettextpo_freea (malloca.c:115)
==19606==    by 0x4E4004F: libgettextpo_po_lex_charset_set (po-charset.c:631)
==19606==    by 0x4E43147: libgettextpo_po_gram_parse (po-gram-gen.y:105)
==19606==    by 0x4E43186: po_parse (read-po.c:40)
==19606==    by 0x4E43D61: libgettextpo_catalog_reader_parse
(read-catalog-abstract.c:175)
==19606==    by 0x4E43F50: libgettextpo_read_catalog_stream (read-catalog.c:492)
==19606==    by 0x4E3A5E5: po_file_read_v3 (gettext-po.c:124)
==19606==    by 0x400853: main (main.cpp:29)
==19606==  Uninitialised value was created by a stack allocation
==19606==    at 0x4E3FF60: libgettextpo_po_lex_charset_set (po-charset.c:465)
==19606==
==19606== Conditional jump or move depends on uninitialised value(s)
==19606==    at 0x4E5E23C: libgettextpo_freea (malloca.c:115)
==19606==    by 0x4E3B07A: libgettextpo_message_list_search (message.c:515)
==19606==    by 0x4E441B8: libgettextpo_default_add_message (read-catalog.c:363)
==19606==    by 0x4E4470E: libgettextpo_default_directive_message
(read-catalog.c:63)
==19606==    by 0x4E43232: libgettextpo_po_callback_message
(read-catalog-abstract.c:105)
==19606==    by 0x4E42FB0: libgettextpo_po_gram_parse (po-gram-gen.y:107)
==19606==    by 0x4E43186: po_parse (read-po.c:40)
==19606==    by 0x4E43D61: libgettextpo_catalog_reader_parse
(read-catalog-abstract.c:175)
==19606==    by 0x4E43F50: libgettextpo_read_catalog_stream (read-catalog.c:492)
==19606==    by 0x4E3A5E5: po_file_read_v3 (gettext-po.c:124)
==19606==    by 0x400853: main (main.cpp:29)
==19606==  Uninitialised value was created by a stack allocation
==19606==    at 0x4E3B004: libgettextpo_message_list_search (message.c:495)
==19606==
==19606== Conditional jump or move depends on uninitialised value(s)
==19606==    at 0x4E5E23C: libgettextpo_freea (malloca.c:115)
==19606==    by 0x4E3B494: message_list_hash_insert_entry (message.c:312)
==19606==    by 0x4E3B733: libgettextpo_message_list_append (message.c:332)
==19606==    by 0x4E4470E: libgettextpo_default_directive_message
(read-catalog.c:63)
==19606==    by 0x4E43232: libgettextpo_po_callback_message
(read-catalog-abstract.c:105)
==19606==    by 0x4E42FB0: libgettextpo_po_gram_parse (po-gram-gen.y:107)
==19606==    by 0x4E43186: po_parse (read-po.c:40)
==19606==    by 0x4E43D61: libgettextpo_catalog_reader_parse
(read-catalog-abstract.c:175)
==19606==    by 0x4E43F50: libgettextpo_read_catalog_stream (read-catalog.c:492)
==19606==    by 0x4E3A5E5: po_file_read_v3 (gettext-po.c:124)
==19606==    by 0x400853: main (main.cpp:29)
==19606==  Uninitialised value was created by a stack allocation
==19606==    at 0x4E3B426: message_list_hash_insert_entry (message.c:294)
==19606==
==19606==
==19606== HEAP SUMMARY:
==19606==     in use at exit: 57,741 bytes in 2,629 blocks
==19606==   total heap usage: 9,766 allocs, 7,137 frees, 617,368 bytes allocated
==19606==
==19606== 727 bytes in 8 blocks are definitely lost in loss record 4 of 6
==19606==    at 0x4C267BE: malloc (vg_replace_malloc.c:236)
==19606==    by 0x4E62788: libgettextpo_xmalloc (xmalloc.c:64)
==19606==    by 0x4E3A928: libgettextpo_string_list_concat (str-list.c:157)
==19606==    by 0x4E3AA3C: libgettextpo_string_list_concat_destroy
(str-list.c:185)
==19606==    by 0x4E423D9: libgettextpo_po_gram_parse (po-gram-gen.y:338)
==19606==    by 0x4E43186: po_parse (read-po.c:40)
==19606==    by 0x4E43D61: libgettextpo_catalog_reader_parse
(read-catalog-abstract.c:175)
==19606==    by 0x4E43F50: libgettextpo_read_catalog_stream (read-catalog.c:492)
==19606==    by 0x4E3A5E5: po_file_read_v3 (gettext-po.c:124)
==19606==    by 0x400853: main (main.cpp:29)
==19606==
==19606== 9,874 bytes in 575 blocks are definitely lost in loss record 5 of 6
==19606==    at 0x4C267BE: malloc (vg_replace_malloc.c:236)
==19606==    by 0x4E62788: libgettextpo_xmalloc (xmalloc.c:64)
==19606==    by 0x4E62871: libgettextpo_xstrdup (xstrdup.c:39)
==19606==    by 0x4E3AB45: libgettextpo_string_list_append (str-list.c:77)
==19606==    by 0x4E42117: libgettextpo_po_gram_parse (po-gram-gen.y:431)
==19606==    by 0x4E43186: po_parse (read-po.c:40)
==19606==    by 0x4E43D61: libgettextpo_catalog_reader_parse
(read-catalog-abstract.c:175)
==19606==    by 0x4E43F50: libgettextpo_read_catalog_stream (read-catalog.c:492)
==19606==    by 0x4E3A5E5: po_file_read_v3 (gettext-po.c:124)
==19606==    by 0x400853: main (main.cpp:29)
==19606==
==19606== 46,914 bytes in 2,043 blocks are definitely lost in loss record 6 of 6
==19606==    at 0x4C267BE: malloc (vg_replace_malloc.c:236)
==19606==    by 0x4E62788: libgettextpo_xmalloc (xmalloc.c:64)
==19606==    by 0x4E62871: libgettextpo_xstrdup (xstrdup.c:39)
==19606==    by 0x4E41516: libgettextpo_po_gram_lex (po-lex.c:1011)
==19606==    by 0x4E4206E: libgettextpo_po_gram_parse (po-gram-gen.c:1421)
==19606==    by 0x4E43186: po_parse (read-po.c:40)
==19606==    by 0x4E43D61: libgettextpo_catalog_reader_parse
(read-catalog-abstract.c:175)
==19606==    by 0x4E43F50: libgettextpo_read_catalog_stream (read-catalog.c:492)
==19606==    by 0x4E3A5E5: po_file_read_v3 (gettext-po.c:124)
==19606==    by 0x400853: main (main.cpp:29)
==19606==
==19606== LEAK SUMMARY:
==19606==    definitely lost: 57,515 bytes in 2,626 blocks
==19606==    indirectly lost: 0 bytes in 0 blocks
==19606==      possibly lost: 0 bytes in 0 blocks
==19606==    still reachable: 226 bytes in 3 blocks
==19606==         suppressed: 0 bytes in 0 blocks
==19606== Reachable blocks (those to which a pointer was found) are not shown.
==19606== To see them, rerun with: --leak-check=full --show-reachable=yes
==19606==
==19606== For counts of detected and suppressed errors, rerun with: -v
==19606== ERROR SUMMARY: 1156 errors from 6 contexts (suppressed: 4 from 4)

The leak is 100% reproducible. And it is huge: 57 KB for a 93 KB
translation file. The new source code of testcase (without the loop)
is attached. You can download the file dolphin.po from
http://websvn.kde.org/*checkout*/trunk/l10n-kde4/ru/messages/kdebase/dolphin.po
or use any other .po file (I could not even find any translation file
that would not cause a leak).

I tried to debug the leak a little bit and found out that the test
program works without the call to "xstrdup" at
gettext-tools/src/po-lex.c:1011:

diff --git a/gettext-tools/src/po-lex.c b/gettext-tools/src/po-lex.c
@@ -1008,7 +1008,7 @@ po_gram_lex ()
                                      _("context separator <EOT>
within string"));

             /* FIXME: Treatment of embedded \000 chars is incorrect.  */
-            po_gram_lval.string.string = xstrdup (buf);
+            po_gram_lval.string.string = buf;
             po_gram_lval.string.pos = gram_pos;
             po_gram_lval.string.obsolete = po_lex_obsolete;
             return (po_lex_previous ? PREV_STRING : STRING);


Please, don't complain about data corruptions if you use this patch,
because I did almost no testing. I'm almost sure that this hack will
fail when you open two .po files simultaneously.


-- 
Alexander Potashev

main.cpp
Description: Text Data