From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Lars Ingebrigtsen Newsgroups: gmane.emacs.bugs Subject: bug#46988: 28.0.50; Documenting and verifying assumptions about C code not calling quit or GCing Date: Mon, 20 Jun 2022 03:41:42 +0200 Message-ID: <87zgi8p0vd.fsf@gnus.org> References: Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="18891"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) Cc: 46988@debbugs.gnu.org, Mattias =?UTF-8?Q?Engdeg=C3=A5rd?= , Stefan Monnier To: Pip Cet Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Jun 20 03:42:14 2022 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 1o36Qg-0004kh-Im for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 20 Jun 2022 03:42:14 +0200 Original-Received: from localhost ([::1]:38296 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1o36Qf-0005w0-9q for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 19 Jun 2022 21:42:13 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:59976) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1o36QU-0005uu-69 for bug-gnu-emacs@gnu.org; Sun, 19 Jun 2022 21:42:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:59693) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1o36QT-0002a0-UG for bug-gnu-emacs@gnu.org; Sun, 19 Jun 2022 21:42:01 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1o36QT-0000RE-Ss for bug-gnu-emacs@gnu.org; Sun, 19 Jun 2022 21:42:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Lars Ingebrigtsen Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 20 Jun 2022 01:42:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 46988 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 46988-submit@debbugs.gnu.org id=B46988.16556893181673 (code B ref 46988); Mon, 20 Jun 2022 01:42:01 +0000 Original-Received: (at 46988) by debbugs.gnu.org; 20 Jun 2022 01:41:58 +0000 Original-Received: from localhost ([127.0.0.1]:53590 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1o36QQ-0000QG-0D for submit@debbugs.gnu.org; Sun, 19 Jun 2022 21:41:58 -0400 Original-Received: from quimby.gnus.org ([95.216.78.240]:56720) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1o36QO-0000Pl-Bc for 46988@debbugs.gnu.org; Sun, 19 Jun 2022 21:41:57 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnus.org; s=20200322; h=Content-Type:MIME-Version:Message-ID:In-Reply-To:Date: References:Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=JJjLbUyHi30oT3LPWlPJVHuDlDS55n5Pdxpwc5AzTCQ=; b=hJnVEUwj7+PMrEttOZQ97E7Swr pchBP2rWflnvYwNVeB0EekVXMRKIVWRtMAbSe+mPeLQgqhJSDUNfHmW4wWH8PTzjEo2ikaIyEI0zx SDnU8s0n58p6ZZl4MgDrxXYqccB80UKfIegrrf+q9q41OwpH9vugrZ7BDCcPY29QPm28=; Original-Received: from [84.212.220.105] (helo=xo) by quimby.gnus.org with esmtpsa (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1o36QB-0004eM-IR; Mon, 20 Jun 2022 03:41:46 +0200 In-Reply-To: (Pip Cet's message of "Sun, 7 Mar 2021 14:06: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" Xref: news.gmane.io gmane.emacs.bugs:234841 Archived-At: Pip Cet writes: > Patch attached. It assumes the standard stack growth direction, and > that __builtin_frame_address (0) is available and works. Uses GCC's > __attribute__ ((cleanup (...))). > > My point here is that the technical implementation isn't the problem, > the question is whether we're disciplined enough to run with checking > enabled and react to bug reports about the fatal error being thrown. I've respun the patch for the current trunk, and I wonder whether anybody has any comments here (so I've added Stefan and Mattias to the CCs). I think if we add this, it should be enabled only if the build is configured with --enable-checking. diff --git a/src/alloc.c b/src/alloc.c index 55e18ecd77..276267ef10 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -7019,6 +7019,7 @@ #define CHECK_ALLOCATED_AND_LIVE_SYMBOL() ((void) 0) void mark_object (Lisp_Object obj) { + DONT_ALLOW_GC (); ptrdiff_t sp = mark_stk.sp; mark_stack_push_value (obj); process_mark_stack (sp); @@ -7921,3 +7922,5 @@ syms_of_alloc (void) enum defined_HAVE_PGTK defined_HAVE_PGTK; } const EXTERNALLY_VISIBLE gdb_make_enums_visible = {0}; #endif /* __GNUC__ */ + +struct dont_allow_gc *global_dont_allow_gc; diff --git a/src/bytecode.c b/src/bytecode.c index fa068e1ec6..6d3b3fdb98 100644 --- a/src/bytecode.c +++ b/src/bytecode.c @@ -958,7 +958,8 @@ #define DEFINE(name, value) [name] = &&insn_ ## name, type = CONDITION_CASE; pushhandler: { - struct handler *c = push_handler (POP, type); + struct handler *c = push_handler (POP, type, + __builtin_frame_address (0)); c->bytecode_dest = FETCH2; c->bytecode_top = top; diff --git a/src/emacs-module.c b/src/emacs-module.c index 1c392d65df..87d9fe070a 100644 --- a/src/emacs-module.c +++ b/src/emacs-module.c @@ -272,7 +272,7 @@ #define MODULE_HANDLE_NONLOCAL_EXIT(retval) \ if (module_non_local_exit_check (env) != emacs_funcall_exit_return) \ return retval; \ struct handler *internal_handler = \ - push_handler_nosignal (Qt, CATCHER_ALL); \ + push_handler_nosignal (Qt, CATCHER_ALL, __builtin_frame_address (0)); \ if (!internal_handler) \ { \ module_out_of_memory (env); \ diff --git a/src/eval.c b/src/eval.c index 346dff8bdc..f04b814c0e 100644 --- a/src/eval.c +++ b/src/eval.c @@ -236,7 +236,7 @@ init_eval (void) which would otherwise leak every time we unwind back to top-level. */ handlerlist_sentinel = xzalloc (sizeof (struct handler)); handlerlist = handlerlist_sentinel->nextfree = handlerlist_sentinel; - struct handler *c = push_handler (Qunbound, CATCHER); + struct handler *c = push_handler (Qunbound, CATCHER, __builtin_frame_address (0)); eassert (c == handlerlist_sentinel); handlerlist_sentinel->nextfree = NULL; handlerlist_sentinel->next = NULL; @@ -1200,7 +1200,7 @@ internal_catch (Lisp_Object tag, Lisp_Object (*func) (Lisp_Object), Lisp_Object arg) { /* This structure is made part of the chain `catchlist'. */ - struct handler *c = push_handler (tag, CATCHER); + struct handler *c = push_handler (tag, CATCHER, __builtin_frame_address (0)); /* Call FUNC. */ if (! sys_setjmp (c->jmp)) @@ -1274,6 +1274,9 @@ unwind_to_catch (struct handler *catch, enum nonlocal_exit type, lisp_eval_depth = catch->f_lisp_eval_depth; set_act_rec (current_thread, catch->act_rec); + void *sp = catch->sp; + while (global_dont_allow_gc && (void *)global_dont_allow_gc < sp) + global_dont_allow_gc = global_dont_allow_gc->prev; sys_longjmp (catch->jmp, 1); } @@ -1283,6 +1286,7 @@ DEFUN ("throw", Fthrow, Sthrow, 2, 2, 0, attributes: noreturn) (register Lisp_Object tag, Lisp_Object value) { + DONT_ALLOW_GC (); struct handler *c; if (!NILP (tag)) @@ -1405,7 +1409,8 @@ internal_lisp_condition_case (Lisp_Object var, Lisp_Object bodyform, Lisp_Object condition = CONSP (clause) ? XCAR (clause) : Qnil; if (!CONSP (condition)) condition = list1 (condition); - struct handler *c = push_handler (condition, CONDITION_CASE); + struct handler *c = push_handler (condition, CONDITION_CASE, + __builtin_frame_address (0)); if (sys_setjmp (c->jmp)) { Lisp_Object val = handlerlist->val; @@ -1472,7 +1477,8 @@ internal_lisp_condition_case (Lisp_Object var, Lisp_Object bodyform, internal_condition_case (Lisp_Object (*bfun) (void), Lisp_Object handlers, Lisp_Object (*hfun) (Lisp_Object)) { - struct handler *c = push_handler (handlers, CONDITION_CASE); + struct handler *c = push_handler (handlers, CONDITION_CASE, + __builtin_frame_address (0)); if (sys_setjmp (c->jmp)) { Lisp_Object val = handlerlist->val; @@ -1496,7 +1502,8 @@ internal_condition_case_1 (Lisp_Object (*bfun) (Lisp_Object), Lisp_Object arg, Lisp_Object handlers, Lisp_Object (*hfun) (Lisp_Object)) { - struct handler *c = push_handler (handlers, CONDITION_CASE); + struct handler *c = push_handler (handlers, CONDITION_CASE, + __builtin_frame_address (0)); if (sys_setjmp (c->jmp)) { Lisp_Object val = handlerlist->val; @@ -1523,7 +1530,8 @@ internal_condition_case_2 (Lisp_Object (*bfun) (Lisp_Object, Lisp_Object), Lisp_Object handlers, Lisp_Object (*hfun) (Lisp_Object)) { - struct handler *c = push_handler (handlers, CONDITION_CASE); + struct handler *c = push_handler (handlers, CONDITION_CASE, + __builtin_frame_address (0)); if (sys_setjmp (c->jmp)) { Lisp_Object val = handlerlist->val; @@ -1552,7 +1560,8 @@ internal_condition_case_n (Lisp_Object (*bfun) (ptrdiff_t, Lisp_Object *), ptrdiff_t nargs, Lisp_Object *args)) { - struct handler *c = push_handler (handlers, CONDITION_CASE); + struct handler *c = push_handler (handlers, CONDITION_CASE, + __builtin_frame_address (0)); if (sys_setjmp (c->jmp)) { Lisp_Object val = handlerlist->val; @@ -1579,7 +1588,8 @@ internal_condition_case_n (Lisp_Object (*bfun) (ptrdiff_t, Lisp_Object *), internal_catch_all (Lisp_Object (*function) (void *), void *argument, Lisp_Object (*handler) (enum nonlocal_exit, Lisp_Object)) { - struct handler *c = push_handler_nosignal (Qt, CATCHER_ALL); + struct handler *c = push_handler_nosignal (Qt, CATCHER_ALL, + __builtin_frame_address (0)); if (c == NULL) return Qcatch_all_memory_full; @@ -1601,16 +1611,18 @@ internal_catch_all (Lisp_Object (*function) (void *), void *argument, } struct handler * -push_handler (Lisp_Object tag_ch_val, enum handlertype handlertype) +push_handler (Lisp_Object tag_ch_val, enum handlertype handlertype, + void *sp) { - struct handler *c = push_handler_nosignal (tag_ch_val, handlertype); + struct handler *c = push_handler_nosignal (tag_ch_val, handlertype, sp); if (!c) memory_full (sizeof *c); return c; } struct handler * -push_handler_nosignal (Lisp_Object tag_ch_val, enum handlertype handlertype) +push_handler_nosignal (Lisp_Object tag_ch_val, enum handlertype handlertype, + void *sp) { struct handler *CACHEABLE c = handlerlist->nextfree; if (!c) @@ -1635,6 +1647,7 @@ push_handler_nosignal (Lisp_Object tag_ch_val, enum handlertype handlertype) #ifdef HAVE_X_WINDOWS c->x_error_handler_depth = x_error_message_count; #endif + c->sp = sp; handlerlist = c; return c; } diff --git a/src/lisp.h b/src/lisp.h index 05b0754ff6..f15abb4519 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -3649,6 +3649,7 @@ record_in_backtrace (Lisp_Object function, Lisp_Object *args, ptrdiff_t nargs) #ifdef HAVE_X_WINDOWS int x_error_handler_depth; #endif + void *sp; }; extern Lisp_Object memory_signal_data; @@ -4560,9 +4561,10 @@ xsignal (Lisp_Object error_symbol, Lisp_Object data) (Lisp_Object (*) (ptrdiff_t, Lisp_Object *), ptrdiff_t, Lisp_Object *, Lisp_Object, Lisp_Object (*) (Lisp_Object, ptrdiff_t, Lisp_Object *)); extern Lisp_Object internal_catch_all (Lisp_Object (*) (void *), void *, Lisp_Object (*) (enum nonlocal_exit, Lisp_Object)); -extern struct handler *push_handler (Lisp_Object, enum handlertype) +extern struct handler *push_handler (Lisp_Object, enum handlertype, void *) ATTRIBUTE_RETURNS_NONNULL; -extern struct handler *push_handler_nosignal (Lisp_Object, enum handlertype); +extern struct handler *push_handler_nosignal (Lisp_Object, enum handlertype, + void *); extern void specbind (Lisp_Object, Lisp_Object); extern void record_unwind_protect (void (*) (Lisp_Object), Lisp_Object); extern void record_unwind_protect_array (Lisp_Object *, ptrdiff_t); @@ -5541,9 +5543,36 @@ #define FOR_EACH_ALIST_VALUE(head_var, list_var, value_var) \ /* Check whether it's time for GC, and run it if so. */ +/* Do not wrap into do { } while (0). */ + +struct dont_allow_gc; +struct dont_allow_gc +{ + struct dont_allow_gc *prev; +}; + +extern struct dont_allow_gc *global_dont_allow_gc; + +INLINE void +dont_allow_gc_init (struct dont_allow_gc *dag) +{ + dag->prev = global_dont_allow_gc; + global_dont_allow_gc = dag; +} + +INLINE void +dont_allow_gc_destroy (struct dont_allow_gc *dag) +{ + global_dont_allow_gc = dag->prev; +} + +#define DONT_ALLOW_GC() struct dont_allow_gc __attribute__ ((cleanup (dont_allow_gc_destroy))) dont_allow_gc; dont_allow_gc_init (&dont_allow_gc) + INLINE void maybe_gc (void) { + if (global_dont_allow_gc) + fatal ("GC disallowed"); if (consing_until_gc < 0) maybe_garbage_collect (); } diff --git a/src/thread.c b/src/thread.c index 626d14aad0..e172785a64 100644 --- a/src/thread.c +++ b/src/thread.c @@ -779,7 +779,7 @@ run_thread (void *state) which would otherwise leak every time we unwind back to top-level. */ handlerlist_sentinel = xzalloc (sizeof (struct handler)); handlerlist = handlerlist_sentinel->nextfree = handlerlist_sentinel; - struct handler *c = push_handler (Qunbound, CATCHER); + struct handler *c = push_handler (Qunbound, CATCHER, __builtin_frame_address (0)); eassert (c == handlerlist_sentinel); handlerlist_sentinel->nextfree = NULL; handlerlist_sentinel->next = NULL; -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no