From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Pip Cet Newsgroups: gmane.emacs.devel Subject: Re: MPS: Win64 testers? Date: Fri, 23 Aug 2024 13:44:04 +0000 Message-ID: <87o75j73ry.fsf@protonmail.com> References: <87bk1k8mzd.fsf@protonmail.com> <861q2gs8xu.fsf@gnu.org> <877cc88j4p.fsf@protonmail.com> <86seuvre6x.fsf@gnu.org> <8734mv8yb6.fsf@protonmail.com> <8634mvqte0.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="14870"; mail-complaints-to="usenet@ciao.gmane.io" Cc: sebastian@sebasmonia.com, kien.n.quang@gmail.com, emacs-devel@gnu.org, yantar92@posteo.net To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Fri Aug 23 16:19:54 2024 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1shV8s-0003dK-9s for ged-emacs-devel@m.gmane-mx.org; Fri, 23 Aug 2024 16:19:54 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1shV7r-0003j2-2t; Fri, 23 Aug 2024 10:18:51 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1shUaK-000689-O9 for emacs-devel@gnu.org; Fri, 23 Aug 2024 09:44:12 -0400 Original-Received: from mail-40133.protonmail.ch ([185.70.40.133]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1shUaI-0005xM-5q for emacs-devel@gnu.org; Fri, 23 Aug 2024 09:44:12 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1724420647; x=1724679847; bh=gi5sc0V4RRQQA/C3qxrcugbXxT4t6Sp7YQMPLiLZSRQ=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector; b=kvKz9jVwYJqVtgzhKPfhbQWa+g978RL2XOZZQpScO9GncnmNwI3SGgKJ/XvzXWrdX 4h9k2N4ZYHzuw3/xtiRDiWIXYooljj4C/7R4p/dyZGOLd3j5nwtwEntnIyEVEuNxTN PotKwzhQzzX8d8Sv4s+HQVYT0HZwBQMk11eBEh1SD4PJHqL/RsoLM9Ehbg7QrlK1sE SJQdpFsI9vMQOWGDf4UNr0+9889b7Z4OB1wclkNLHkY+DfCSV5eNIU97cGGlaZ55cd iT3gq6c9jcgjjWYF5RZRCjENHo4zGKrYap1UYffrZdVXExxuyuND7vWevXXeQ1dwyB gnEW81nhARBIA== In-Reply-To: <8634mvqte0.fsf@gnu.org> Feedback-ID: 112775352:user:proton X-Pm-Message-ID: 303be339970733473c136c256f5ee5b14ddf043e Received-SPF: pass client-ip=185.70.40.133; envelope-from=pipcet@protonmail.com; helo=mail-40133.protonmail.ch X-Spam_score_int: -10 X-Spam_score: -1.1 X-Spam_bar: - X-Spam_report: (-1.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, FREEMAIL_REPLY=1, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, RCVD_IN_VALIDITY_CERTIFIED_BLOCKED=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=no autolearn_force=no X-Spam_action: no action X-Mailman-Approved-At: Fri, 23 Aug 2024 10:18:49 -0400 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:323085 Archived-At: "Eli Zaretskii" writes: >> Date: Fri, 23 Aug 2024 07:59:13 +0000 >> From: Pip Cet >> Cc: sebastian@sebasmonia.com, kien.n.quang@gmail.com, emacs-devel@gnu.or= g, yantar92@posteo.net >> >> >> 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. >> > >> > But the pointer to 'proc' is passed to GnuTLS for the sole purpose >> > that GnuTLS passes it back to our callback, emacs_gnutls_pull. GnuTLS >> > itself does not (and cannot) use that pointer, for the obvious reasons >> > that it knows nothing about our objects. >> >> That is my understanding as well. I'm not sure why you think that's a >> "but", though. > > Because you seemed to say we were passing the object to GnuTLS for its > own use. Sorry about that. The problem isn't who uses the pointer, it's who stores it (gnutls, in memory unknown to MPS) until it's finally used (by Emacs). >> A pointer to memory managed in a copying pool, stored outside of >> MPS-managed memory, as 'proc' presumably is (in malloc()ed memory, I >> would guess, or maybe in a static location somewhere), must be pinned >> somehow, so MPS does not move it during GC. > > Then let's pin it. Okay. > But my bother is that we will need to similarly pin many other > pointers and objects, or risk segfaults. Indeed. There are quite a few changes along the same lines in the scratch/igc branch, and it makes me think we need a slightly more general infrastructure to turn a pointer (or Lisp_Object) known to Emacs into a void * suitable for passing as a cookie to an external library, and then for turning the cookie passed to a callback by that external library back into a pointer (or Lisp_Object) the callback (in Emacs) can use. > Maybe it means that MPS and similar igc systems are not suitable for > Emacs. I believe said infrastructure would be generally useful, because it allows us to catch another class of subtle bugs that may appear without MPS: passing a pointer to an external library which is freed by our traditional GC, which would cause very similar segfaults (if anything, much trickier to debug) without MPS. IOW, it's always worth our time to think about why something we pass as a cookie to an external callback cannot be garbage-collected and/or copied, regardless of which garbage collector is in use. The price we'd pay is to require such cookies to be explicitly deregistered, but we'd gain a very useful feature (maybe for --enable-checking, maybe enabled by default): a clear and loud warning informing us that a cookie is still registered for an object we just tried to GC. So I think this could improve Emacs in general, even for people who will never use MPS. >> > And what do you mean by "asynchronous calls"? For example, you can >> > see in w32font.c that we call certain Windows APIs which work by >> > enumerating objects of some kind and calling our callback for each >> > such object (the callback that does with each object what it needs to >> > do). Is this an "asynchronous call"? If not, what is? >> >> It doesn't look asynchronous to me, though I'm not sure we're talking >> about the same code. The LPARAM arguments referring to stack locations >> are fine, because the stack is known to MPS. > > I mean this: > > EnumFontFamiliesEx (dc, &match_data->pattern, > (FONTENUMPROC) add_font_entity_to_list, > (LPARAM) match_data, 0); > > where add_font_entity_to_list is our callback which is called for > every font family and decides whether it matches the font spec; if so, > the callback adds the family to a list that is then used by the caller > of EnumFontFamiliesEx. This particular call is safe. 'match_data' lives on the stack, MPS treats the stack conservatively as pinning every object to which it might possibly refer, thus 'match_data->pattern' cannot be moved. >> The problem here is that 'proc' might move. One solution is to make the >> cookie a pointer to memory known to MPS, which contains the actual >> pointer to the Lisp_Process. Note that it is not strictly necessary for >> the pointer to be allocated with igc_xzalloc_ambig, we could use the >> _raw_exact variant. > > Why not tell MPS not to move this pointer instead? It sounds like a > simpler solution than the change you proposed. The change I proposed is redundant: it tells MPS not to move the pointer, and it uses a pointer-to-a-pointer to catch potential moves. Only one part of that is needed, but in both cases, we need to remember to un-pin/free something. > The solution you propose will complicate large parts of the Emacs code > by using double indirection; Indeed, which is why I'm seriously proposing that cookies for use by external libraries be generated via an opaque API that has different implementations: 1. for standard Emacs, where it just returns the pointer 2. for MPS, where it returns a pointer to the pointer 3. for debug builds, where it registers the object so we warn if we try to GC it, then returns the pointer. > pinning a pointer is much simpler. Or am I missing something? Pinning it certainly is, remembering to un-pin it when we're done with it is the hard part. And also something we should always do, since forgetting to unpin it would result in memory leaks and slower GCs for the lifetime of the Emacs session. >> There are other solutions which may be better. > > I certainly hope so, since auditing all of our sources for such > gotchas is an impossible job. I think I disagree that it's impossible. We've already caught many of them (the PGTK and Lucid toolkit fixes were essentially the same problem), and auditing our usage of such callbacks is necessary anyway because traditional GC might free (but not move) pointers prematurely, resulting in subtle bugs which might not be caught for years. I think working on this issue is worth our time and has the potential to improve Emacs generally, independently of the MPS branch. The price we'd pay is that we would be forced to make explicit the point at which an external library stops using our cookie/opaque pointer/LPARAM/whatever you want to call it; put differently, it would force us to think about that question at all, while right now it's possible just to hope the Emacs object will live for long enough. I believe the cookies are generally of one of the following types: - void * - gpointer - LPARAM - WPARAM? And grepping for these returns a manageable but large set of hits (1865, most of which are trivial). Pip