This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 9/9] Remove a useless Guile finalizer


Andy Wingo <wingo@igalia.com> writes:

> * gdb/guile/scm-symtab.c (stscm_free_sal_smob): Remove useless free
>   function.  (This was the only useless free function.)
> ---
>  gdb/guile/scm-symtab.c | 14 --------------
>  1 file changed, 14 deletions(-)
>
> diff --git a/gdb/guile/scm-symtab.c b/gdb/guile/scm-symtab.c
> index 8910973..876bf64 100644
> --- a/gdb/guile/scm-symtab.c
> +++ b/gdb/guile/scm-symtab.c
> @@ -386,19 +386,6 @@ gdbscm_symtab_static_block (SCM self)
>  
>  /* Administrivia for sal (symtab-and-line) smobs.  */
>  
> -/* The smob "free" function for <gdb:sal>.  */
> -
> -static size_t
> -stscm_free_sal_smob (SCM self)
> -{
> -  sal_smob *s_smob = (sal_smob *) SCM_SMOB_DATA (self);
> -
> -  /* Not necessary, done to catch bugs.  */
> -  s_smob->symtab_scm = SCM_UNDEFINED;
> -
> -  return 0;
> -}
> -
>  /* The smob "print" function for <gdb:sal>.  */
>  
>  static int
> @@ -692,7 +679,6 @@ gdbscm_initialize_symtabs (void)
>    scm_set_smob_print (symtab_smob_tag, stscm_print_symtab_smob);
>  
>    sal_smob_tag = gdbscm_make_smob_type (sal_smob_name, sizeof (sal_smob));
> -  scm_set_smob_free (sal_smob_tag, stscm_free_sal_smob);
>    scm_set_smob_print (sal_smob_tag, stscm_print_sal_smob);
>  
>    gdbscm_define_functions (symtab_functions, 1);

How useful is valgrind with Guile's GC?
My experience is that the S/N ratio is just too low, but maybe there's
a canonical suppression file that works well enough.
And given that we have this hook, it seems a shame to just throw out
such useful protections against use-after-free (I'm pretty sure early on
I found one bug with them), especially given the subtleties with GC,
and gdb's extensive need to have references to SCM objects from outside
GC-controlled code.

If we're going to have a rule that such code is disallowed,
there is more such code that needs to be removed besides the above
(grep for "catch bugs").

But is this something we want?
At this stage in gdb+guile's development, I like the protection,
and the cost is within epsilon of zero (to me anyway).
[The debug version of the GC may have similar support, but the
benefit of this is that it's "always on" for negligible cost.]
I'm not opposed to removing the code at some later point after things
are really stable and after there's data that they've stopped being
useful.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]