GNU bug report logs - #65796
dynamic module non_local_exit_get overwrites exit signals

Previous Next

To reply to this bug, email your comments to 65796 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Xinyang Chen <chenxinyang99 <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: dynamic module non_local_exit_get overwrites exit signals
Date: Wed, 6 Sep 2023 18:52:14 -0400

[Message part 1 (text/plain, inline)]

Currently `module_non_local_exit_get` returns pointers to fields
in emacs_env_private:
```
  if (p->pending_non_local_exit != emacs_funcall_exit_return)
    {
      *symbol = &p->non_local_exit_symbol;
      *data = &p->non_local_exit_data;
    }
```
this means that if one tries to:
```
funcall(...);
non_local_exit_get(&s1, &d1);
funcall(...);
non_local_exit_get(&s2, &d2);
non_local_exit_signal(s1, d1);
```
you would signal the second error, instead of the first error (I expected
this to happen).
It seems to me that `module_non_local_exit_get` should
`allocate_emacs_value` instead.

-Alan

[Message part 2 (text/html, inline)]

Message #8 received at 65796 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Xinyang Chen <chenxinyang99 <at> gmail.com>, Philipp Stephani <phst <at> google.com>,
 Daniel Colascione <dancol <at> dancol.org>
Cc: 65796 <at> debbugs.gnu.org
Subject: Re: bug#65796: dynamic module non_local_exit_get overwrites exit
 signals
Date: Thu, 07 Sep 2023 10:07:33 +0300

> From: Xinyang Chen <chenxinyang99 <at> gmail.com>
> Date: Wed, 6 Sep 2023 18:52:14 -0400
> 
> Currently `module_non_local_exit_get` returns pointers to fields
> in emacs_env_private:
> ```
>   if (p->pending_non_local_exit != emacs_funcall_exit_return)
>     {
>       *symbol = &p->non_local_exit_symbol;
>       *data = &p->non_local_exit_data;
>     }
> ```
> this means that if one tries to:
> ```
> funcall(...);
> non_local_exit_get(&s1, &d1);
> funcall(...);
> non_local_exit_get(&s2, &d2);
> non_local_exit_signal(s1, d1);
> ```
> you would signal the second error, instead of the first error (I expected
> this to happen).
> It seems to me that `module_non_local_exit_get` should
> `allocate_emacs_value` instead.

Philipp, Daniel: any comments?

Btw, the non_local_exit_get function is currently not documented in
the ELisp manual; should it be?

Message #11 received at 65796 <at> debbugs.gnu.org (full text, mbox):

From: Philipp Stephani <phst <at> google.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Xinyang Chen <chenxinyang99 <at> gmail.com>, p.stephani2 <at> gmail.com,
 Daniel Colascione <dancol <at> dancol.org>, 65796 <at> debbugs.gnu.org
Subject: Re: bug#65796: dynamic module non_local_exit_get overwrites exit
 signals
Date: Thu, 7 Sep 2023 12:24:03 +0200

On Thu, 7 Sept 2023 at 09:07, Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> > From: Xinyang Chen <chenxinyang99 <at> gmail.com>
> > Date: Wed, 6 Sep 2023 18:52:14 -0400
> >
> > Currently `module_non_local_exit_get` returns pointers to fields
> > in emacs_env_private:
> > ```
> >   if (p->pending_non_local_exit != emacs_funcall_exit_return)
> >     {
> >       *symbol = &p->non_local_exit_symbol;
> >       *data = &p->non_local_exit_data;
> >     }
> > ```
> > this means that if one tries to:
> > ```
> > funcall(...);
> > non_local_exit_get(&s1, &d1);
> > funcall(...);
> > non_local_exit_get(&s2, &d2);
> > non_local_exit_signal(s1, d1);
> > ```
> > you would signal the second error, instead of the first error (I expected
> > this to happen).
> > It seems to me that `module_non_local_exit_get` should
> > `allocate_emacs_value` instead.
>
> Philipp, Daniel: any comments?

Nice find!
We can't use allocate_emacs_value here because non_local_exit_get has
to work in OOM situations and can never fail. What we could do here is
e.g.:
- Document the current behavior, stating that the emacs_value objects
returned from non_local_exit_get are ephemeral. I'm not a huge fan of
this because it makes non_local_exit_get behave different from all
other functions.
- Provide an alternative non_local_exit_copy that copies the 2
Lisp_Objects into an opaque buffer supplied by the user (plus a way to
determine the buffer size). That way we shift the responsibility of
dealing with allocation failures to the user.
- Attempt to allocate a new emacs_value, fall back to the current
behavior if that fails. I don't really like that option either because
it doesn't solve the initial problem in all cases (so users still need
to deal with it), but makes both the interface and the implementation
more complex.
- Crash if we can't allocate memory. That has been rejected in other cases.

>
> Btw, the non_local_exit_get function is currently not documented in
> the ELisp manual; should it be?

At least in Emacs 29 I see it documented ("Module Nonlocal" node).


--
Google Germany GmbH
Erika-Mann-Straße 33
80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Falls Sie diese fälschlicherweise
erhalten haben sollten, leiten Sie diese bitte nicht an jemand anderes
weiter, löschen Sie alle Kopien und Anhänge davon und lassen Sie mich
bitte wissen, dass die E-Mail an die falsche Person gesendet wurde.

This e-mail is confidential. If you received this communication by
mistake, please don’t forward it to anyone else, please erase all
copies and attachments, and please let me know that it has gone to the
wrong person.

Message #14 received at 65796 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philipp Stephani <phst <at> google.com>
Cc: chenxinyang99 <at> gmail.com, p.stephani2 <at> gmail.com, dancol <at> dancol.org,
 65796 <at> debbugs.gnu.org
Subject: Re: bug#65796: dynamic module non_local_exit_get overwrites exit
 signals
Date: Thu, 07 Sep 2023 13:55:09 +0300

> From: Philipp Stephani <phst <at> google.com>
> Date: Thu, 7 Sep 2023 12:24:03 +0200
> Cc: Xinyang Chen <chenxinyang99 <at> gmail.com>, Daniel Colascione <dancol <at> dancol.org>, 65796 <at> debbugs.gnu.org, 
> 	p.stephani2 <at> gmail.com
> 
> Nice find!
> We can't use allocate_emacs_value here because non_local_exit_get has
> to work in OOM situations and can never fail. What we could do here is
> e.g.:
> - Document the current behavior, stating that the emacs_value objects
> returned from non_local_exit_get are ephemeral. I'm not a huge fan of
> this because it makes non_local_exit_get behave different from all
> other functions.
> - Provide an alternative non_local_exit_copy that copies the 2
> Lisp_Objects into an opaque buffer supplied by the user (plus a way to
> determine the buffer size). That way we shift the responsibility of
> dealing with allocation failures to the user.
> - Attempt to allocate a new emacs_value, fall back to the current
> behavior if that fails. I don't really like that option either because
> it doesn't solve the initial problem in all cases (so users still need
> to deal with it), but makes both the interface and the implementation
> more complex.
> - Crash if we can't allocate memory. That has been rejected in other cases.

I guess just one alternative is acceptable?

> > Btw, the non_local_exit_get function is currently not documented in
> > the ELisp manual; should it be?
> 
> At least in Emacs 29 I see it documented ("Module Nonlocal" node).

I need new glasses.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.