From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#75322: SAFE_ALLOCA assumed to root Lisp_Objects/SSDATA(string) Date: Sat, 04 Jan 2025 20:02:42 +0200 Message-ID: <86y0zqbgot.fsf@gnu.org> References: <87jzbbke6u.fsf@protonmail.com> <86sepzf4h3.fsf@gnu.org> <87a5c6j0qn.fsf@protonmail.com> <86jzbad735.fsf@gnu.org> <877c7aha9n.fsf@protonmail.com> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="31915"; mail-complaints-to="usenet@ciao.gmane.io" Cc: gerd.moellmann@gmail.com, 75322@debbugs.gnu.org To: Pip Cet Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Jan 04 19:04:18 2025 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1tU8VW-0008AK-8U for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 04 Jan 2025 19:04:18 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1tU8VL-00036j-JW; Sat, 04 Jan 2025 13:04:08 -0500 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 1tU8VI-00036Q-Lu for bug-gnu-emacs@gnu.org; Sat, 04 Jan 2025 13:04:04 -0500 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1tU8VG-0005Lc-Vs for bug-gnu-emacs@gnu.org; Sat, 04 Jan 2025 13:04:03 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=References:In-Reply-To:From:Date:To:Subject; bh=lcPBe0uDI1zMMDREtvxafqOmBRSeFgCBG1PCul83AKk=; b=DXpcmD2N7osxlQwIb4uL8mEj6WOkMSBpHK90neFNarduAuOHTNJ9ax5EfPbsuDpNQGxs6SSW7RGaN/lN4F1fJ+Idhbp61LiCp/F9NA0b3ePRhN7tXOgl9LXVDdakVmpk4mFyhq5LHr0H1V/8X9XHXIOSJw6Tmk4MO44jpJvTuT0zND7lg7ETWSjyJ2Kr7QMDmoYvKJAQV5uXoJCfZDUhLIplA2+tVW9+FdTs8YM4u+RV8zv7HHxjL7AcqVGj+4HgmaACeoh6WJnQ1uR4nVJXP3TlcdKaAFZq4Hp79ipwha39woyEUadFJ5Kj0i0Uy0PfIFrLUjLfZo3Fdq6xbJhqkQ==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1tU8VG-00050r-Hz for bug-gnu-emacs@gnu.org; Sat, 04 Jan 2025 13:04:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 04 Jan 2025 18:04:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 75322 X-GNU-PR-Package: emacs Original-Received: via spool by 75322-submit@debbugs.gnu.org id=B75322.173601380619218 (code B ref 75322); Sat, 04 Jan 2025 18:04:02 +0000 Original-Received: (at 75322) by debbugs.gnu.org; 4 Jan 2025 18:03:26 +0000 Original-Received: from localhost ([127.0.0.1]:57222 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1tU8Uf-0004zu-AF for submit@debbugs.gnu.org; Sat, 04 Jan 2025 13:03:25 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:38156) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1tU8Uc-0004ze-AO for 75322@debbugs.gnu.org; Sat, 04 Jan 2025 13:03:23 -0500 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1tU8UW-0005B2-NJ; Sat, 04 Jan 2025 13:03:16 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=lcPBe0uDI1zMMDREtvxafqOmBRSeFgCBG1PCul83AKk=; b=escBBF7DzipX lBuPXiVK9+5GENweizU+Gh2Djw/C34n0wx0PmrH7Pz0jDuHs5Xy6CrSVCwyE6xkaUZ/jgW4+9ZwsO X5rp0kfWJIouNwVA8oCGX2vtqs+J63RAwkXiErSgaYT4cO5U23g4iMs3JSVWaxyAvvXhzmpmFwg5o 7X1d2CGBfSScXJp4hfo1Uf8xe4CGBg8jAVzBOiLz+ZuGm93R/ZEx3GQwtnAoT3onDIHIxiCJqTq3G UGMd2hAJ+tKVPL9S/Q5FYe51Q7CWa57UkMoylSvwBK+PwOLuQ3GCOAJV+XubHGOUZ5Or3CiiT8paI llyqK9OH+N04NmP3vejK6w==; In-Reply-To: <877c7aha9n.fsf@protonmail.com> (message from Pip Cet on Sat, 04 Jan 2025 15:26:01 +0000) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:298443 Archived-At: > Date: Sat, 04 Jan 2025 15:26:01 +0000 > From: Pip Cet > Cc: gerd.moellmann@gmail.com, 75322@debbugs.gnu.org > > "Eli Zaretskii" writes: > > > AFAICT, expand-file-name is called before we start using the SSDATA of > > strings in the args[] array. Or what did I miss? > > You're right, thanks. I got confused between args and new_argv. > > The next thing I'd look at is the final call to ENCODE_FILE, called > after new_argv is set up; this ends up in encode_coding_object, which > calls safe_funcall in what seems to me to be an unlikely but possible > code path. I assume that's unsafe (the safe_ refers to redisplay, not > GC, IIUC)? ENCODE_FILE can indeed trigger GC in rare cases, but I see only one such call: path = ENCODE_FILE (path); We could simply move this to before the loop that sets up new_argv[]. > While maybe_quit is "safe" because it inhibits GC, I believe it can > still call the debugger, which might require more memory than is > available until GC is re-enabled. maybe_quit is not the problem, the problem AFAIU is that any encoding could have pre-write-conversion function written in Lisp. > >> Yes, make_environment_block does say its callers can't run GC, but > >> call_process doesn't indicate when and how it establishes a no-GC > >> assumption. > > > > What would be needed to indicate that? > > I'd prefer a pair of macros (no-ops in regular builds) to comments, but > there is no obvious best solution here. Sorry, I don't understand: why macros? Do we use something like that anywhere else in Emacs? > My proposal would be to remove most (ideally, all, and then we're done) > no-GC assumptions, and put the few ones that must remain into separate > function bodies for the no-GC / possible-GC cases. Then we can put the > no-GC function bodies into a separate section and prohibit references > from no-GC to possible-GC functions, and have the linker check that. First, the techniques that rely on separate segments are non-portable, so not all platforms will support them. More importantly, I'm afraid the amount of code where we currently don't expect GC is much more than you seem to imagine, so I don't think the above is practical. In any case, the starting point is to audit all the places where GC can happen, and that needs to be done anyway if we want to do it thoroughly and proactively (as opposed to only when someone reports a bug). We can delay the decision what to do with these places to later, when we understand better the magnitude of the problem and its various aspects (in terms of several different effects that GC can have on various objects). > >> As we agreed, code should be written to assume GC can strike at any > >> time. > > > > I don't think we agreed to that. At least I didn't, not in this > > general form. It would be a huge job to make all of our code comply > > with this. > > I said "That's what you should think: GC can strike at any time", and > you said "The same is true with the old GC". I misread that as > agreement. I only meant that MPS is not very different from the old GC in this regard, that's all. > >> IIUC, Gerd explained that the old GC can still move the string *data* > >> used in that structure, even if the string metadata stays in place. > > > > If string data is moved, then accessing the old pointer would trigger > > the barrier and MPS will do its thing, not? > > Sorry, but I think I'm confused here. > > IIUC, MPS doesn't currently use barriers on data that is moved (it > could, so the data is copied lazily, but I don't think that's what you > meant), it uses barriers on data that contains references that may not > have been fixed. The safe thing is to re-initialize the pointer from the string whenever we think GC could happen, and otherwise make sure GC cannot happen. > If a pointer to "old" data is ever exposed to Emacs, we lose, because > MPS will reuse the memory for new data, which might be behind a barrier. > > If we ever do: > > static Lisp_Object unmarked; > unmarked = string; > ... trigger GC here ... > puts (SDATA (unmarked); > > the most likely outcome (thanks to Gerd for the hint) is that > nonsensical data is printed Are you sure? AFAIU, referencing 'unmarked' is safe, even after GC. The below is indeed unsafe: char *p = SDATA (unmarked); ... trigger GC here ... puts (p); > > To clarify, I was trying to understand whether the error message > > reported by Ihor in another thread could have happened because of GC > > in this are of the code. > > I currently think that Ihor's test case calls execve () with nonsensical > "environment" strings a lot, and once in a while they'll even be behind > the barrier which causes an EFAULT. Before we agree that this could be the reason, we'd need to find the places where GC could reuse the memory of a live string, while that string appears in some live data structure, and as long as no GC can happen between the time we put the SDATA pointers into the environment array and the time posix_spawn returns.