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

[Philipp Stephani <p.stephani2@gmail.com>]

Am Do., 21. März 2019 um 20:27 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> > > I will CC Stefan, who committed 3eb93c07f7a60ac9ce8a16f10c3afd5a3a31243a.
> > >
> > > I'm not sure we should revert that; we could instead add GC protection
> > > for those parts that need it.
> >
> > Yes, that's what reverting that commit does :-)
>
> AFAIU, it does much more.  Stefan intended for the conservative stack
> marking to do the job, so maybe there's a little more that should be
> done to get there.  Or maybe Stefan didn't consider some important
> factor(s).  In either case, I'd like to hear his POV on this before we
> decide how to proceed.

Let's go back to the known good state first, and then discuss how to
go from there.

>
> > We need to mark the objects in all cases, not just when module
> > assertions are enabled.
>
> If we get stack marking to work, we won't need to mark objects
> explicitly.

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, and the
garbage collector couldn't possibly find it.

Or:

emacs_value x = ...;
emacs_value *y = malloc (sizeof emacs_value);
*y = x;
... stop using x...
(garbage-collect)
...use *y ...

Again, during garbage collection x is no longer on the stack.

We can only use stack scanning in Emacs because we control the Emacs
source code and can make sure these patterns don't occur. Module code
is completely arbitrary.

>
> > Note that both the designer of the module API (Daniel) and I as one of
> > its main implementers disagree with commit
> > 3eb93c07f7a60ac9ce8a16f10c3afd5a3a31243a.
>
> OK, but I think Stefan's opinion is not less important.

I value his opinion, but again: let's make the thing work first, and
then discuss options.

>
> > I've already confirmed that reverting commit
> > 3eb93c07f7a60ac9ce8a16f10c3afd5a3a31243a fixes bug#31238, and I can
> > try it with this bug as well.
>
> Please do, it's important to know that, I think.

Basil, could you check that with the revert your code now works? Thanks!