From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30655 invoked by alias); 15 Apr 2014 09:29:01 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 30554 invoked by uid 89); 15 Apr 2014 09:29:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=no version=3.3.2 X-HELO: hera.aquilenet.fr Received: from hera.aquilenet.fr (HELO hera.aquilenet.fr) (141.255.128.1) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 15 Apr 2014 09:28:57 +0000 Received: from localhost (localhost [127.0.0.1]) by hera.aquilenet.fr (Postfix) with ESMTP id 8BC1BAAC; Tue, 15 Apr 2014 11:28:53 +0200 (CEST) Received: from hera.aquilenet.fr ([127.0.0.1]) by localhost (hera.aquilenet.fr [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 4abxlEXWvjn9; Tue, 15 Apr 2014 11:28:53 +0200 (CEST) Received: from pluto (pluto.bordeaux.inria.fr [193.50.110.57]) by hera.aquilenet.fr (Postfix) with ESMTPSA id 44F8125B; Tue, 15 Apr 2014 11:28:53 +0200 (CEST) From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) To: Doug Evans Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 9/9] Remove a useless Guile finalizer References: <1397060028-18158-1-git-send-email-wingo@igalia.com> <1397060028-18158-10-git-send-email-wingo@igalia.com> <87tx9wjejo.fsf@gnu.org> X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: 26 Germinal an 222 de la =?utf-8?Q?R=C3=A9volution?= X-PGP-Key-ID: 0xEA52ECF4 X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 83C4 F8E5 10A3 3B4C 5BEA D15D 77DD 95E2 EA52 ECF4 X-OS: x86_64-unknown-linux-gnu Date: Tue, 15 Apr 2014 09:29:00 -0000 In-Reply-To: (Doug Evans's message of "Mon, 14 Apr 2014 14:39:00 -0700") Message-ID: <87sipf9c17.fsf@gnu.org> User-Agent: Gnus/5.130009 (Ma Gnus v0.9) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SW-Source: 2014-04/txt/msg00289.txt.bz2 Doug Evans skribis: > ludo@gnu.org (Ludovic Court=C3=A8s) writes: > >> Doug Evans skribis: >> >>> Andy Wingo 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) >>>> =0C >>>> /* Administrivia for sal (symtab-and-line) smobs. */ >>>>=20=20 >>>> -/* The smob "free" function for . */ >>>> - >>>> -static size_t >>>> -stscm_free_sal_smob (SCM self) >>>> -{ >>>> - sal_smob *s_smob =3D (sal_smob *) SCM_SMOB_DATA (self); >>>> - >>>> - /* Not necessary, done to catch bugs. */ >>>> - s_smob->symtab_scm =3D SCM_UNDEFINED; >>>> - >>>> - return 0; >>>> -} >>>> - >>>> /* The smob "print" function for . */ >>>>=20=20 >>>> static int >>>> @@ -692,7 +679,6 @@ gdbscm_initialize_symtabs (void) >>>> scm_set_smob_print (symtab_smob_tag, stscm_print_symtab_smob); >>>>=20=20 >>>> sal_smob_tag =3D 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); >>>>=20=20 >>>> gdbscm_define_functions (symtab_functions, 1); >>> >>> How useful is valgrind with Guile's GC? >> >> I think there=E2=80=99s a suppression file for libgc floating around, bu= t I >> haven=E2=80=99t 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=E2=80=99t help much to have finalizers (free functions) lik= e this. >> For debugging, you could always use a guardian, which has the same effec= t. > > 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. Why do you say it =E2=80=9Crequires=E2=80=9D a free function? The comment = says =E2=80=9CNot necessary, done to catch bugs.=E2=80=9D >>> 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=E2=80=99t think it provides any significant protection or d= ebugging >> 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. Can a =E2=80=98sal_smob=E2=80=99 be referred to by GDB objects once the cor= responding SMOB has been reclaimed? My impression is that the answer is =E2=80=9Cno.= =E2=80=9D Thus I=E2=80=99m not sure what you mean by =E2=80=9Caccessed from outside S= cheme.=E2=80=9D > It's an open question, I guess, whether modifying the smob like this in > a finalizer could confuse gc. No problem with that. Thanks, Ludo=E2=80=99.