MPS: Problem with dynamic modules

MPS: Problem with dynamic modules

[Gerd Möllmann]

I think I've found a problem with modules, which I currently don't have
a plan how to fix it or work around it.

Example: vterm.

Its emacs_module_init uses module_make_global_ref to get handles to
functions in Emacs that it can call, among other things. One example

  emacs_value Fapply;

  int emacs_module_init(struct emacs_runtime *ert) {
    emacs_env *env = ert->get_environment(ert);
  {
    Fapply = env->make_global_ref(env, env->intern(env, "apply"));
  }

Note that Fapply here is a variable in the shared lib for vterm.

module_make_global_ref creates a module_global_reference in Emacs, say
R, a pseudo-vector, and remembers it in a global hash table. Its return
value is &R->value,

As a pseudo-vector, R moves, and &R->value changes when it does. And so,
Fapply in the vterm modules becomes invalid.

Re: MPS: Problem with dynamic modules

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Emacs Devel <emacs-devel@gnu.org>
> Date: Sun, 19 May 2024 15:01:35 +0200
> 
> I think I've found a problem with modules, which I currently don't have
> a plan how to fix it or work around it.
> 
> Example: vterm.
> 
> Its emacs_module_init uses module_make_global_ref to get handles to
> functions in Emacs that it can call, among other things. One example
> 
>   emacs_value Fapply;
> 
>   int emacs_module_init(struct emacs_runtime *ert) {
>     emacs_env *env = ert->get_environment(ert);
>   {
>     Fapply = env->make_global_ref(env, env->intern(env, "apply"));
>   }
> 
> Note that Fapply here is a variable in the shared lib for vterm.
> 
> module_make_global_ref creates a module_global_reference in Emacs, say
> R, a pseudo-vector, and remembers it in a global hash table. Its return
> value is &R->value,
> 
> As a pseudo-vector, R moves, and &R->value changes when it does. And so,
> Fapply in the vterm modules becomes invalid.

Can we prevent the pseudo-vector R from moving?

Re: MPS: Problem with dynamic modules

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> As a pseudo-vector, R moves, and &R->value changes when it does. And so,
>> Fapply in the vterm modules becomes invalid.
>
> Can we prevent the pseudo-vector R from moving?

I'm not sure.

MPS has an AMS pool class (mark-sweep, non-moving) that could work.
Alas, they say themselves that is not production-ready. And on reading
the reference again

  https://memory-pool-system.readthedocs.io/en/latest/pool/ams.html#pool-ams

I also see this which I have difficulties interpreting

  Blocks may only be referenced by base pointers (unless they have
  in-band headers).

In the best case, it simmpy means that interior pointers as references
are not supported.

Another idea would be to malloc R->value and return R->value instead of
&R->value. But I have not the slightest idea what consequences that
might possibly have in terms of leaking abstractions and compatibilty.
Maybe a module expert could tell.

Re: MPS: Problem with dynamic modules

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: eller.helmut@gmail.com,  emacs-devel@gnu.org
> Date: Sun, 19 May 2024 15:58:28 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> As a pseudo-vector, R moves, and &R->value changes when it does. And so,
> >> Fapply in the vterm modules becomes invalid.
> >
> > Can we prevent the pseudo-vector R from moving?
> 
> I'm not sure.
> 
> MPS has an AMS pool class (mark-sweep, non-moving) that could work.
> Alas, they say themselves that is not production-ready. And on reading
> the reference again
> 
>   https://memory-pool-system.readthedocs.io/en/latest/pool/ams.html#pool-ams

No, I mean tell the pool we are using now that this vector cannot be
moved.  AFAIU, this should be possible, and we do that in other cases,
no?

> Another idea would be to malloc R->value and return R->value instead of
> &R->value. But I have not the slightest idea what consequences that
> might possibly have in terms of leaking abstractions and compatibilty.
> Maybe a module expert could tell.

Daniel, can you help here?

Re: MPS: Problem with dynamic modules

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

> No, I mean tell the pool we are using now that this vector cannot be
> moved.  AFAIU, this should be possible, and we do that in other cases,
> no?

Sort of. The only way to do this is to add a reference to the object to an
ambiguous root. I.e. we need some registry data structure for such
objects and make that an ambig root.

Re: MPS: Problem with dynamic modules

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Daniel Colascione <dancol@dancol.org>,  eller.helmut@gmail.com,
>   emacs-devel@gnu.org
> Date: Sun, 19 May 2024 18:17:06 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > No, I mean tell the pool we are using now that this vector cannot be
> > moved.  AFAIU, this should be possible, and we do that in other cases,
> > no?
> 
> Sort of. The only way to do this is to add a reference to the object to an
> ambiguous root. I.e. we need some registry data structure for such
> objects and make that an ambig root.

That sounds like the easiest solution, no?

Re: MPS: Problem with dynamic modules

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: Daniel Colascione <dancol@dancol.org>,  eller.helmut@gmail.com,
>>   emacs-devel@gnu.org
>> Date: Sun, 19 May 2024 18:17:06 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > No, I mean tell the pool we are using now that this vector cannot be
>> > moved.  AFAIU, this should be possible, and we do that in other cases,
>> > no?
>> 
>> Sort of. The only way to do this is to add a reference to the object to an
>> ambiguous root. I.e. we need some registry data structure for such
>> objects and make that an ambig root.
>
> That sounds like the easiest solution, no?

Hm, I wouldn't say so. The easiest for me would be to malloc the value,
followed by using the AMS pool, followed by implementing a tree or
whatever is good for that purpose, making it a root and scan it.

Re: MPS: Problem with dynamic modules

[Gerd Möllmann]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>>> Cc: Daniel Colascione <dancol@dancol.org>,  eller.helmut@gmail.com,
>>>   emacs-devel@gnu.org
>>> Date: Sun, 19 May 2024 18:17:06 +0200
>>> 
>>> Eli Zaretskii <eliz@gnu.org> writes:
>>> 
>>> > No, I mean tell the pool we are using now that this vector cannot be
>>> > moved.  AFAIU, this should be possible, and we do that in other cases,
>>> > no?
>>> 
>>> Sort of. The only way to do this is to add a reference to the object to an
>>> ambiguous root. I.e. we need some registry data structure for such
>>> objects and make that an ambig root.
>>
>> That sounds like the easiest solution, no?
>
> Hm, I wouldn't say so. The easiest for me would be to malloc the value,
> followed by using the AMS pool, followed by implementing a tree or
> whatever is good for that purpose, making it a root and scan it.

I've added said pool now.

Re: MPS: Problem with dynamic modules

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: dancol@dancol.org,  eller.helmut@gmail.com,  emacs-devel@gnu.org
> Date: Mon, 20 May 2024 07:35:33 +0200
> 
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
> > Eli Zaretskii <eliz@gnu.org> writes:
> >
> >>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> >>> Cc: Daniel Colascione <dancol@dancol.org>,  eller.helmut@gmail.com,
> >>>   emacs-devel@gnu.org
> >>> Date: Sun, 19 May 2024 18:17:06 +0200
> >>> 
> >>> Eli Zaretskii <eliz@gnu.org> writes:
> >>> 
> >>> > No, I mean tell the pool we are using now that this vector cannot be
> >>> > moved.  AFAIU, this should be possible, and we do that in other cases,
> >>> > no?
> >>> 
> >>> Sort of. The only way to do this is to add a reference to the object to an
> >>> ambiguous root. I.e. we need some registry data structure for such
> >>> objects and make that an ambig root.
> >>
> >> That sounds like the easiest solution, no?
> >
> > Hm, I wouldn't say so. The easiest for me would be to malloc the value,
> > followed by using the AMS pool, followed by implementing a tree or
> > whatever is good for that purpose, making it a root and scan it.
> 
> I've added said pool now.

I tried to run the emacs-module test, but it doesn't compile here:

    CCLD     src/emacs-module-resources/mod-test.dll
  make[1]: Entering directory `/d/gnu/git/emacs/feature/test'
  In file included from src/emacs-module-resources/mod-test.c:47:
  ../src/emacs-module.h:1120:26: error: unknown type name 'Lisp_Object'
   1120 | struct emacs_value_tag { Lisp_Object v; };
	|                          ^~~~~~~~~~~
  ../src/emacs-module.h:1128:27: error: field 'header' has incomplete type
   1128 |   union vectorlike_header header;
	|                           ^~~~~~

Does it compile for you?  Why did you move the emacs_value_tag stuff
from emacs-module.c (which is compiled as part of Emacs) to
emacs-module.h (which is included by modules, which aren't supposed to
know about Lisp_Object, they are supposed to use emacs_value instead.
But not a module cannot be compiled without including lisp.h, which I
think is wrong.

Re: MPS: Problem with dynamic modules

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: dancol@dancol.org,  eller.helmut@gmail.com,  emacs-devel@gnu.org
>> Date: Mon, 20 May 2024 07:35:33 +0200
>> 
>> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>> 
>> > Eli Zaretskii <eliz@gnu.org> writes:
>> >
>> >>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> >>> Cc: Daniel Colascione <dancol@dancol.org>,  eller.helmut@gmail.com,
>> >>>   emacs-devel@gnu.org
>> >>> Date: Sun, 19 May 2024 18:17:06 +0200
>> >>> 
>> >>> Eli Zaretskii <eliz@gnu.org> writes:
>> >>> 
>> >>> > No, I mean tell the pool we are using now that this vector cannot be
>> >>> > moved.  AFAIU, this should be possible, and we do that in other cases,
>> >>> > no?
>> >>> 
>> >>> Sort of. The only way to do this is to add a reference to the object to an
>> >>> ambiguous root. I.e. we need some registry data structure for such
>> >>> objects and make that an ambig root.
>> >>
>> >> That sounds like the easiest solution, no?
>> >
>> > Hm, I wouldn't say so. The easiest for me would be to malloc the value,
>> > followed by using the AMS pool, followed by implementing a tree or
>> > whatever is good for that purpose, making it a root and scan it.
>> 
>> I've added said pool now.
>
> I tried to run the emacs-module test, but it doesn't compile here:
>
>     CCLD     src/emacs-module-resources/mod-test.dll
>   make[1]: Entering directory `/d/gnu/git/emacs/feature/test'
>   In file included from src/emacs-module-resources/mod-test.c:47:
>   ../src/emacs-module.h:1120:26: error: unknown type name 'Lisp_Object'
>    1120 | struct emacs_value_tag { Lisp_Object v; };
> 	|                          ^~~~~~~~~~~
>   ../src/emacs-module.h:1128:27: error: field 'header' has incomplete type
>    1128 |   union vectorlike_header header;
> 	|                           ^~~~~~
>
> Does it compile for you?  Why did you move the emacs_value_tag stuff
> from emacs-module.c (which is compiled as part of Emacs) to
> emacs-module.h (which is included by modules, which aren't supposed to
> know about Lisp_Object, they are supposed to use emacs_value instead.
> But not a module cannot be compiled without including lisp.h, which I
> think is wrong.

Because of this

  static mps_res_t
  fix_global_ref (mps_ss_t ss, struct module_global_reference *r)
  {
    MPS_SCAN_BEGIN (ss)
    {
      IGC_FIX_CALL_FN (ss, struct Lisp_Vector, r, fix_vectorlike);
      IGC_FIX12_OBJ (ss, &r->value.v);
    }
    MPS_SCAN_END (ss);
    return MPS_RES_OK;
  }

a longer time ago. What would be a good fix?

Re: MPS: Problem with dynamic modules

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: dancol@dancol.org,  eller.helmut@gmail.com,  emacs-devel@gnu.org
> Date: Mon, 20 May 2024 14:07:19 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I tried to run the emacs-module test, but it doesn't compile here:
> >
> >     CCLD     src/emacs-module-resources/mod-test.dll
> >   make[1]: Entering directory `/d/gnu/git/emacs/feature/test'
> >   In file included from src/emacs-module-resources/mod-test.c:47:
> >   ../src/emacs-module.h:1120:26: error: unknown type name 'Lisp_Object'
> >    1120 | struct emacs_value_tag { Lisp_Object v; };
> > 	|                          ^~~~~~~~~~~
> >   ../src/emacs-module.h:1128:27: error: field 'header' has incomplete type
> >    1128 |   union vectorlike_header header;
> > 	|                           ^~~~~~
> >
> > Does it compile for you?  Why did you move the emacs_value_tag stuff
> > from emacs-module.c (which is compiled as part of Emacs) to
> > emacs-module.h (which is included by modules, which aren't supposed to
> > know about Lisp_Object, they are supposed to use emacs_value instead.
> > But not a module cannot be compiled without including lisp.h, which I
> > think is wrong.
> 
> Because of this
> 
>   static mps_res_t
>   fix_global_ref (mps_ss_t ss, struct module_global_reference *r)
>   {
>     MPS_SCAN_BEGIN (ss)
>     {
>       IGC_FIX_CALL_FN (ss, struct Lisp_Vector, r, fix_vectorlike);
>       IGC_FIX12_OBJ (ss, &r->value.v);
>     }
>     MPS_SCAN_END (ss);
>     return MPS_RES_OK;
>   }
> 
> a longer time ago. What would be a good fix?

Move fix_global_ref (or at least its part that knows about
module_global_reference) to emacs-module.c?

There's a tangled web of data types there, and I don't know which ones
are important (e.g., does IGC_FIX_CALL_FN really need to know the data
type of client_addr?), but I'm quite sure some way of using an opaque
data type can work here.  AFAIU, all you need is to supply to
IGC_FIX_CALL_FN a value that it will pass to fix_global_ref, and it
looks to me like both of them treat those as 'void *' pointers.  So
the actual type is not really important, is it?

Re: MPS: Problem with dynamic modules

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: dancol@dancol.org,  eller.helmut@gmail.com,  emacs-devel@gnu.org
>> Date: Mon, 20 May 2024 14:07:19 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > I tried to run the emacs-module test, but it doesn't compile here:
>> >
>> >     CCLD     src/emacs-module-resources/mod-test.dll
>> >   make[1]: Entering directory `/d/gnu/git/emacs/feature/test'
>> >   In file included from src/emacs-module-resources/mod-test.c:47:
>> >   ../src/emacs-module.h:1120:26: error: unknown type name 'Lisp_Object'
>> >    1120 | struct emacs_value_tag { Lisp_Object v; };
>> > 	|                          ^~~~~~~~~~~
>> >   ../src/emacs-module.h:1128:27: error: field 'header' has incomplete type
>> >    1128 |   union vectorlike_header header;
>> > 	|                           ^~~~~~
>> >
>> > Does it compile for you?  Why did you move the emacs_value_tag stuff
>> > from emacs-module.c (which is compiled as part of Emacs) to
>> > emacs-module.h (which is included by modules, which aren't supposed to
>> > know about Lisp_Object, they are supposed to use emacs_value instead.
>> > But not a module cannot be compiled without including lisp.h, which I
>> > think is wrong.
>> 
>> Because of this
>> 
>>   static mps_res_t
>>   fix_global_ref (mps_ss_t ss, struct module_global_reference *r)
>>   {
>>     MPS_SCAN_BEGIN (ss)
>>     {
>>       IGC_FIX_CALL_FN (ss, struct Lisp_Vector, r, fix_vectorlike);
>>       IGC_FIX12_OBJ (ss, &r->value.v);
>>     }
>>     MPS_SCAN_END (ss);
>>     return MPS_RES_OK;
>>   }
>> 
>> a longer time ago. What would be a good fix?
>
> Move fix_global_ref (or at least its part that knows about
> module_global_reference) to emacs-module.c?
>
> There's a tangled web of data types there, and I don't know which ones
> are important (e.g., does IGC_FIX_CALL_FN really need to know the data
> type of client_addr?), but I'm quite sure some way of using an opaque
> data type can work here.  AFAIU, all you need is to supply to
> IGC_FIX_CALL_FN a value that it will pass to fix_global_ref, and it
> looks to me like both of them treat those as 'void *' pointers.  So
> the actual type is not really important, is it?

Probably not, but we also need MPS data types and macros like
MPS_FIX_CALL. That's a PITA, because it means that wherever that
function wanders, that place will need to include MPS headers, and maybe
macros from igc.

Isn't there something easier? Can emacs_value_tag be changed to
something containing s struct Lisp_X *, for example? I think that's what
native comp does, at least judging from the C that it generates.

Re: MPS: Problem with dynamic modules

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: dancol@dancol.org,  eller.helmut@gmail.com,  emacs-devel@gnu.org
> Date: Mon, 20 May 2024 15:23:21 +0200
> 
> Isn't there something easier? Can emacs_value_tag be changed to
> something containing s struct Lisp_X *, for example? I think that's what
> native comp does, at least judging from the C that it generates.

Andrea, can you help us here, please?

Re: MPS: Problem with dynamic modules

[Andrea Corallo]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: dancol@dancol.org,  eller.helmut@gmail.com,  emacs-devel@gnu.org
>> Date: Mon, 20 May 2024 15:23:21 +0200
>> 
>> Isn't there something easier? Can emacs_value_tag be changed to
>> something containing s struct Lisp_X *, for example? I think that's what
>> native comp does, at least judging from the C that it generates.
>
> Andrea, can you help us here, please?

Sure,

Gerd I'm not sure what you refer to, in native code we don't do any
wrapping different form what we do in C (AFAIR), maybe you've a piece of
generated code you are looking at as example?

  Andrea

Re: MPS: Problem with dynamic modules

[Gerd Möllmann]

Andrea Corallo <acorallo@gnu.org> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>>> Cc: dancol@dancol.org,  eller.helmut@gmail.com,  emacs-devel@gnu.org
>>> Date: Mon, 20 May 2024 15:23:21 +0200
>>>
>>> Isn't there something easier? Can emacs_value_tag be changed to
>>> something containing s struct Lisp_X *, for example? I think that's what
>>> native comp does, at least judging from the C that it generates.
>>
>> Andrea, can you help us here, please?
>
> Sure,
>
> Gerd I'm not sure what you refer to, in native code we don't do any
> wrapping different form what we do in C (AFAIR), maybe you've a piece of
> generated code you are looking at as example?

This is not about wrapping.

emacs_value_tag is a struct containing a Lisp_Object member. The
question is if it could use struct Lisp_X * instead of Lisp_Object for
the member.

Re: MPS: Problem with dynamic modules

[Andrea Corallo]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Andrea Corallo <acorallo@gnu.org> writes:
>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>>>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>>>> Cc: dancol@dancol.org,  eller.helmut@gmail.com,  emacs-devel@gnu.org
>>>> Date: Mon, 20 May 2024 15:23:21 +0200
>>>>
>>>> Isn't there something easier? Can emacs_value_tag be changed to
>>>> something containing s struct Lisp_X *, for example? I think that's what
>>>> native comp does, at least judging from the C that it generates.
>>>
>>> Andrea, can you help us here, please?
>>
>> Sure,
>>
>> Gerd I'm not sure what you refer to, in native code we don't do any
>> wrapping different form what we do in C (AFAIR), maybe you've a piece of
>> generated code you are looking at as example?
>
> This is not about wrapping.
>
> emacs_value_tag is a struct containing a Lisp_Object member. The
> question is if it could use struct Lisp_X * instead of Lisp_Object for
> the member.

I see, but I'm not sure what you mean with

>>>> Isn't there something easier? Can emacs_value_tag be changed to
>>>> something containing s struct Lisp_X *, for example? I think that's what
>>>> native comp does, at least judging from the C that it generates.

we don't' have emacs_value_tag in native code so I'm not sure what you
are referring to.

Thanks

  Andrea

Re: MPS: Problem with dynamic modules

[Gerd Möllmann]

Andrea Corallo <acorallo@gnu.org> writes:

>> emacs_value_tag is a struct containing a Lisp_Object member. The
>> question is if it could use struct Lisp_X * instead of Lisp_Object for
>> the member.
>
> I see, but I'm not sure what you mean with
>
>>>>> Isn't there something easier? Can emacs_value_tag be changed to
>>>>> something containing s struct Lisp_X *, for example? I think that's what
>>>>> native comp does, at least judging from the C that it generates.
>
> we don't' have emacs_value_tag in native code so I'm not sure what you
> are referring to.

We are in the context of dynamic modules, not native code.

Your are using struct Lisp_X * in native code all over the place? As a
placeholder for Lisp_Object. Example from a generated C file:

  struct comp_lisp_symbol_with_position
  {
    long header;
    struct Lisp_X * sym;
    struct Lisp_X * pos;
  };

  struct comp_jmp_buf
  {
    char[192] stuff;
  };

  struct comp_handler
  {
    char[24] pad0;
    struct Lisp_X * val;
    struct comp_handler * next;
    char[20] pad1;
    struct comp_jmp_buf jmp;
    char[36] pad2;
  };

  struct comp_thread_state
  {
    char[96] pad0;
    struct comp_handler * m_handlerlist;
    char[416] pad1;
  };

  struct freloc_link_table
  {
    const void (*) (struct Lisp_X *, struct Lisp_X *) R77726f6e675f747970655f617267756d656e74_wrong_type_argument_0;
    const bool (*) (struct Lisp_X *, int) R68656c7065725f50534555444f564543544f525f54595045505f58554e544147_helper_PSEUDOVECTOR_TYPEP_XUNTAG_0;
    const void (*) (struct Lisp_X *) R707572655f77726974655f6572726f72_pure_write_error_0;
    const struct comp_handler * (*) (struct Lisp_X *, int) R707573685f68616e646c6572_push_handler_0;

I asked Eli if we could do the same in the context of dynamic modules,
here emacs_value_tag so that modules don't need to know Lisp_Object from
lisp.h.

Maybe Eli can explain why he involved you.