* bug#31238: Dynamic Module Crash
@ 2018-04-22 11:35 Tuấn Anh Nguyễn
2018-04-24 14:08 ` Philipp Stephani
0 siblings, 1 reply; 6+ messages in thread
From: Tuấn Anh Nguyễn @ 2018-04-22 11:35 UTC (permalink / raw)
To: 31238
[-- Attachment #1: Type: text/plain, Size: 297 bytes --]
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
[-- Attachment #2: Type: text/html, Size: 599 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#31238: Dynamic Module Crash
2018-04-22 11:35 bug#31238: Dynamic Module Crash Tuấn Anh Nguyễn
@ 2018-04-24 14:08 ` Philipp Stephani
2018-09-21 17:16 ` Philipp Stephani
0 siblings, 1 reply; 6+ messages in thread
From: Philipp Stephani @ 2018-04-24 14:08 UTC (permalink / raw)
To: Tuấn Anh Nguyễn; +Cc: 31238
[-- Attachment #1: Type: text/plain, Size: 488 bytes --]
Tuấn Anh Nguyễn <ubolonton@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.
[-- Attachment #2: Type: text/html, Size: 982 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#31238: Dynamic Module Crash
2018-04-24 14:08 ` Philipp Stephani
@ 2018-09-21 17:16 ` Philipp Stephani
2018-09-21 18:33 ` Noam Postavsky
2018-09-21 22:48 ` Stefan Monnier
0 siblings, 2 replies; 6+ messages in thread
From: Philipp Stephani @ 2018-09-21 17:16 UTC (permalink / raw)
To: Tuấn Anh Nguyễn; +Cc: 31238
[-- Attachment #1: Type: text/plain, Size: 782 bytes --]
Philipp Stephani <p.stephani2@gmail.com> schrieb am Di., 24. Apr. 2018 um
16:08 Uhr:
> Tuấn Anh Nguyễn <ubolonton@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.
[-- Attachment #2: Type: text/html, Size: 1591 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#31238: Dynamic Module Crash
2018-09-21 17:16 ` Philipp Stephani
@ 2018-09-21 18:33 ` Noam Postavsky
2018-09-21 22:48 ` Stefan Monnier
1 sibling, 0 replies; 6+ messages in thread
From: Noam Postavsky @ 2018-09-21 18:33 UTC (permalink / raw)
To: Philipp Stephani; +Cc: 31238, ubolonton, Stefan Monnier
On Fri, 21 Sep 2018 at 13:18, Philipp Stephani <p.stephani2@gmail.com> wrote:
>
> Philipp Stephani <p.stephani2@gmail.com> schrieb am Di., 24. Apr. 2018 um 16:08 Uhr:
>>
>> Tuấn Anh Nguyễn <ubolonton@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...
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#31238: Dynamic Module Crash
2018-09-21 17:16 ` Philipp Stephani
2018-09-21 18:33 ` Noam Postavsky
@ 2018-09-21 22:48 ` Stefan Monnier
2019-03-21 19:31 ` Philipp Stephani
1 sibling, 1 reply; 6+ messages in thread
From: Stefan Monnier @ 2018-09-21 22:48 UTC (permalink / raw)
To: Philipp Stephani; +Cc: 31238, Tuấn Anh Nguyễn
>>> 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
^ permalink raw reply [flat|nested] 6+ messages in thread* bug#31238: Dynamic Module Crash
2018-09-21 22:48 ` Stefan Monnier
@ 2019-03-21 19:31 ` Philipp Stephani
0 siblings, 0 replies; 6+ messages in thread
From: Philipp Stephani @ 2019-03-21 19:31 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 31238, Tuấn Anh Nguyễn
Am Sa., 22. Sept. 2018 um 00:48 Uhr schrieb Stefan Monnier
<monnier@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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-03-21 19:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-04-22 11:35 bug#31238: Dynamic Module Crash Tuấn Anh Nguyễn
2018-04-24 14:08 ` Philipp Stephani
2018-09-21 17:16 ` Philipp Stephani
2018-09-21 18:33 ` Noam Postavsky
2018-09-21 22:48 ` Stefan Monnier
2019-03-21 19:31 ` Philipp Stephani
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).