bug#42360: [feature/native-comp] miscompilation(?) of functions with non local exits

bug#42360: [feature/native-comp] miscompilation(?) of functions with non local exits

[Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Due to a recent modification native-comp is (probably?) miscompiling
lexical scope functions with non local exits if they involve code with
side effect on local variables.  Before fixing I prefer to double check
the correct behavior we want.

Considering the following piece of (lexical scoped) code:

===
(let (x)
  (ignore-errors
    (setq x t)
    (error "foo"))
  x)
===

Three options:

  1- Because setq is evaluated the expression should always evaluate to
  t.

  2- Unwinding the original state of the stack is restored, when it was
  saved 'x' was nil so the expression should evaluate to nil.

  3- This is unspecified.

The current stock implementaion does always 1 so I'm prone to just go
for it but I wanted to double check to be on the safe side.

FYI 1 implies C register variables cannot be used to implement Lisp
local variable if non local exits are present.

Thanks

  Andrea

--
akrl@sdf.org

bug#42360: [feature/native-comp] miscompilation(?) of functions with non local exits

[Eli Zaretskii]

> From: Andrea Corallo <akrl@sdf.org>
> Cc: Eli Zaretskii <eliz@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Wed, 15 Jul 2020 08:24:00 +0000
> 
> ===
> (let (x)
>   (ignore-errors
>     (setq x t)
>     (error "foo"))
>   x)
> ===
> 
> Three options:
> 
>   1- Because setq is evaluated the expression should always evaluate to
>   t.
> 
>   2- Unwinding the original state of the stack is restored, when it was
>   saved 'x' was nil so the expression should evaluate to nil.
> 
>   3- This is unspecified.
> 
> The current stock implementaion does always 1 so I'm prone to just go
> for it but I wanted to double check to be on the safe side.

I tend to alternative 1 as well.

bug#42360: [feature/native-comp] miscompilation(?) of functions with non local exits

[Stefan Monnier]

> Three options:
>
>   1- Because setq is evaluated the expression should always evaluate to
>   t.
>
>   2- Unwinding the original state of the stack is restored, when it was
>   saved 'x' was nil so the expression should evaluate to nil.
>
>   3- This is unspecified.

Very definitely (1)!

We don't want to introduce into Elisp the kind of messy semantics you
get in C with non-volatile variables and longjmp.

> FYI 1 implies C register variables cannot be used to implement Lisp
> local variable if non local exits are present.

IIUC, the problem only occurs for those vars which have
a `condition-case` (or `unwind-protect` or `catch`) in their scope and
where the var is modified within that construct and that a non-local
exit can jump to the end of that construct after the var was thus
modified, and that the var is used after the construct.

This should be fairly rare (not sure if those cases can easily be
written differently, OTOH).    The compiler could replace those vars
by boxing them inside a cons-cell (so the register-stored C var is
immutable and contains a pointer to a cons cell which holds the real
value in the `car`), just like we do with mutated Elisp vars captured
by closures.


        Stefan

bug#42360: [feature/native-comp] miscompilation(?) of functions with non local exits

[Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Three options:
>>
>>   1- Because setq is evaluated the expression should always evaluate to
>>   t.
>>
>>   2- Unwinding the original state of the stack is restored, when it was
>>   saved 'x' was nil so the expression should evaluate to nil.
>>
>>   3- This is unspecified.
>
> Very definitely (1)!

All right, I pushed a fix that restores behavior 1.

>> FYI 1 implies C register variables cannot be used to implement Lisp
>> local variable if non local exits are present.
>
> IIUC, the problem only occurs for those vars which have
> a `condition-case` (or `unwind-protect` or `catch`) in their scope and
> where the var is modified within that construct and that a non-local
> exit can jump to the end of that construct after the var was thus
> modified, and that the var is used after the construct.

Correct.  If the compiler keep these variables in the stack then it's
all fine because setjump will restore SP and inside the stack you'll
find the most updated value.  On the contrary if the variable was kept
in a register then its updated value may be lost if the reg is callee
saved.

> This should be fairly rare (not sure if those cases can easily be
> written differently, OTOH).

The case I've encountered is `truncate-string-to-width'.

> The compiler could replace those vars
> by boxing them inside a cons-cell (so the register-stored C var is
> immutable and contains a pointer to a cons cell which holds the real
> value in the `car`), just like we do with mutated Elisp vars captured
> by closures.

What I pushed now is (for functions with non locals) just to keep stored
all local vars in an array (as the bytecompiler does).  I added note and
we should be able to implement something more selective as suggested.
Either adding an indirection or marking the sensitive variables as
volatile.

Thanks both for the feedback.

  Andrea

-- 
akrl@sdf.org

bug#42360: [feature/native-comp] miscompilation(?) of functions with non local exits

[Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Right I think we can close this.

Thanks

  Andrea

-- 
akrl@sdf.org