Re: [RFA] Fix leak in the dwarf reader

[Simon Marchi <simark@simark.ca>]

On 11/27/22 07:54, Philippe Waroquiers via Gdb-patches wrote:
> Valgrind reports a leak in the dwarf reader (see details below).
> The function dw2_get_file_names_reader is interning in the per_objfile
> all the file names it finds, except the name of 'fnd file name and directory'.
> Instead, it was xstrdup-ing the name.
> Fix the leaks by also interning the name.
> 
> This was validated running the tests natively, and under valgrind.
> Leaks have decreased as mentionned below.
> Valgrind detected no error such as double free or use after free.

I manually checked the uses of this field, it indeed doesn't appear to
be freed, so:`

Approved-By: Simon Marchi <simon.marchi@efficios.com>

> 
> Stack trace of the leak:
> ==4113266== 490,735 bytes in 17,500 blocks are definitely lost in loss record 7,061 of 7,074
> ==4113266==    at 0x483979B: malloc (vg_replace_malloc.c:393)
> ==4113266==    by 0x25A454: xmalloc (alloc.c:57)
> ==4113266==    by 0x7D1E1E: xstrdup (xstrdup.c:34)
> ==4113266==    by 0x39D141: dw2_get_file_names_reader (read.c:2825)
> ==4113266==    by 0x39D141: dw2_get_file_names(dwarf2_per_cu_data*, dwarf2_per_objfile*) (read.c:2851)
> ==4113266==    by 0x39DD6C: dw_expand_symtabs_matching_file_matcher(dwarf2_per_objfile*, gdb::function_view<bool (char const*, bool)>) (read.c:4149)
> ==4113266==    by 0x3BC8B5: cooked_index_functions::expand_symtabs_matching(objfile*, gdb::function_view<bool (char const*, bool)>, lookup_name_info const*, gdb::function_view<bool (char const*)>, gdb::function_view<bool (compunit_symtab*)>, enum_flags<block_search_flag_values>, domain_enum, search_domain) (read.c:18688)
> ==4113266==    by 0x5DD1EA: objfile::map_symtabs_matching_filename(char const*, char const*, gdb::function_view<bool (symtab*)>) (symfile-debug.c:207)
> ==4113266==    by 0x5F04CC: iterate_over_symtabs(char const*, gdb::function_view<bool (symtab*)>) (symtab.c:633)
> ==4113266==    by 0x477EE3: collect_symtabs_from_filename(char const*, program_space*) (linespec.c:3712)
> ==4113266==    by 0x477FC1: symtabs_from_filename(char const*, program_space*) (linespec.c:3726)
> ==4113266==    by 0x47A9B8: convert_explicit_location_spec_to_linespec(linespec_state*, linespec*, char const*, char const*, symbol_name_match_type, char const*, line_offset) (linespec.c:2329)
> ==4113266==    by 0x47E86E: convert_explicit_location_spec_to_sals (linespec.c:2388)
> ==4113266==    by 0x47E86E: location_spec_to_sals(linespec_parser*, location_spec const*) (linespec.c:3104)
> ==4113266==    by 0x47EDAC: decode_line_full(location_spec*, int, program_space*, symtab*, int, linespec_result*, char const*, char const*) (linespec.c:3149)
> ...
> 
> Without the fix, the top 10 leaks are:
> ./gdb/testsuite/outputs/gdb.base/condbreak-bad/gdb.log:345:==3213924==    definitely lost: 130,937 bytes in 5,409 blocks
> ./gdb/testsuite/outputs/gdb.base/hbreak2/gdb.log:619:==3758919==    definitely lost: 173,323 bytes in 7,204 blocks
> ./gdb/testsuite/outputs/gdb.mi/mi-var-cp/gdb.log:1320:==4152873==    definitely lost: 172,826 bytes in 7,207 blocks
> ./gdb/testsuite/outputs/gdb.base/advance-until-multiple-locations/gdb.log:398:==2992643==    definitely lost: 172,965 bytes in 7,211 blocks
> ./gdb/testsuite/outputs/gdb.mi/mi-var-cmd/gdb.log:2302:==4159476==    definitely lost: 173,129 bytes in 7,211 blocks
> ./gdb/testsuite/outputs/gdb.cp/gdb2384/gdb.log:222:==3811851==    definitely lost: 218,106 bytes in 7,761 blocks
> ./gdb/testsuite/outputs/gdb.cp/mb-templates/gdb.log:310:==3787344==    definitely lost: 290,311 bytes in 10,340 blocks
> ./gdb/testsuite/outputs/gdb.mi/mi-var-rtti/gdb.log:2568:==4158350==    definitely lost: 435,427 bytes in 15,507 blocks
> ./gdb/testsuite/outputs/gdb.mi/mi-catch-cpp-exceptions/gdb.log:1704:==4119722==    definitely lost: 435,405 bytes in 15,510 blocks
> ./gdb/testsuite/outputs/gdb.mi/mi-vla-fortran/gdb.log:768:==4113266==    definitely lost: 508,585 bytes in 18,109 blocks
> 
> With the fix:
> ./gdb/testsuite/outputs/gdb.base/fork-running-state/gdb.log:1536:==2924193==    indirectly lost: 13,848 bytes in 98 blocks
> ./gdb/testsuite/outputs/gdb.base/fork-running-state/gdb.log:1675:==2928777==    indirectly lost: 13,848 bytes in 98 blocks
> ./gdb/testsuite/outputs/gdb.python/py-inferior-leak/gdb.log:4729:==3353335==    definitely lost: 3,360 bytes in 140 blocks
> ./gdb/testsuite/outputs/gdb.base/kill-detach-inferiors-cmd/gdb.log:210:==2746927==    indirectly lost: 13,246 bytes in 154 blocks
> ./gdb/testsuite/outputs/gdb.base/inferior-clone/gdb.log:179:==3034984==    indirectly lost: 12,921 bytes in 161 blocks
> ./gdb/testsuite/outputs/gdb.base/interrupt-daemon/gdb.log:209:==3006248==    indirectly lost: 20,683 bytes in 174 blocks
> ./gdb/testsuite/outputs/gdb.threads/watchpoint-fork/gdb.log:714:==3512403==    indirectly lost: 20,707 bytes in 175 blocks
> ./gdb/testsuite/outputs/gdb.threads/watchpoint-fork/gdb.log:962:==3514498==    indirectly lost: 20,851 bytes in 178 blocks
> ./gdb/testsuite/outputs/gdb.base/multi-forks/gdb.log:336:==2585839==    indirectly lost: 53,630 bytes in 386 blocks
> ./gdb/testsuite/outputs/gdb.base/multi-forks/gdb.log:1338:==2592417==    indirectly lost: 100,008 bytes in 1,154 blocks

Thanks for checking this, I've been wanting to fix reported leaks for a
really long time, to make any new leak more obvious.  But it's always a
really low prio task.

Simon