From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#29066: 26.0.90; crash in gc involving buffer local symbols Date: Tue, 31 Oct 2017 21:10:39 +0200 Message-ID: <83375zt9ow.fsf@gnu.org> References: <83a808tlqp.fsf@gnu.org> <838tfst27n.fsf@gnu.org> <87efpjzv2p.fsf@linux-m68k.org> Reply-To: Eli Zaretskii NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Trace: blaine.gmane.org 1509477136 13739 195.159.176.226 (31 Oct 2017 19:12:16 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 31 Oct 2017 19:12:16 +0000 (UTC) Cc: 29066@debbugs.gnu.org, schwab@linux-m68k.org, mshinwell@janestreet.com To: Valentin Gatien-Baron Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Oct 31 20:12:10 2017 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1e9bxF-0002hf-W4 for geb-bug-gnu-emacs@m.gmane.org; Tue, 31 Oct 2017 20:12:06 +0100 Original-Received: from localhost ([::1]:47033 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1e9bxN-0000vF-EY for geb-bug-gnu-emacs@m.gmane.org; Tue, 31 Oct 2017 15:12:13 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:40073) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1e9bxH-0000v9-49 for bug-gnu-emacs@gnu.org; Tue, 31 Oct 2017 15:12:08 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1e9bxC-0005vQ-Hp for bug-gnu-emacs@gnu.org; Tue, 31 Oct 2017 15:12:07 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:35882) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1e9bxC-0005vK-Dq for bug-gnu-emacs@gnu.org; Tue, 31 Oct 2017 15:12:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1e9bxC-000484-8I for bug-gnu-emacs@gnu.org; Tue, 31 Oct 2017 15:12:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 31 Oct 2017 19:12:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 29066 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 29066-submit@debbugs.gnu.org id=B29066.150947706915804 (code B ref 29066); Tue, 31 Oct 2017 19:12:02 +0000 Original-Received: (at 29066) by debbugs.gnu.org; 31 Oct 2017 19:11:09 +0000 Original-Received: from localhost ([127.0.0.1]:44563 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1e9bwK-00046q-N8 for submit@debbugs.gnu.org; Tue, 31 Oct 2017 15:11:08 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:48475) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1e9bwI-00046J-NQ for 29066@debbugs.gnu.org; Tue, 31 Oct 2017 15:11:07 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1e9bwC-0004kK-8A for 29066@debbugs.gnu.org; Tue, 31 Oct 2017 15:11:01 -0400 Original-Received: from fencepost.gnu.org ([2001:4830:134:3::e]:47816) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1e9bw5-0004fI-9n; Tue, 31 Oct 2017 15:10:53 -0400 Original-Received: from [176.228.60.248] (port=1416 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1e9bw4-0006ze-L4; Tue, 31 Oct 2017 15:10:53 -0400 In-reply-to: (message from Valentin Gatien-Baron on Tue, 31 Oct 2017 10:52:44 -0400) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:139274 Archived-At: > From: Valentin Gatien-Baron > Date: Tue, 31 Oct 2017 10:52:44 -0400 > Cc: Eli Zaretskii , > 29066@debbugs.gnu.org, > Mark Shinwell > > > That was my first attempt, but various macros like SYMBOL_BLV and > > SET_SYMBOL_BLV insist on val.blv being non-NULL. > > SET_SYMBOL_BLV doesn't. And calling SYMBOL_BLV with a freed symbol is a > bug anyway. > > ​SET_SYMBOL_BLV insists that the new value is not NULL, even if it asserts nothing about the current value. > > We do call SYMBOL_BLV after freeing, when we re-sweep the symbol, which is fine because free does > nothing when given NULL, but triggers the assertion​. > > I would do this, to avoid the assertion failure: > > diff --git a/src/alloc.c b/src/alloc.c > index da0c3ad4b3..72550e812b 100644 > --- a/src/alloc.c > +++ b/src/alloc.c > @@ -7030,8 +7030,10 @@ sweep_symbols (void) > { > if (!sym->s.gcmarkbit) > { > - if (sym->s.redirect == SYMBOL_LOCALIZED) > + if (sym->s.redirect == SYMBOL_LOCALIZED && sym->s.val.blv) { > xfree (SYMBOL_BLV (&sym->s)); > + sym->s.val.blv = NULL; > + } > sym->s.next = symbol_free_list; > symbol_free_list = &sym->s; > symbol_free_list->function = Vdead; Thanks, but it makes little sense to me to work around our own assertions this way. Why do we have these macros if we sometimes don't use them? And why do we have the assertions if they sometimes get in the way? > Or changing the redirect type: > > diff --git a/src/alloc.c b/src/alloc.c > index da0c3ad4b3..6966d96c6d 100644 > --- a/src/alloc.c > +++ b/src/alloc.c > @@ -7030,8 +7030,11 @@ sweep_symbols (void) > { > if (!sym->s.gcmarkbit) > { > - if (sym->s.redirect == SYMBOL_LOCALIZED) > + if (sym->s.redirect == SYMBOL_LOCALIZED) { > xfree (SYMBOL_BLV (&sym->s)); > + sym->s.redirect = SYMBOL_PLAINVAL; > + SET_SYMBOL_VAL (&sym->s, Qunbound); > + } We could do several things, but I still didn't hear even a single reason why my suggestion isn't OK. IMO, it's simpler, and the test for Vdead is already in at least one other place. So I went ahead and pushed my change. Is there anything else we need to do before closing this bug? Thanks.