FYI: remove free_symtab

FYI: remove free_symtab

[Tom Tromey]

I'm checking this in on the trunk.

After the preceding patches, it is obvious that `free_symtab' is dead:
it had no callers in the tree, but now it is clear that it isn't needed
even in theory.  So, this patch removes it.

Currently, nothing frees the `line_charpos' and `fullname' fields of
symtabs in a dying objfile.  This patch clears up this memory leak by
freeing these from free_objfile.

Built and regtested on x86-64 (compile farm).

Tom

2011-04-04  Tom Tromey  <tromey@redhat.com>

	* symtab.h (free_symtab): Remove.
	(forget_cached_source_info_for_objfile): Declare.
	* symmisc.c (free_symtab): Remove.
	* source.c (forget_cached_source_info_for_objfile): New function.
	(forget_cached_source_info): Use it.
	* objfiles.c (free_objfile): Simplify check before calling
	clear_current_source_symtab_and_line.  Call
	forget_cached_source_info_for_objfile.

Index: objfiles.c
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.c,v
retrieving revision 1.128
diff -u -r1.128 objfiles.c
--- objfiles.c	4 Apr 2011 14:37:16 -0000	1.128
+++ objfiles.c	4 Apr 2011 15:17:54 -0000
@@ -632,15 +632,13 @@
 
   {
     struct symtab_and_line cursal = get_current_source_symtab_and_line ();
-    struct symtab *s;
 
-    ALL_OBJFILE_SYMTABS (objfile, s)
-      {
-	if (s == cursal.symtab)
-	  clear_current_source_symtab_and_line ();
-      }
+    if (cursal.symtab && cursal.symtab->objfile == objfile)
+      clear_current_source_symtab_and_line ();
   }
 
+  forget_cached_source_info_for_objfile (objfile);
+
   /* The last thing we do is free the objfile struct itself.  */
 
   xfree (objfile->name);
Index: source.c
===================================================================
RCS file: /cvs/src/src/gdb/source.c,v
retrieving revision 1.122
diff -u -r1.122 source.c
--- source.c	4 Apr 2011 14:18:11 -0000	1.122
+++ source.c	4 Apr 2011 15:17:55 -0000
@@ -335,6 +335,32 @@
   show_directories_1 (NULL, from_tty);
 }
 
+/* Forget line positions and file names for the symtabs in a
+   particular objfile.  */
+
+void
+forget_cached_source_info_for_objfile (struct objfile *objfile)
+{
+  struct symtab *s;
+
+  ALL_OBJFILE_SYMTABS (objfile, s)
+    {
+      if (s->line_charpos != NULL)
+	{
+	  xfree (s->line_charpos);
+	  s->line_charpos = NULL;
+	}
+      if (s->fullname != NULL)
+	{
+	  xfree (s->fullname);
+	  s->fullname = NULL;
+	}
+
+      if (objfile->sf)
+	objfile->sf->qf->forget_cached_source_info (objfile);
+    }
+}
+
 /* Forget what we learned about line positions in source files, and
    which directories contain them; must check again now since files
    may be found in a different directory now.  */
@@ -349,22 +375,7 @@
   ALL_PSPACES (pspace)
     ALL_PSPACE_OBJFILES (pspace, objfile)
     {
-      for (s = objfile->symtabs; s != NULL; s = s->next)
-	{
-	  if (s->line_charpos != NULL)
-	    {
-	      xfree (s->line_charpos);
-	      s->line_charpos = NULL;
-	    }
-	  if (s->fullname != NULL)
-	    {
-	      xfree (s->fullname);
-	      s->fullname = NULL;
-	    }
-	}
-
-      if (objfile->sf)
-	objfile->sf->qf->forget_cached_source_info (objfile);
+      forget_cached_source_info_for_objfile (objfile);
     }
 
   last_source_visited = NULL;
Index: symmisc.c
===================================================================
RCS file: /cvs/src/src/gdb/symmisc.c,v
retrieving revision 1.79
diff -u -r1.79 symmisc.c
--- symmisc.c	4 Apr 2011 15:11:19 -0000	1.79
+++ symmisc.c	4 Apr 2011 15:17:55 -0000
@@ -79,18 +79,6 @@
 
 static int print_symbol (void *);
 \f
-/* Free all the storage associated with the struct symtab <- S.  */
-
-void
-free_symtab (struct symtab *s)
-{
-  /* Free source-related stuff.  */
-  if (s->line_charpos != NULL)
-    xfree (s->line_charpos);
-  if (s->fullname != NULL)
-    xfree (s->fullname);
-  xfree (s);
-}
 
 void
 print_symbol_bcache_statistics (void)
Index: symtab.h
===================================================================
RCS file: /cvs/src/src/gdb/symtab.h,v
retrieving revision 1.176
diff -u -r1.176 symtab.h
--- symtab.h	4 Apr 2011 15:11:19 -0000	1.176
+++ symtab.h	4 Apr 2011 15:17:55 -0000
@@ -1165,8 +1165,6 @@
 
 void maintenance_print_statistics (char *, int);
 
-extern void free_symtab (struct symtab *);
-
 /* Symbol-reading stuff in symfile.c and solib.c.  */
 
 extern void clear_solib (void);
@@ -1177,6 +1175,7 @@
 
 extern void print_source_lines (struct symtab *, int, int, int);
 
+extern void forget_cached_source_info_for_objfile (struct objfile *);
 extern void forget_cached_source_info (void);
 
 extern void select_source_symtab (struct symtab *);

[patch] Regression: Re: FYI: remove free_symtab

[Jan Kratochvil]

Hi Tom,

heavy crash regression on Fedora 15, in fact on any system using .gdb_index.
One of such testfiles is for example gdb.base/cursal.exp.

3405069491ed7249b28958123166f160d7fe7fcb is the first bad commit
commit 3405069491ed7249b28958123166f160d7fe7fcb
Author: Tom Tromey <tromey@redhat.com>
Date:   Mon Apr 4 15:19:54 2011 +0000

        * symtab.h (free_symtab): Remove.
        (forget_cached_source_info_for_objfile): Declare.
        * symmisc.c (free_symtab): Remove.
        * source.c (forget_cached_source_info_for_objfile): New function.
        (forget_cached_source_info): Use it.
        * objfiles.c (free_objfile): Simplify check before calling
        clear_current_source_symtab_and_line.  Call
        forget_cached_source_info_for_objfile.

:040000 040000 be020dc3c59fd93e1e2dd2372227733c585a9e83 2ab4f20b502413ce56eafe87828ae10f7f7f1ee8 M      gdb
bisect run success

Program received signal SIGSEGV, Segmentation fault.
0x0000000000488b0e in objfile_data (objfile=0x2128400, data=0x1e25a20) at objfiles.c:1480
1480      return objfile->data[data->index];
(gdb) p *objfile
$1 = {next = 0x0, name = 0x20fdf60 "/usr/lib/debug/lib64/ld-2.13.90.so.debug",

objfile->data is already NULL.

#0  in objfile_data (objfile=0x2128400, data=0x1e25a20) at objfiles.c:1480
#1  in dw2_setup (objfile=0x2128400) at dwarf2read.c:2139
#2  in dw2_forget_cached_source_info (objfile=0x2128400) at dwarf2read.c:2308
#3  in forget_cached_source_info_for_objfile (objfile=0x2128400) at source.c:360
#4  in free_objfile (objfile=0x2128400) at objfiles.c:640
#5  in objfile_purge_solibs () at objfiles.c:988
#6  in no_shared_libraries (ignored=0x0, from_tty=0) at solib.c:1308
#7  in target_pre_inferior (from_tty=0) at target.c:2390
#8  in run_command_1 (args=0x0, from_tty=0, tbreak_at_main=0) at infcmd.c:506
#9  in run_command (args=0x0, from_tty=0) at infcmd.c:610

I do not see any deep design in the freeing order so I have just reshuffled it
so that it started working again.

No regressions on {x86_64,x86_64-m32,i686}-fedora{13,14,15,-rawhide}-linux-gnu
together with the debugformat (as has been posted along) fix against yesterday.

I will check it in today.


Thanks,
Jan


gdb/
2011-04-05  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix crash regression on systems featuring .gdb_index.
	* objfiles.c (free_objfile): Move the
	forget_cached_source_info_for_objfile call earlier.  Comment it.
	Extend the comment for objfile_free_data.

--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -583,6 +583,10 @@ free_objfile (struct objfile *objfile)
      lists.  */
   preserve_values (objfile);
 
+  /* It still may reference data modules have associated with the objfile and
+     the symbol file data.  */
+  forget_cached_source_info_for_objfile (objfile);
+
   /* First do any symbol file specific actions required when we are
      finished with a particular symbol file.  Note that if the objfile
      is using reusable symbol information (via mmalloc) then each of
@@ -595,7 +599,8 @@ free_objfile (struct objfile *objfile)
       (*objfile->sf->sym_finish) (objfile);
     }
 
-  /* Discard any data modules have associated with the objfile.  */
+  /* Discard any data modules have associated with the objfile.  The function
+     still may reference objfile->obfd.  */
   objfile_free_data (objfile);
 
   gdb_bfd_unref (objfile->obfd);
@@ -637,8 +642,6 @@ free_objfile (struct objfile *objfile)
       clear_current_source_symtab_and_line ();
   }
 
-  forget_cached_source_info_for_objfile (objfile);
-
   /* The last thing we do is free the objfile struct itself.  */
 
   xfree (objfile->name);

Re: [patch] Regression: Re: FYI: remove free_symtab

[Jan Kratochvil]

On Tue, 05 Apr 2011 10:51:28 +0200, Jan Kratochvil wrote:
> gdb/
> 2011-04-05  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Fix crash regression on systems featuring .gdb_index.
> 	* objfiles.c (free_objfile): Move the
> 	forget_cached_source_info_for_objfile call earlier.  Comment it.
> 	Extend the comment for objfile_free_data.

Checked in:
	http://sourceware.org/ml/gdb-cvs/2011-04/msg00041.html


Thanks,
Jan