[PATCH] Clear current source symtab if belongs to objfile being freed

[PATCH] Clear current source symtab if belongs to objfile being freed

[Fred Fish]

[-- Attachment #1: Type: text/plain, Size: 452 bytes --]

I stumbled over this problem while chasing something else.  If the
current source symtab belongs to a loaded shared library, and the
objfile for that library is freed in the process or rerunning the
executable, the current source symtab can be left pointing to garbage.

An alternative solution, though a little more heavy handed, would be
to replace the previous clear_pc_function_cache() call with a call to
clear_symtab_users().

Comments?

-Fred



[-- Attachment #2: symtab.patch --]
[-- Type: text/x-diff, Size: 2071 bytes --]

2006-01-03  Fred Fish  <fnf@specifix.com>

	* objfiles.c (source.h): Include.
	(free_objfile): Update comment about clear_symtab_users().
	(free_objfile): Check all symtabs of objfile being freed and if
	one of them is the current source symtab, call
 	clear_current_source_symtab_and_line().

Index: objfiles.c
===================================================================
RCS file: /cvsroots/latest/src/gdb/gdb/objfiles.c,v
retrieving revision 1.1.1.2
diff -c -p -r1.1.1.2 objfiles.c
*** objfiles.c	30 Dec 2005 18:53:15 -0000	1.1.1.2
--- objfiles.c	3 Jan 2006 15:57:05 -0000
***************
*** 45,50 ****
--- 45,51 ----
  #include "breakpoint.h"
  #include "block.h"
  #include "dictionary.h"
+ #include "source.h"
  
  /* Prototypes for local functions */
  
*************** free_objfile (struct objfile *objfile)
*** 432,441 ****
       is unknown, but we play it safe for now and keep each action until
       it is shown to be no longer needed. */
  
!   /* I *think* all our callers call clear_symtab_users.  If so, no need
!      to call this here.  */
    clear_pc_function_cache ();
  
    /* The last thing we do is free the objfile struct itself. */
  
    objfile_free_data (objfile);
--- 433,457 ----
       is unknown, but we play it safe for now and keep each action until
       it is shown to be no longer needed. */
  
!   /* I *think* all our callers call clear_symtab_users.  If so, no
!      need to call this here.  Update: We are called via
!      objfile_purge_solibs which doesn't call clear_symtab_users. */
    clear_pc_function_cache ();
  
+   /* Check to see if the current_source_symtab belongs to this objfile,
+      and if so, call clear_current_source_symtab_and_line. */
+ 
+   {
+     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 ();
+       }
+   }
+ 
    /* The last thing we do is free the objfile struct itself. */
  
    objfile_free_data (objfile);

Re: [PATCH] Clear current source symtab if belongs to objfile being freed

[Jim Blandy]

On 1/3/06, Fred Fish <fnf@specifix.com> wrote:
> I stumbled over this problem while chasing something else.  If the
> current source symtab belongs to a loaded shared library, and the
> objfile for that library is freed in the process or rerunning the
> executable, the current source symtab can be left pointing to garbage.
>
> An alternative solution, though a little more heavy handed, would be
> to replace the previous clear_pc_function_cache() call with a call to
> clear_symtab_users().
>
> Comments?

!   /* I *think* all our callers call clear_symtab_users.  If so, no
!      need to call this here.  Update: We are called via
!      objfile_purge_solibs which doesn't call clear_symtab_users. */

I don't think we need our comments to be so archeological.  If we
apply this patch, we should just change it to something direct like:

  /* Not all our callers call clear_symtab_users
(objfile_purge_solibs, for example),
     so we need to call this here.  */

It seems to me, though, that the 'nosharedlibrary' and 'set
solib-absolute-prefix' commands actually ought to be calling
clear_symtab_users, no?  I mean, types and blocks used by displays and
all those things are going to become invalidated, aren't they?

Re: [PATCH] Clear current source symtab if belongs to objfile being freed

[Fred Fish]

On Tuesday 03 January 2006 18:26, Jim Blandy wrote:
> It seems to me, though, that the 'nosharedlibrary' and 'set
> solib-absolute-prefix' commands actually ought to be calling
> clear_symtab_users, no?  I mean, types and blocks used by displays and
> all those things are going to become invalidated, aren't they?

I tried the following patch but it broke a bunch of stuff in the testsuite.

*** objfiles.c  30 Dec 2005 18:53:15 -0000      1.1.1.2
--- objfiles.c  4 Jan 2006 19:11:32 -0000
*************** objfile_purge_solibs (void)
*** 689,694 ****
--- 689,695 ----
    struct objfile *objf;
    struct objfile *temp;

+   clear_symtab_users ();
    ALL_OBJFILES_SAFE (objf, temp)
    {
      /* We assume that the solib package has been purged already, or will

I've not investigated all the failures, but I suppose it's reasonable to
assume that this is overkill.  What if there is a lot of debugging state
set up, none of which references the discarded objfile, and we expect
it to still be set up after discarding the objfile?

The patch I submitted is pretty minimalist in what it affects, only
discarding a reference that gdb knows to be invalidated.  Perhaps there
will be others, but they should probably be dealt with as they are
discovered.

It might be a good idea though to put in some testsuite tests that
specifically check some of the other operations you suspect could 
also be affected.

-Fred

Re: [PATCH] Clear current source symtab if belongs to objfile being freed

[Jim Blandy]

On 1/4/06, Fred Fish <fnf@specifix.com> wrote:
> The patch I submitted is pretty minimalist in what it affects, only
> discarding a reference that gdb knows to be invalidated.  Perhaps there
> will be others, but they should probably be dealt with as they are
> discovered.

Okay.  The original patch is fine; consider the doc change I suggested.

> It might be a good idea though to put in some testsuite tests that
> specifically check some of the other operations you suspect could
> also be affected.

Yes.  I'm pretty confused about why those aren't a problem...

[commit] Clear current source symtab if belongs to objfile being freed

[Fred Fish]

On Thursday 05 January 2006 17:08, Jim Blandy wrote:
> Okay.  The original patch is fine; consider the doc change I suggested.

Done.  Here is the revised patch.

-Fred

 2006-01-06  Fred Fish  <fnf@specifix.com>

	* objfiles.c (source.h): Include.
	(free_objfile): Update comment about clear_symtab_users().
	(free_objfile): Check all symtabs of objfile being freed and if
	one of them is the current source symtab, call
 	clear_current_source_symtab_and_line().

Index: objfiles.c
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.c,v
retrieving revision 1.62
diff -c -p -r1.62 objfiles.c
*** objfiles.c	17 Dec 2005 22:34:01 -0000	1.62
--- objfiles.c	6 Jan 2006 16:22:26 -0000
***************
*** 45,50 ****
--- 45,51 ----
  #include "breakpoint.h"
  #include "block.h"
  #include "dictionary.h"
+ #include "source.h"
  
  /* Prototypes for local functions */
  
*************** free_objfile (struct objfile *objfile)
*** 432,441 ****
       is unknown, but we play it safe for now and keep each action until
       it is shown to be no longer needed. */
  
!   /* I *think* all our callers call clear_symtab_users.  If so, no need
!      to call this here.  */
    clear_pc_function_cache ();
  
    /* The last thing we do is free the objfile struct itself. */
  
    objfile_free_data (objfile);
--- 433,456 ----
       is unknown, but we play it safe for now and keep each action until
       it is shown to be no longer needed. */
  
!   /* Not all our callers call clear_symtab_users (objfile_purge_solibs,
!      for example), so we need to call this here.  */
    clear_pc_function_cache ();
  
+   /* Check to see if the current_source_symtab belongs to this objfile,
+      and if so, call clear_current_source_symtab_and_line. */
+ 
+   {
+     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 ();
+       }
+   }
+ 
    /* The last thing we do is free the objfile struct itself. */
  
    objfile_free_data (objfile);