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

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

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.

>
> > 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?
Because the module is written in a language that doesn't use the stack
in a way that the garbage collector expects? There's no reason to
assume modules have any form of C-compatible stack layout.

>
> Moreover, emacs_value is actually a pointer to a Lisp object, so this
> object is also somewhere on the stack, right?
>
> > 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.
>
> Why do you think it isn't on the stack?

Same as above.

>
> > We can only use stack scanning in Emacs because we control the Emacs
> > source code
>
> Actually, we do nothing special about stack-based values in our
> sources, except avoiding undefined behavior.

(Stack scanning is undefined behavior, but that's not the point.)
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.

>
> > > 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.
>
> Fixing one bug doesn't necessarily mean things now "work"; there's
> always one more bug.

That might be theoretically true, but shouldn't impact decisions until
that bug is actually found. All regression tests still pass after
reverting the commit.