Subject: bug#34655: 26.1.92; Segfault in module with --module-assertions

From: Philipp Stephani <[email-masked]>
Date: Thu, 21 Mar 2019 21:26:25 +0100

Am Do., 21. März 2019 um 21:14 Uhr schrieb Eli Zaretskii <[email-masked]>:
>
> > From: Philipp Stephani <[email-masked]>
> > Date: Thu, 21 Mar 2019 21:01:43 +0100
> > Cc: Stefan Monnier <[email-masked]>, "Basil L. Contovounesios" <[email-masked]>, [email-masked],
> >       Daniel Colascione <[email-masked]>
> >
> > Am Do., 21. März 2019 um 20:50 Uhr schrieb Eli Zaretskii <[email-masked]>:
> > >
> > > > From: Philipp Stephani <[email-masked]>
> > > > Date: Thu, 21 Mar 2019 20:37:24 +0100
> > > > Cc: Stefan Monnier <[email-masked]>, "Basil L. Contovounesios" <[email-masked]>, [email-masked]
> > > >
> > > > Let's go back to the known good state first, and then discuss how to
> > > > go from there.
> > >
> > > I don't see why that is better than discuss first and then go to where
> > > we decide to go.  It's not like Emacs 27 will be released any time
> > > soon, so there's no rush.
> >
> > For one, it becomes harder and harder to revert commits the older they
> > get. Also such discussions tend to turn into endless debates about the
> > "perfect" solution until one side gives up, without improving
> > anything. I strongly prefer fixing actual bugs that affect users in
> > practice and then discussing (or not discussing) the gold-plating
> > steps later.
>
> I also prefer fixing bugs (which is why I spent several hours looking
> into Basil's crash, when no one else was replying to that bug report),
> but this is a community project, so we should discuss first and act
> later.  Especially when controversial issues are involved.

Well, as you can see, these discussions seem to lead nowhere, and both
bug#34655 and bug#31238 remain unfixed.

>
> > > > We can't get stack marking to work, even theoretically.
> > > >
> > > > A module is free to do
> > > >
> > > > emacs_value x = ...;
> > > > uintptr_t y = ((uintrptr_t) x) ^ 0x123456u;
> > > > (garbage-collect)
> > > > emacs_value z = (emacs_value) (y ^ 0x123456u);
> > > > ... use z ...
> > > >
> > > > During the garbage collection, x isn't on the stack anywhere
> > >
> > > Why do you think x isn't on the stack in this case?
> >
> > Because the compiler reused the stack slot for something else?
>
> How can it?  You are using the same pointer later.

Assume I don't use x later, and only y is on the stack during GC.

>  Garbage collection
> cannot happen unless you call an Emacs function, such as Ffuncall.
> Calling a function means that even if the pointer to a Lisp object was
> in a register, it will be put on the stack when calling Emacs.

No matter whether y here is in a register or on the stack, it's not a
Lisp_Value, so the GC can't find it.

>
> > Because the module is written in a language that doesn't use the stack
> > in a way that the garbage collector expects?
>
> Which language is that, and how can it use the emacs-module machinery?

Go, for example. It uses green threads with its own stack management
and calling conventions. The GC couldn't ever find such a stack.

>
> > > Moreover, emacs_value is actually a pointer to a Lisp object, so this
> > > object is also somewhere on the stack, right?
>
> No answer?

An emacs_value in the current implementation *is* a Lisp_Object cast
to emacs_value. If the emacs_value is not on the stack, then there's
no way to find the Lisp_Object.

>
> > We do something very specific with the stack: we make sure that
> > Lisp_Objects are never manipulated in a way similar to the above, and
> > we use the C language.
>
> If worse comes to worst, we can request module writers to adhere to
> the same discipline.  We already request them to do/not to do quite a
> few extraordinary things.

No we can't. Modules can contain arbitrary code and call arbitrary
libraries, which we can't ever control. We need to work with
everything that provides a C-compatible interface.

>
> > All regression tests still pass after reverting the commit.
>
> Didn't they also pass before?

Yes, but the reproduction steps in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=31238 didn't.