Re: MPS: Win64 testers?

[Pip Cet <pipcet@protonmail.com>]

"Eli Zaretskii" <eliz@gnu.org> writes:

>> Date: Thu, 22 Aug 2024 17:51:38 +0000
>> From: Pip Cet <pipcet@protonmail.com>
>> Cc: Quang Kien Nguyen <kien.n.quang@gmail.com>, Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org, Ihor Radchenko <yantar92@posteo.net>
>>
>> > I was running this version today:
>> >
>> > https://github.com/kiennq/emacs-build/releases/tag/v31.305.20240819.4794ea5
>> >
>> > Had two different crashes, hope that these backtraces are more useful
>> > (thanks again to Pip and Ihor for their help)
>>
>> Thank you, that's very useful.  Indeed, it looks like the
>> Windows-specific code hands off a pointer to MPS-managed memory to
>> GNUTLS to use as a cookie, while other OSes use an FD number cast to an
>> intptr_t cast to void *.
>

I meant this code in gnutls.c:

  if (proc->gnutls_initstage < GNUTLS_STAGE_TRANSPORT_POINTERS_SET)
    {
# ifdef WINDOWSNT
      /* On W32 we cannot transfer socket handles between different runtime
	 libraries, so we tell GnuTLS to use our special push/pull
	 functions.  */
      gnutls_transport_set_ptr2 (state,
				 (gnutls_transport_ptr_t) proc,
				 (gnutls_transport_ptr_t) proc);
      gnutls_transport_set_push_function (state, &emacs_gnutls_push);
      gnutls_transport_set_pull_function (state, &emacs_gnutls_pull);
# else
      /* This is how GnuTLS takes sockets: as file descriptors passed
	 in.  For an Emacs process socket, infd and outfd are the
	 same but we use this two-argument version for clarity.  */
      gnutls_transport_set_ptr2 (state,
				 (void *) (intptr_t) proc->infd,
				 (void *) (intptr_t) proc->outfd);
      if (proc->is_non_blocking_client)
	gnutls_transport_set_errno_function (state,
					     emacs_gnutls_nonblock_errno);
# endif

      proc->gnutls_initstage = GNUTLS_STAGE_TRANSPORT_POINTERS_SET;
    }


That hands a pointer to 'proc' to gnutls, and if 'proc' subsequently
moves, emacs_gnutls_push or emacs_gnutls_pull will try to dereference a
stale pointer.

> You are talking about this line in emacs_gnutls_pull:
>
>   emacs_gnutls_transport_set_errno (process->gnutls_state,
>                                     errno == EWOULDBLOCK ? EAGAIN : errno);
>
> which causes us to call GnuTLS thusly:
>
>   gnutls_transport_set_errno (state, err);
>
> is that right?

> Does this mean we will have to audit all our calls to
> external libraries and make sure we never pass to them any part of any
> Lisp object?  That's a tough requirement; it's very easy to miss some
> such use, which then triggers segfaults.  Am I missing something?

Synchronous calls do not need to be changed, only asynchronous callbacks
that currently use a pointer to a Lisp object as their cookie.

> Btw, I tried using EWW in my MinGW MPS build, and it didn't crash.  Is
> it possible to have a reproducible recipe for this?

I'll try coming up with one, though that's probably just going to mean
adding (igc-collect) after the gnutls handshake somewhere in the Lisp
code.

By the way, I fully agree more debugging information would be helpful.

Pip