From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Sergio Durigan Junior Newsgroups: gmane.emacs.bugs Subject: bug#61880: Native compilation fails to generate trampolines on certain scenarios Date: Thu, 02 Mar 2023 18:50:50 -0500 Message-ID: <87a60uemqt.fsf@sergiodj.net> References: <877cw1l455.fsf@sergiodj.net> <83sfeofyi6.fsf@gnu.org> <87edq8hxom.fsf@sergiodj.net> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="15426"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: Eli Zaretskii , 61880@debbugs.gnu.org To: Andrea Corallo Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Mar 03 00:51:23 2023 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 1pXshl-0003kh-Vw for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 03 Mar 2023 00:51:22 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pXshU-0006aB-0G; Thu, 02 Mar 2023 18:51:04 -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 1pXshS-0006Vx-Eo for bug-gnu-emacs@gnu.org; Thu, 02 Mar 2023 18:51:02 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pXshS-0002jl-2k for bug-gnu-emacs@gnu.org; Thu, 02 Mar 2023 18:51:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1pXshR-0006dD-IO for bug-gnu-emacs@gnu.org; Thu, 02 Mar 2023 18:51:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Sergio Durigan Junior Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 02 Mar 2023 23:51:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 61880 X-GNU-PR-Package: emacs Original-Received: via spool by 61880-submit@debbugs.gnu.org id=B61880.167780106025484 (code B ref 61880); Thu, 02 Mar 2023 23:51:01 +0000 Original-Received: (at 61880) by debbugs.gnu.org; 2 Mar 2023 23:51:00 +0000 Original-Received: from localhost ([127.0.0.1]:59227 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pXshP-0006cy-R5 for submit@debbugs.gnu.org; Thu, 02 Mar 2023 18:51:00 -0500 Original-Received: from mail.sergiodj.net ([167.114.15.217]:59166) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pXshN-0006cf-AM for 61880@debbugs.gnu.org; Thu, 02 Mar 2023 18:50:58 -0500 Original-Received: from localhost (unknown [IPv6:2607:f2c0:ed84:d00:694f:48a1:de49:29be]) by mail.sergiodj.net (Postfix) with ESMTPSA id 9C096A007D; Thu, 2 Mar 2023 18:50:50 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=sergiodj.net; s=20160602; t=1677801050; bh=n91snxJsO2sKH5siz43XQznH/BG6tcyzvBGqDkJZWNw=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=K+dRLN09hE9Y43rIvjX+ubhI+JU4idy4PYIIaYn8fp3fvWnZC7j7eyW+aKTW6YKSM GY6/BJuEo0kWGVemtRkAefWyrrNUc16xlnu3t72bQtGlMWxoUP5jh0C9uskq0ktZwV Wp2N5njX35PCHwcP3mR0SHuH1LorMBZPNvxI9MJE= In-Reply-To: (Andrea Corallo's message of "Thu, 02 Mar 2023 11:47:43 +0000") X-URL: http://blog.sergiodj.net 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:257184 Archived-At: On Thursday, March 02 2023, Andrea Corallo wrote: > Sergio Durigan Junior writes: > >> On Wednesday, March 01 2023, Andrea Corallo wrote: >> >>> Eli Zaretskii writes: >>> >>>>> From: Sergio Durigan Junior >>>>> Date: Tue, 28 Feb 2023 19:13:58 -0500 >>>>> >>>>> While investigating a few bugs affecting Debian's and Ubuntu's Emacs >>>>> packages (for example, >>>>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1028725), I stumbled >>>>> upon a problem that's affecting native compilation on Emacs 28.1+, >>>>> currently reproducible with git master as well. >>>>> >>>>> I haven't been able to fully understand why the problem is happening, >>>>> but when there are two primitive functions (that would become >>>>> trampolines) being used sequentially, Emacs doesn't generate the >>>>> corresponding .eln file for the second function. >>>>> >>>>> I spent some time investigating the problem and came up with a "minimal" >>>>> reproducer: >>>>> >>>>> --8<---------------cut here---------------start------------->8--- >>>>> (require 'cl-lib) >>>>> >>>>> (defmacro foo--flet (funcs &rest body) >>>>> "Like `cl-flet' but with dynamic function scope." >>>>> (declare (indent 1)) >>>>> (let* ((names (mapcar #'car funcs)) >>>>> (lambdas (mapcar #'cdr funcs)) >>>>> (gensyms (cl-loop for name in names >>>>> collect (make-symbol (symbol-name name))))) >>>>> `(let ,(cl-loop for name in names >>>>> for gensym in gensyms >>>>> collect `(,gensym (symbol-function ',name))) >>>>> (unwind-protect >>>>> (progn >>>>> ,@(cl-loop for name in names >>>>> for lambda in lambdas >>>>> for body = `(lambda ,@lambda) >>>>> collect `(setf (symbol-function ',name) ,body)) >>>>> ,@body) >>>>> ,@(cl-loop for name in names >>>>> for gensym in gensyms >>>>> collect `(setf (symbol-function ',name) ,gensym)))))) >>>>> >>>>> (defun bar (file) >>>>> (and (file-exists-p file) (file-readable-p file))) >>>>> >>>>> (defun test () >>>>> (foo--flet ((file-exists-p (file) t) >>>>> (file-readable-p (file) nil)) >>>>> (message "%s" (bar "/home/sergio/.lesshst")))) >>>>> --8<---------------cut here---------------end--------------->8--- >>>>> >>>>> When I run it using the following Emacs: >>>>> >>>>> --8<---------------cut here---------------start------------->8--- >>>>> GNU Emacs 30.0.50 >>>>> Development version 68cc286c0495 on master branch; build date 2023-02-28. >>>>> --8<---------------cut here---------------end--------------->8--- >>>>> >>>>> here is the output I see: >>>>> >>>>> --8<---------------cut here---------------start------------->8--- >>>>> $ emacs -batch -Q -l t.el -f test -L . >>>>> Error: native-lisp-load-failed ("file does not exists" "/home/sergio/.emacs.d/eln-cache/30.0.50-23de7b18/subr--trampoline-66696c652d7265616461626c652d70_file_readable_p_0.eln") >>>>> debug-early-backtrace() >>>>> debug-early(error (native-lisp-load-failed "file does not exists" "/home/sergio/.emacs.d/eln-cache/30.0.50-23de7b18/subr--trampoline-66696c652d7265616461626c652d70_file_readable_p_0.eln")) >>>>> native-elisp-load("/home/sergio/.emacs.d/eln-cache/30.0.50-23de7b18/subr--trampoline-66696c652d7265616461626c652d70_file_readable_p_0.eln") >>>>> comp-trampoline-search(file-readable-p) >>>>> comp-subr-trampoline-install(file-readable-p) >>>>> fset(file-readable-p (lambda (file) nil)) >>>>> (progn (fset 'file-exists-p #'(lambda (file) t)) (fset 'file-readable-p #'(lambda (file) nil)) (message "%s" (bar "/home/sergio/.lesshst"))) >>>>> (unwind-protect (progn (fset 'file-exists-p #'(lambda (file) t)) (fset 'file-readable-p #'(lambda (file) nil)) (message "%s" (bar "/home/sergio/.lesshst"))) (fset 'file-exists-p file-exist >>>>> s-p) (fset 'file-readable-p file-readable-p)) >>>>> (let ((file-exists-p (symbol-function 'file-exists-p)) (file-readable-p (symbol-function 'file-readable-p))) (unwind-protect (progn (fset 'file-exists-p #'(lambda (file) t)) (fset 'file-re >>>>> adable-p #'(lambda (file) nil)) (message "%s" (bar "/home/sergio/.lesshst"))) (fset 'file-exists-p file-exists-p) (fset 'file-readable-p file-readable-p))) >>>>> test() >>>>> command-line-1(("-l" "t.el" "-f" "test" "-L" ".")) >>>>> command-line() >>>>> normal-top-level() >>>>> Native elisp load failed: "file does not exists", "/home/sergio/.emacs.d/eln-cache/30.0.50-23de7b18/subr--trampoline-66696c652d7265616461626c652d70_file_readable_p_0.eln" >>>>> --8<---------------cut here---------------end--------------->8--- >>>>> >>>>> Do note that this is already affecting a few packages, like buttercup >>>>> (see https://github.com/jorgenschaefer/emacs-buttercup/issues/230) and >>>>> emacs-web-server, for example. >>>>> >>>>> Please let me know if you need more information regarding the problem. >>>> >>>> Thanks. >>>> >>>> Andrea, can you please look into this? I guess if this happens in >>>> Emacs 28 and on master, it also affects Emacs 29, so I hope this can >>>> be fixed quickly and safely. TIA. >>>> >>> >>> Yep, sorry the IP of my mail provider is (temporary?) banned and I don't >>> receive nor emacs-bugs nor emacs-devel since a couple of days :( thanks >>> for Ccing me. >>> >>> So what should be going on here is that `file-exists-p' gets redefined >>> with a non working mock function while we are trying to compile a >>> trampoline for `file-readable-p' (it's redefined just afterward). >> >> Thank you for taking the time to reply and investigate this problem. >> >>> I don't think there is a trivial fix for this, we should rewrite >>> `comp-trampoline-search' in C in order to have it not sensitive to the >>> redefinition of `file-exists-p', or define another primitive like >>> `comp-file-exists-p' (that calls directly Ffile_exists_p from C) and use >>> that in `comp-trampoline-search'. This would cover this specific case >>> but potentially not others. >> >> Forgive my ignorance, but wouldn't we need to do that for every >> primitive that can be compiled into a trampoline? > > No that's only for primitives used by the trampoline machinery (and > specifically the part written in lisp). Once `file-exists-p' is > crippled the trampoline machinery is broken for all following primitives > being redefined. OK, understood. What's strange to me is the fact that there are other primitives that are affected by this problem, like 'buffer-file-name' and 'file-readable-p', but they don't seem to call 'file-exists-p'. >> I say that because >> the error I've encountered above refers to 'file-readable-p', which >> doesn't seem to call 'file-exists-p'. FWIW, I did encounter the same >> problem with 'file-exists-p' and 'buffer-file-name' as well, which is >> the reason why I think solely having a 'comp-file-exists-p' wouldn't be >> enough. >> >>> Fact is, Emacs can't be robust against the redefinition of all >>> primitives (actually never was), the programmer that redefines >>> primitives should be ready to understand the underlying Emacs machinery, >>> and with native compilation this machinery changed a bit. >> >> I understand where you're coming from, but it's also important to note >> that this behaviour was accepted without problems until the native >> compilation feature came about, so it is understandable that we are >> getting a lot of confusing people wondering why their tests started >> failing now. I believe there should be more emphasis in the >> documentation that this problem can creep in, especially for those who >> are relying on redefinitions for testing purposes. > > Agree > >>> So two options: >>> >>> * The redefinition of `file-exists-p' is tipically done for test >>> purposes only, we accept that and for this case we suggest to run >>> these specific tests setting `native-comp-enable-subr-trampolines' to >>> nil >> >> This is what I'm currently doing in Debian/Ubuntu, and will start >> suggesting upstream maintainers to do the same. > > Note, I think this should be suggested only for tests redefining > `file-exists-p'. What about 'buffer-file-name' and 'file-readable-p'? -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible https://sergiodj.net/