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


ludo@gnu.org (Ludovic CourtÃs) writes:

> Doug Evans <xdje42@gmail.com> skribis:
>
>> 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?
>
> I think thereâs a suppression file for libgc floating around, but I
> havenât really used it myself.
>
>> 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.
>
> IMO it doesnât help much to have finalizers (free functions) like this.
> For debugging, you could always use a guardian, which has the same effect.

Ah.  I probably didn't make myself clear.  Sorry 'bout that.
For smobs that are only ever accessed from Scheme then sure,
no disagreement there.

For smobs accessed from gdb ... that's the context in which my
comments were made.  Alas, any such object probably requires a free
function anyway.

>> 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).
>
> Again, I donât think it provides any significant protection or debugging
> aid.

I'm going to keep my NULL'ing out of smob elements in free functions
(I see I'm missing a few), but only for such smobs that can be accessed
from outside Scheme.
It's an open question, I guess, whether modifying the smob like this in
a finalizer could confuse gc.


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