[RFA] Fix 4K leak in open_source_file each time next/step changes of function.

Message ID 20181104165507.24341-1-philippe.waroquiers@skynet.be
State New, archived
Headers

Commit Message

Philippe Waroquiers Nov. 4, 2018, 4:55 p.m. UTC
  When current function changes after a next/step, GDB shows a message such as:
  (gdb) s
  info_fun1 ()
      at /bd/home/philippe/gdb/git/build_smallthing/gdb/testsuite/../../../smallthing/gdb/testsuite/gdb.base/info_qt.c:41
  41	  info_qt_inc++;
  (gdb)

Valgrind reports a 4K definite leak for each such message (full stacktrace of
the leak below).

This patch fixes this leak, by transferring the current s->fullname to the
unique_xmalloc_ptr fullname given to find_and_open_source.

Note that I do not understand why find_and_open_source always tries to
re-execute the substitution rules on the provided fullname, as source.c
symtab_to_fullname just blindly returns a non NULL s->fullname, counting on
forget_cached_source_info to be called if search dir or substitution rules are
changed.  Similarly, psymtab_to_fullname also just returns a non NULL
ps->fullname.

==15309== VALGRIND_GDB_ERROR_BEGIN
==15309== 69,632 bytes in 17 blocks are definitely lost in loss record 3,158 of 3,186
==15309==    at 0x4C2BE2D: malloc (vg_replace_malloc.c:299)
==15309==    by 0x5BF0987: realpath@@GLIBC_2.3 (canonicalize.c:78)
==15309==    by 0x41F713: gdb_realpath(char const*) (pathstuff.c:72)
==15309==    by 0x608833: openp(char const*, enum_flags<openp_flag>, char const*, int, std::unique_ptr<char, gdb::xfree_deleter<char> >*) (source.c:861)
==15309==    by 0x608B89: find_and_open_source(char const*, char const*, std::unique_ptr<char, gdb::xfree_deleter<char> >*) (source.c:1049)
==15309==    by 0x608D0B: open_source_file(symtab*) (source.c:1074)
==15309==    by 0x609101: print_source_lines_base(symtab*, int, int, enum_flags<print_source_lines_flag>) (source.c:1291)
==15309==    by 0x614ADF: print_frame_info(frame_info*, int, print_what, int, int) (stack.c:911)
==15309==    by 0x614C45: print_stack_frame(frame_info*, int, print_what, int) (stack.c:181)
==15309==    by 0x511D5E: print_stop_location (infrun.c:8044)
==15309==    by 0x511D5E: print_stop_event(ui_out*) (infrun.c:8061)
==15309==    by 0x40DD6D: cli_on_normal_stop(bpstats*, int) (cli-interp.c:145)
==15309==    by 0x512409: operator() (functional:2127)
==15309==    by 0x512409: notify (observable.h:106)
==15309==    by 0x512409: normal_stop() (infrun.c:8334)
==15309==    by 0x5156D8: fetch_inferior_event(void*) (infrun.c:3955)
==15309==    by 0x4B3EEC: gdb_wait_for_event(int) (event-loop.c:859)
==15309==    by 0x4B3FF6: gdb_do_one_event() [clone .part.4] (event-loop.c:322)
==15309==    by 0x4B41B4: gdb_do_one_event (common-exceptions.h:219)
==15309==    by 0x4B41B4: start_event_loop() (event-loop.c:371)
==15309==    by 0x551217: captured_command_loop() (main.c:330)
==15309==    by 0x55220C: captured_main (main.c:1177)
==15309==    by 0x55220C: gdb_main(captured_main_args*) (main.c:1193)
==15309==    by 0x29B4F7: main (gdb.c:32)
==15309==
==15309== VALGRIND_GDB_ERROR_END

gdb/ChangeLog
2018-11-04  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* source.c (open_source_file): Fix leak by transferring the
	current s->fullname to the unique_xmalloc_ptr fullname given
	to find_and_open_source.
---
 gdb/source.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
  

Comments

Tom Tromey Nov. 5, 2018, 8:02 p.m. UTC | #1
>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> Note that I do not understand why find_and_open_source always tries to
Philippe> re-execute the substitution rules on the provided fullname, as source.c
Philippe> symtab_to_fullname just blindly returns a non NULL s->fullname, counting on
Philippe> forget_cached_source_info to be called if search dir or substitution rules are
Philippe> changed.  Similarly, psymtab_to_fullname also just returns a non NULL
Philippe> ps-> fullname.

It would be good to investigate & maybe remove this.

Philippe> 2018-11-04  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

Philippe> 	* source.c (open_source_file): Fix leak by transferring the
Philippe> 	current s->fullname to the unique_xmalloc_ptr fullname given
Philippe> 	to find_and_open_source.

Thanks, this is ok.

Tom
  
Philippe Waroquiers Nov. 5, 2018, 8:45 p.m. UTC | #2
On Mon, 2018-11-05 at 13:02 -0700, Tom Tromey wrote:
> > > > > > "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:
> 
> Philippe> Note that I do not understand why find_and_open_source always tries to
> Philippe> re-execute the substitution rules on the provided fullname, as source.c
> Philippe> symtab_to_fullname just blindly returns a non NULL s->fullname, counting on
> Philippe> forget_cached_source_info to be called if search dir or substitution rules are
> Philippe> changed.  Similarly, psymtab_to_fullname also just returns a non NULL
> Philippe> ps-> fullname.
> 
> It would be good to investigate & maybe remove this.
> 
> Philippe> 2018-11-04  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> Philippe> 	* source.c (open_source_file): Fix leak by transferring the
> Philippe> 	current s->fullname to the unique_xmalloc_ptr fullname given
> Philippe> 	to find_and_open_source.
> 
> Thanks, this is ok.
Thanks for the review, pushed.

I have added on my list of things to do to clarify find_and_open_source
substitution rules apply/re-apply logic.

Philippe
  

Patch

diff --git a/gdb/source.c b/gdb/source.c
index ec0ea3b81e..3cdf0b9ec7 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1070,7 +1070,8 @@  open_source_file (struct symtab *s)
   if (!s)
     return -1;
 
-  gdb::unique_xmalloc_ptr<char> fullname;
+  gdb::unique_xmalloc_ptr<char> fullname (s->fullname);
+  s->fullname = NULL;
   int fd = find_and_open_source (s->filename, SYMTAB_DIRNAME (s), &fullname);
   s->fullname = fullname.release ();
   return fd;