From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Valentin Gatien-Baron Newsgroups: gmane.emacs.bugs Subject: bug#29066: 26.0.90; crash in gc involving buffer local symbols Date: Tue, 31 Oct 2017 10:52:44 -0400 Message-ID: References: <83a808tlqp.fsf@gnu.org> <838tfst27n.fsf@gnu.org> <87efpjzv2p.fsf@linux-m68k.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="001a113f8fa463c2e8055cd8e8ce" X-Trace: blaine.gmane.org 1509461653 18549 195.159.176.226 (31 Oct 2017 14:54:13 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 31 Oct 2017 14:54:13 +0000 (UTC) Cc: 29066@debbugs.gnu.org, Mark Shinwell To: Andreas Schwab Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Oct 31 15:54:05 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 1e9XvQ-0003Na-GF for geb-bug-gnu-emacs@m.gmane.org; Tue, 31 Oct 2017 15:53:56 +0100 Original-Received: from localhost ([::1]:45983 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1e9XvX-0000Qs-Ks for geb-bug-gnu-emacs@m.gmane.org; Tue, 31 Oct 2017 10:54:03 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:58619) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1e9Xud-0008QR-Hw for bug-gnu-emacs@gnu.org; Tue, 31 Oct 2017 10:53:08 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1e9XuY-0005Hy-8B for bug-gnu-emacs@gnu.org; Tue, 31 Oct 2017 10:53:06 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:35665) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1e9XuY-0005Hr-2M for bug-gnu-emacs@gnu.org; Tue, 31 Oct 2017 10:53:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1e9XuX-000470-S3 for bug-gnu-emacs@gnu.org; Tue, 31 Oct 2017 10:53:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Valentin Gatien-Baron Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 31 Oct 2017 14:53:01 +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.150946157415792 (code B ref 29066); Tue, 31 Oct 2017 14:53:01 +0000 Original-Received: (at 29066) by debbugs.gnu.org; 31 Oct 2017 14:52:54 +0000 Original-Received: from localhost ([127.0.0.1]:44346 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1e9XuQ-00046e-44 for submit@debbugs.gnu.org; Tue, 31 Oct 2017 10:52:54 -0400 Original-Received: from mxout3.mail.janestreet.com ([38.105.200.229]:55514) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1e9XuN-00046N-RE for 29066@debbugs.gnu.org; Tue, 31 Oct 2017 10:52:52 -0400 Original-Received: from [172.27.56.68] (helo=tot-qpr-mailcore1) by mxout3.mail.janestreet.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.89) (envelope-from ) id 1e9XuI-0000qN-FG for 29066@debbugs.gnu.org; Tue, 31 Oct 2017 10:52:46 -0400 X-JS-Flow: external X-JS-Scanner-attachment: (ok) No attachments Original-Received: by tot-qpr-mailcore1 with ocaml/mailcore/mailcore 1.0+136 (04e1cd915edc) (envelope-from ) id BZ-I4--FdUSQA-Od; 2017-10-31 10:52:46.466622-04:00 Original-Received: from mail-lf0-f71.google.com ([209.85.215.71]) by mxgoog1.mail.janestreet.com with esmtps (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.89) (envelope-from ) id 1e9XuI-0001Eh-AE for 29066@debbugs.gnu.org; Tue, 31 Oct 2017 10:52:46 -0400 Original-Received: by mail-lf0-f71.google.com with SMTP id 75so5061767lfx.15 for <29066@debbugs.gnu.org>; Tue, 31 Oct 2017 07:52:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=EPa58Lw4rNdYTExawNipAbJODgSTdUT/Mi5CLzeZgRY=; b=TPXttA/HE6XeEldO8jGUpNp5FVGFx2B14QTGJvLnAXuerL1yXEPbmwHE8xRyL3uhcS 3FAzeNKHKrRs2wPUD+oUPBqQhuDE24vBHpT/oze0fLsPhCyguvNVfwURDxwgjdd0fS9v r20vK1ThMDDp4j+f2bdYib90Bv1NZL0Nk2bkg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=EPa58Lw4rNdYTExawNipAbJODgSTdUT/Mi5CLzeZgRY=; b=e5fPmkvr+5SldnmMuzkBWRTELrnnVwbAFpiIATp5eJXwkQJaP/6/97xP8qF0xcFIQ4 O83mRvyS4N437tuMYSBaoOYDYvJmuwLYpAhYeb7dWUffm3VdelKzXeSdjz6rtxQr505k mZXlhncHifE5GGyslLbGbI/aqzbteUwdIL0wJruS6jb/i33FPHh5k9MjQTyA71IM9v3S ozzkUFhZXxnMI1MvmgNxfdhf8INWjgPpZITOPPYxqxwtmDgwZgp8iMq2zCS30PReW94j X3XhOmJbJIzdDUdm94ZjNisK3lTYcBRhu+gzi4ihLUQsnitQ4AByeT5raAUH741j4caU hHJQ== X-Gm-Message-State: AMCzsaXuMiyuuGgnLpYyMYc2y+e89XvOiSdaT0OBO5N3ALMxFaQtz12I N4F9chuy/MDPwOvG15W5x9w8AaweHVusgbW5y3qGLf7c10iIE2ZmWQJt93P10zWQPwLM0PJckFP VkbnBNuXjL8yACDBtTzTXkXAfxyQd/g== X-Received: by 10.25.16.28 with SMTP id f28mr909786lfi.133.1509461565295; Tue, 31 Oct 2017 07:52:45 -0700 (PDT) X-Google-Smtp-Source: ABhQp+Rm7hJBN3wCeU8XWgr0hiqyYUBOyIeMWd0gqd86r/aXBdaO8Gr51iVCBVVk1aldc697TaR38qCKPp2RsKlGk70= X-Received: by 10.25.16.28 with SMTP id f28mr909780lfi.133.1509461565087; Tue, 31 Oct 2017 07:52:45 -0700 (PDT) Original-Received: by 10.25.234.11 with HTTP; Tue, 31 Oct 2017 07:52:44 -0700 (PDT) In-Reply-To: <87efpjzv2p.fsf@linux-m68k.org> X-JS-Exim-Data-Received: 2017-10-31 10:52:46-0400 X-JS-Processed-by: mailcore 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:139265 Archived-At: --001a113f8fa463c2e8055cd8e8ce Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Oct 31, 2017 at 2:32 AM, Andreas Schwab wrote: > On Okt 31 2017, Eli Zaretskii wrote: > > >> I also checked the following works, and seems better to me (stop havin= g > dangling pointers, instead of being > >> careful with them): > >> > >> diff --git a/src/alloc.c b/src/alloc.c > >> index da0c3ad4b3..44dfa95cf5 100644 > >> --- a/src/alloc.c > >> +++ b/src/alloc.c > >> @@ -7030,8 +7030,10 @@ sweep_symbols (void) > >> { > >> if (!sym->s.gcmarkbit) > >> { > >> - if (sym->s.redirect =3D=3D SYMBOL_LOCALIZED) > >> + if (sym->s.redirect =3D=3D SYMBOL_LOCALIZED) { > >> xfree (SYMBOL_BLV (&sym->s)); > >> + sym->s.val.blv =3D NULL; > >> + } > > > > 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. > =E2=80=8BSET_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= =E2=80=8B. 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 =3D=3D SYMBOL_LOCALIZED) + if (sym->s.redirect =3D=3D SYMBOL_LOCALIZED && sym->s.val.bl= v) { xfree (SYMBOL_BLV (&sym->s)); + sym->s.val.blv =3D NULL; + } sym->s.next =3D symbol_free_list; symbol_free_list =3D &sym->s; symbol_free_list->function =3D Vdead; 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 =3D=3D SYMBOL_LOCALIZED) + if (sym->s.redirect =3D=3D SYMBOL_LOCALIZED) { xfree (SYMBOL_BLV (&sym->s)); + sym->s.redirect =3D SYMBOL_PLAINVAL; + SET_SYMBOL_VAL (&sym->s, Qunbound); + } sym->s.next =3D symbol_free_list; symbol_free_list =3D &sym->s; symbol_free_list->function =3D Vdead; --001a113f8fa463c2e8055cd8e8ce Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Tue, Oct 31, 2017 at 2:32 AM, Andreas Schwab <schwab@l= inux-m68k.org> wrote:
On Okt 31 2017, Eli Zaretskii <eliz@gnu.org> wrote:

>> I also checked the following works, and seems better to me (stop h= aving dangling pointers, instead of being
>> careful with them):
>>
>> diff --git a/src/alloc.c b/src/alloc.c
>> index da0c3ad4b3..44dfa95cf5 100644
>> --- a/src/alloc.c
>> +++ b/src/alloc.c
>> @@ -7030,8 +7030,10 @@ sweep_symbols (void)
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 {
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!sym->s.gcmarkbit)=
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 {
>> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (sym->s.re= direct =3D=3D SYMBOL_LOCALIZED)
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (sym->s.re= direct =3D=3D SYMBOL_LOCALIZED) {
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 xfre= e (SYMBOL_BLV (&sym->s));
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sym->s= .val.blv =3D NULL;
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
>
> 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.=C2=A0 And calling SYMBOL_BLV with a free= d symbol is a
bug anyway.

=E2=80=8BSET_SYMBOL_BLV = insists that the new value is not NULL, even if it asserts nothing about th= e current value.

We do call SYMBOL_BLV af= ter freeing, when we re-sweep the symbol, which is fine because free does n= othing when given NULL, but triggers the assertion=E2=80=8B.

I would do this, to avoid the assertion failure:
<= /div>

diff --git a/src/alloc.c b/src/alloc.c
ind= ex da0c3ad4b3..72550e812b 100644
--- a/src/alloc.c
+++ = b/src/alloc.c
@@ -7030,8 +7030,10 @@ sweep_symbols (void)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0{
=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0if (!sym->s.gcmarkbit)
=C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0{
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 if (sym->s.redirect =3D=3D SYMBOL_LOCALIZED)
+= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (sym->s.redirect =3D= =3D SYMBOL_LOCALIZED && sym->s.val.blv) {
=C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0xfree (SYMBOL_BLV (&= ;sym->s));
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 sym->s.val.blv =3D NULL;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 }
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0sym->s.next =3D symbol_free_list;
=C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0symbol_free_list =3D &sym->= s;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0symbol_= free_list->function =3D Vdead;

Or chan= ging the redirect type:

d= iff --git a/src/alloc.c b/src/alloc.c
ind= ex da0c3ad4b3..6966d96c6d 100644
--- a/sr= c/alloc.c
+++ b/src/alloc.c
@@ -7030,8 +7030,11 @@ sweep_symbols (void)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0{
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!sym->s.= gcmarkbit)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0{
-=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (sym->s.redirect =3D=3D SYMBOL_LOCALI= ZED)
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 if (sym->s.redirect =3D=3D SYMBOL_LOCALIZED) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0xfree (SYMBOL_BLV (&sym->s));
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sym->s.re= direct =3D SYMBOL_PLAINVAL;
+=C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 SET_SYMBOL_VAL (&sym->= s, Qunbound);
+=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 }
=C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0sym->s.next =3D symbol_free_lis= t;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0symbol_free_list =3D &sym->s;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0symbol_= free_list->function =3D Vdead;

--001a113f8fa463c2e8055cd8e8ce--