GNU bug report logs - #31238
Dynamic Module Crash

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 31238 in the body.
You can then email your comments to 31238 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Tuấn Anh Nguyễn <ubolonton <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Dynamic Module Crash
Date: Sun, 22 Apr 2018 11:35:01 +0000

[Message part 1 (text/plain, inline)]

Emacs 25.3.1 crashes with a segfault when trying to print a value
constructed (in certain ways) by a dynamic module.

Reproduction: https://github.com/ubolonton/emacs-module-crash

Context: https://github.com/ubolonton/emacs-module-rs/issues/2

-- 
Tuấn-Anh Nguyễn
Software Engineer

[Message part 2 (text/html, inline)]

Message #8 received at 31238 <at> debbugs.gnu.org (full text, mbox):

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: Tuấn Anh Nguyễn <ubolonton <at> gmail.com>
Cc: 31238 <at> debbugs.gnu.org
Subject: Re: bug#31238: Dynamic Module Crash
Date: Tue, 24 Apr 2018 14:08:01 +0000

[Message part 1 (text/plain, inline)]

Tuấn Anh Nguyễn <ubolonton <at> gmail.com> schrieb am So., 22. Apr. 2018 um
18:32 Uhr:

> Emacs 25.3.1 crashes with a segfault when trying to print a value
> constructed (in certain ways) by a dynamic module.
>
> Reproduction: https://github.com/ubolonton/emacs-module-crash
>
> Context: https://github.com/ubolonton/emacs-module-rs/issues/2
>
>
This might be caused by commit 3eb93c07f7a60ac9ce8a16f10c3afd5a3a31243a,
which removed GC protection for module-allocated values.

[Message part 2 (text/html, inline)]

Message #11 received at 31238 <at> debbugs.gnu.org (full text, mbox):

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: Tuấn Anh Nguyễn <ubolonton <at> gmail.com>
Cc: 31238 <at> debbugs.gnu.org
Subject: Re: bug#31238: Dynamic Module Crash
Date: Fri, 21 Sep 2018 19:16:44 +0200

[Message part 1 (text/plain, inline)]

Philipp Stephani <p.stephani2 <at> gmail.com> schrieb am Di., 24. Apr. 2018 um
16:08 Uhr:

> Tuấn Anh Nguyễn <ubolonton <at> gmail.com> schrieb am So., 22. Apr. 2018 um
> 18:32 Uhr:
>
>> Emacs 25.3.1 crashes with a segfault when trying to print a value
>> constructed (in certain ways) by a dynamic module.
>>
>> Reproduction: https://github.com/ubolonton/emacs-module-crash
>>
>> Context: https://github.com/ubolonton/emacs-module-rs/issues/2
>>
>>
> This might be caused by commit 3eb93c07f7a60ac9ce8a16f10c3afd5a3a31243a,
> which removed GC protection for module-allocated values.
>

Unless anybody complains, I'll revert that commit. Modules can store/use
emacs_value objects in arbitrary ways; there's no way we can rely on stack
scanning to reliably find them.

[Message part 2 (text/html, inline)]

Message #14 received at 31238 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Philipp Stephani <p.stephani2 <at> gmail.com>
Cc: 31238 <at> debbugs.gnu.org, ubolonton <at> gmail.com,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#31238: Dynamic Module Crash
Date: Fri, 21 Sep 2018 14:33:47 -0400

On Fri, 21 Sep 2018 at 13:18, Philipp Stephani <p.stephani2 <at> gmail.com> wrote:
>
> Philipp Stephani <p.stephani2 <at> gmail.com> schrieb am Di., 24. Apr. 2018 um 16:08 Uhr:
>>
>> Tuấn Anh Nguyễn <ubolonton <at> gmail.com> schrieb am So., 22. Apr. 2018 um 18:32 Uhr:
>>>
>>> Emacs 25.3.1 crashes with a segfault when trying to print a value constructed (in certain ways) by a dynamic module.
>>> Reproduction: https://github.com/ubolonton/emacs-module-crash
>>> Context: https://github.com/ubolonton/emacs-module-rs/issues/2
>>
>> This might be caused by commit 3eb93c07f7a60ac9ce8a16f10c3afd5a3a31243a, which removed GC protection for module-allocated values.
>
> Unless anybody complains, I'll revert that commit. Modules can store/use emacs_value objects in arbitrary ways; there's no way we can rely on stack scanning to reliably find them.

I'll just add author of said commit to Cc, perhaps that will increase
the chance of getting some useful response...

Message #17 received at 31238 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Philipp Stephani <p.stephani2 <at> gmail.com>
Cc: 31238 <at> debbugs.gnu.org,
 Tuấn Anh Nguyễn
 <ubolonton <at> gmail.com>
Subject: Re: bug#31238: Dynamic Module Crash
Date: Fri, 21 Sep 2018 18:48:40 -0400

>>> Emacs 25.3.1 crashes with a segfault when trying to print a value
>>> constructed (in certain ways) by a dynamic module.
>>> Reproduction: https://github.com/ubolonton/emacs-module-crash
>>> Context: https://github.com/ubolonton/emacs-module-rs/issues/2
>> This might be caused by commit 3eb93c07f7a60ac9ce8a16f10c3afd5a3a31243a,

IIUC that's just a wild guess, right?

>> which removed GC protection for module-allocated values.

Not really, no.  It just relied on another pre-existing mechanism.

> Unless anybody complains, I'll revert that commit.

Do we even know that reverting it circumvents this crash?

And IIRC this patch is not just an implementation detail, it changes the
module's API and ABI, so reverting it shouldn't be done lightly.

> Modules can store/use emacs_value objects in arbitrary ways;
> there's no way we can rely on stack scanning to reliably find them.

We don't blindly rely on stack scanning: there are functions to
(un)register other values stored in the (non-Lisp) heap.

The previous code wasn't magical either.

I don't know nearly enough about Rust's implementation strategy and the
`emacs` crate used to guess how its memory management could/should
interact with Emacs's.


        Stefan

Message #20 received at 31238 <at> debbugs.gnu.org (full text, mbox):

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 31238 <at> debbugs.gnu.org,
 Tuấn Anh Nguyễn <ubolonton <at> gmail.com>
Subject: Re: bug#31238: Dynamic Module Crash
Date: Thu, 21 Mar 2019 20:31:23 +0100

Am Sa., 22. Sept. 2018 um 00:48 Uhr schrieb Stefan Monnier
<monnier <at> iro.umontreal.ca>:
>
> >>> Emacs 25.3.1 crashes with a segfault when trying to print a value
> >>> constructed (in certain ways) by a dynamic module.
> >>> Reproduction: https://github.com/ubolonton/emacs-module-crash
> >>> Context: https://github.com/ubolonton/emacs-module-rs/issues/2
> >> This might be caused by commit 3eb93c07f7a60ac9ce8a16f10c3afd5a3a31243a,
>
> IIUC that's just a wild guess, right?

Let's rather say that it's the obvious suspect. I've now also
confirmed that I can consistently reproduce this bug with commit
3eb93c07f7a60ac9ce8a16f10c3afd5a3a31243a, and that it's consistently
fixed after reverting 3eb93c07f7a60ac9ce8a16f10c3afd5a3a31243a. Since
there was also no consensus to install commit
3eb93c07f7a60ac9ce8a16f10c3afd5a3a31243a in the first place (see
https://lists.gnu.org/archive/html/emacs-devel/2016-01/msg00150.html),
I went ahead and reverted commit
3eb93c07f7a60ac9ce8a16f10c3afd5a3a31243a on master.

>
> >> which removed GC protection for module-allocated values.
>
> Not really, no.  It just relied on another pre-existing mechanism.

A mechanism that can't work. Lisp_Object values in Emacs's source code
might be known to be on the stack, but that's not the case for
modules.

>
> > Unless anybody complains, I'll revert that commit.
>
> Do we even know that reverting it circumvents this crash?

Yes, I've tested this now extensively.

>
> And IIRC this patch is not just an implementation detail, it changes the
> module's API and ABI, so reverting it shouldn't be done lightly.

Commit 3eb93c07f7a60ac9ce8a16f10c3afd5a3a31243a did introduce a
breaking change in the module API in the sense that it removed NULL as
a sentinel value. Reverting that commit restores the original behavior
(which is itself not a breaking change). I don't see any ABI changes
with 3eb93c07f7a60ac9ce8a16f10c3afd5a3a31243a since it didn't change
emacs-module.h (meaningfully).

>
> > Modules can store/use emacs_value objects in arbitrary ways;
> > there's no way we can rely on stack scanning to reliably find them.
>
> We don't blindly rely on stack scanning: there are functions to
> (un)register other values stored in the (non-Lisp) heap.

Do you mean the "global value" function? These are only for values
that have to survive longer than their environment. All other values
live as long as their environment.

>
> The previous code wasn't magical either.

No, and I certainly don't claim it's perfect, but at least it didn't
exhibit this bug. I'm happy to discuss alternatives as long as we (at
least Daniel and I) agree with them and they don't reintroduce this
bug.

>
> I don't know nearly enough about Rust's implementation strategy and the
> `emacs` crate used to guess how its memory management could/should
> interact with Emacs's.

I don't think Rust is the issue here. As with any language exposing a
C interface, it has to play by C's rules for the scope of that
interface.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.