GNU bug report logs - #46988
28.0.50; Documenting and verifying assumptions about C code not calling quit or GCing

Previous Next

To reply to this bug, email your comments to 46988 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; Documenting and verifying assumptions about C code not
 calling quit or GCing
Date: Sun, 7 Mar 2021 13:47:10 +0000

This is a request for a debugging feature which would verify (at run
time or at compile time or both) that functions we know shouldn't call
GC don't call GC.

See https://lists.gnu.org/archive/html/emacs-devel/2021-03/msg00306.html

Proof-of-concept patch for a runtime check will be attached once this
has a bug number.

Message #8 received at 46988 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> gmail.com>
To: 46988 <at> debbugs.gnu.org
Subject: Re: bug#46988: 28.0.50; Documenting and verifying assumptions about C
 code not calling quit or GCing
Date: Sun, 7 Mar 2021 14:06:01 +0000

[Message part 1 (text/plain, inline)]

On Sun, Mar 7, 2021 at 1:48 PM Pip Cet <pipcet <at> gmail.com> wrote:
> Proof-of-concept patch for a runtime check will be attached once this
> has a bug number.

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.

Pip

[0001-Runtime-check-that-some-functions-don-t-GC-bug-46988.patch (text/x-patch, attachment)]

Message #11 received at 46988 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Pip Cet <pipcet <at> gmail.com>
Cc: 46988 <at> debbugs.gnu.org
Subject: Re: bug#46988: 28.0.50; Documenting and verifying assumptions about
 C code not calling quit or GCing
Date: Mon, 08 Mar 2021 20:42:46 +0100

Pip Cet <pipcet <at> gmail.com> 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.

This is something that comes up again and again, so having
infrastructure to getting feedback faster on this stuff sounds like a
good idea to me.

Even better would be to have build-time warnings, but I guess that's
pretty much impossible?

On the other hand, even if this just gives us run-time feedback, I guess
the test suite would give some coverage here...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #14 received at 46988 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 46988 <at> debbugs.gnu.org
Subject: Re: bug#46988: 28.0.50; Documenting and verifying assumptions about C
 code not calling quit or GCing
Date: Mon, 8 Mar 2021 19:57:07 +0000

On Mon, Mar 8, 2021 at 7:42 PM Lars Ingebrigtsen <larsi <at> gnus.org> wrote:
> Pip Cet <pipcet <at> gmail.com> 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.
>
> This is something that comes up again and again, so having
> infrastructure to getting feedback faster on this stuff sounds like a
> good idea to me.
>
> Even better would be to have build-time warnings, but I guess that's
> pretty much impossible?

It would be very easy if we could categorize our functions into "don't
call this unless you can accept GC" and "I won't GC", but there's a
third category, "depending on my arguments, I may or may not call GC"
(and, of course, a fourth category, "uncategorized").

It's easy to warn about category 1 functions being called from
category 2 functions, but category 3 ruins everything.

I'm very impressed with gcc's -fanalyzer (in conjunction with -flto)
If you have 20 GB of RAM and 15 minutes to spare, you can get it to
find a few places in the Emacs sources that really do look suspicious
and should be fixed to more obviously not dereference NULL.

Essentially, it does a lot of hard work trying to prove your code is
okay; but, if it can't, it will warn even though it might be (and, in
the case of Emacs, is) wrong.

So "all" we'd have to do is to teach it about some category 1
functions, some category 2 functions, and have it prove there's no
valid control flow from the second to the first. If it can't, we just
might have to accept that our arguments are not obvious enough, and
make them explicit enough for the analyzer to understand.

But I haven't figured out how to do that, yet. My suspicion is the
analyzer works on local variables and ignores the state of global
variables, and our state vars would have to be global.

And I realize I'm poking a hornet's nest, but what this really is is
dynamic scope, which might be easier for the analyzer to grok than
general global variables... hmm.

In summary, I think we can install the run-time check, and when the
analyzer is ready, we'll automatically get compile-time warnings! Even
though it might require a terabyte of RAM!

I also like the way we can build specbind-free dynamic C bindings
using stack structures, global variables, __attribute__((cleanup)),
and some code in unwind_to_catch, but of course that's a GCC extension
and not something we can do outside the realm of debugging.

Pip

Message #17 received at 46988 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Pip Cet <pipcet <at> gmail.com>
Cc: 46988 <at> debbugs.gnu.org
Subject: Re: bug#46988: 28.0.50; Documenting and verifying assumptions about
 C code not calling quit or GCing
Date: Tue, 09 Mar 2021 15:05:27 +0100

Pip Cet <pipcet <at> gmail.com> writes:

> In summary, I think we can install the run-time check, and when the
> analyzer is ready, we'll automatically get compile-time warnings! Even
> though it might require a terabyte of RAM!

Makes sense to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #20 received at 46988 <at> debbugs.gnu.org (full text, mbox):

From: Matt Armstrong <matt <at> rfc20.org>
To: Pip Cet <pipcet <at> gmail.com>, 46988 <at> debbugs.gnu.org
Subject: Re: bug#46988: 28.0.50; Documenting and verifying assumptions about
 C code not calling quit or GCing
Date: Wed, 10 Mar 2021 10:28:52 -0800

Pip Cet <pipcet <at> gmail.com> writes:

Hey Pip, just minor comments from me.

(I assume you already plan to put the use of the cleanup attribute
behind conditional macros for portability)

Why a linked list/stack implementation?  How about a global count var
that is incremented, decremented, and asserted zero?

You wrote that this impl depends on the stack direction, but I can't
figure out why.  If it is indeed the case, add a comment explaining
this?

As a macro name, I think something like ASSERT_NO_GC_IN_SCOPE would be
clearer.

Signaling that this is a "magical" scope based construct is useful
because this sort of thing is so unusual in C.  The first thing I looked
for was an "END_SCOPE" macro and started scratching my head.

I'd also use a clear that indicates a debug time check (ASSERT,
CRASH_IF, etc.)

"Don't allow" states an invariant but does not clearly indicate a
consequence or other intent.  It could imply something as polite as "GC
is disabled for this scope".

For the C level stuff, maybe call it gc_forbidden_scope?


> + /* Do not wrap into do { } while (0). */

Move the comment next to the #define.  Ideally, don't issue a command
for the next programmer but instead explain why the code is the way is.

Message #23 received at 46988 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> gmail.com>
To: Matt Armstrong <matt <at> rfc20.org>
Cc: 46988 <at> debbugs.gnu.org
Subject: Re: bug#46988: 28.0.50; Documenting and verifying assumptions about C
 code not calling quit or GCing
Date: Wed, 10 Mar 2021 19:09:29 +0000

On Wed, Mar 10, 2021 at 6:28 PM Matt Armstrong <matt <at> rfc20.org> wrote:
> Pip Cet <pipcet <at> gmail.com> writes:
>
> Hey Pip, just minor comments from me.

Thanks (I'm responding to my email in a LIFO fashion after receiving a
new computer)!

> (I assume you already plan to put the use of the cleanup attribute
> behind conditional macros for portability)

Yes, absolutely.

> Why a linked list/stack implementation?  How about a global count var
> that is incremented, decremented, and asserted zero?

Emacs' specpdl implementation is independent of, and probably
predates, the widespread availability of C exceptions. In fact, as it
is written today, C exceptions will not work in Emacs C code (neither
will C++ exceptions), because Emacs uses setjmp / longjmp instead, and
unwinds the stack by itself. Since I was too lazy to fix it to use
compatible unwinding routines on supported platforms, I simply created
a linked list on the stack, unwinding it in unwind_to_catch.

> You wrote that this impl depends on the stack direction, but I can't
> figure out why.  If it is indeed the case, add a comment explaining
> this?

The unwind_to_catch code checks how far up the stack it has to go by
comparing the stack pointer to the address of a local variable. You're
absolutely right about the comment.

> As a macro name, I think something like ASSERT_NO_GC_IN_SCOPE would be
> clearer.

Thanks! You're quite right, I chose that as a placeholder, and
explicitly mentioning the scope is a really good idea.

> Signaling that this is a "magical" scope based construct is useful
> because this sort of thing is so unusual in C.  The first thing I looked
> for was an "END_SCOPE" macro and started scratching my head.
>
> I'd also use a clear that indicates a debug time check (ASSERT,
> CRASH_IF, etc.)
>
> "Don't allow" states an invariant but does not clearly indicate a
> consequence or other intent.  It could imply something as polite as "GC
> is disabled for this scope".
>
> For the C level stuff, maybe call it gc_forbidden_scope?

I like your proposed names!

> > + /* Do not wrap into do { } while (0). */
>
> Move the comment next to the #define.  Ideally, don't issue a command
> for the next programmer but instead explain why the code is the way is.

Yes, you're right. It's impolite at best. I'll try to avoid that in future.

Thanks again
Pip

Message #26 received at 46988 <at> debbugs.gnu.org (full text, mbox):

From: Matt Armstrong <matt <at> rfc20.org>
To: Pip Cet <pipcet <at> gmail.com>
Cc: 46988 <at> debbugs.gnu.org
Subject: Re: bug#46988: 28.0.50; Documenting and verifying assumptions about
 C code not calling quit or GCing
Date: Thu, 11 Mar 2021 15:17:10 -0800

Pip Cet <pipcet <at> gmail.com> writes:

> On Wed, Mar 10, 2021 at 6:28 PM Matt Armstrong <matt <at> rfc20.org> wrote:
>> Why a linked list/stack implementation?  How about a global count var
>> that is incremented, decremented, and asserted zero?
>
> Emacs' specpdl implementation is independent of, and probably
> predates, the widespread availability of C exceptions. In fact, as it
> is written today, C exceptions will not work in Emacs C code (neither
> will C++ exceptions), because Emacs uses setjmp / longjmp instead, and
> unwinds the stack by itself. Since I was too lazy to fix it to use
> compatible unwinding routines on supported platforms, I simply created
> a linked list on the stack, unwinding it in unwind_to_catch.
>
>> You wrote that this impl depends on the stack direction, but I can't
>> figure out why.  If it is indeed the case, add a comment explaining
>> this?
>
> The unwind_to_catch code checks how far up the stack it has to go by
> comparing the stack pointer to the address of a local variable. You're
> absolutely right about the comment.

Ah, makes perfect sense.  Thanks for this and your other responses.

Message #31 received at 46988 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Pip Cet <pipcet <at> gmail.com>
Cc: 46988 <at> debbugs.gnu.org,
 Mattias Engdegård <mattiase <at> acm.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: 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

Pip Cet <pipcet <at> gmail.com> 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

Message #36 received at 46988 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 46988 <at> debbugs.gnu.org, mattiase <at> acm.org, monnier <at> iro.umontreal.ca,
 pipcet <at> gmail.com
Subject: Re: bug#46988: 28.0.50;
 Documenting and verifying assumptions about C code not calling quit
 or GCing
Date: Mon, 20 Jun 2022 14:47:48 +0300

> Cc: 46988 <at> debbugs.gnu.org,
>  Mattias Engdegård <mattiase <at> acm.org>,
>  Stefan Monnier <monnier <at> iro.umontreal.ca>
> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Date: Mon, 20 Jun 2022 03:41:42 +0200
> 
> Pip Cet <pipcet <at> gmail.com> 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.

Yes, I agree.

In addition, AFAIU this uses some GCC-specific stuff without providing
for other compilers that don't support the same literal features.

Message #39 received at 46988 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 46988 <at> debbugs.gnu.org, Mattias <mattiase <at> acm.org>,
 Lars Ingebrigtsen <larsi <at> gnus.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#46988: 28.0.50; Documenting and verifying assumptions about C
 code not calling quit or GCing
Date: Thu, 23 Jun 2022 15:56:49 +0000

On Mon, Jun 20, 2022 at 11:48 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> > I think if we add this, it should be enabled only if the build is
> > configured with --enable-checking.
>
> Yes, I agree.

So do I.

> In addition, AFAIU this uses some GCC-specific stuff without providing
> for other compilers that don't support the same literal features.

I'm not sure to what extent clang emulates those same features, but
it's certainly not standard C.

Pip

Message #42 received at 46988 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> gmail.com>
Cc: 46988 <at> debbugs.gnu.org, mattiase <at> acm.org, larsi <at> gnus.org,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#46988: 28.0.50; Documenting and verifying assumptions about C
 code not calling quit or GCing
Date: Thu, 23 Jun 2022 19:08:55 +0300

> From: Pip Cet <pipcet <at> gmail.com>
> Date: Thu, 23 Jun 2022 15:56:49 +0000
> Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 46988 <at> debbugs.gnu.org, Mattias <mattiase <at> acm.org>, 
> 	Stefan Monnier <monnier <at> iro.umontreal.ca>
> 
> > In addition, AFAIU this uses some GCC-specific stuff without providing
> > for other compilers that don't support the same literal features.
> 
> I'm not sure to what extent clang emulates those same features, but
> it's certainly not standard C.

Clang at least attempts to be GCC-compliant.  But there are other C
compilers we want to support which don't do even that.

Message #45 received at 46988 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattiase <at> acm.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 46988 <at> debbugs.gnu.org, larsi <at> gnus.org, monnier <at> iro.umontreal.ca,
 Pip Cet <pipcet <at> gmail.com>
Subject: Re: bug#46988: 28.0.50; Documenting and verifying assumptions about C
 code not calling quit or GCing
Date: Thu, 23 Jun 2022 18:20:09 +0200

23 juni 2022 kl. 18.08 skrev Eli Zaretskii <eliz <at> gnu.org>:

> Clang at least attempts to be GCC-compliant.  But there are other C
> compilers we want to support which don't do even that.

Quite true. I don't think we should bother supporting them (at least not right away); just disable the feature for them.

The feature looks sound in principle and probably useful. Matt Armstrong had some good comments on the surface (https://debbugs.gnu.org/cgi/bugreport.cgi?bug=46988#20) to make it more readable.

Maybe it would be useful to generalise it to other dynamic checks, such as 'no consing', 'no lisp', 'no bytecode', 'no regexp', 'no signalling'?

Message #48 received at 46988 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattiase <at> acm.org>
Cc: 46988 <at> debbugs.gnu.org, larsi <at> gnus.org, monnier <at> iro.umontreal.ca,
 pipcet <at> gmail.com
Subject: Re: bug#46988: 28.0.50; Documenting and verifying assumptions about C
 code not calling quit or GCing
Date: Thu, 23 Jun 2022 19:35:19 +0300

> From: Mattias Engdegård <mattiase <at> acm.org>
> Date: Thu, 23 Jun 2022 18:20:09 +0200
> Cc: Pip Cet <pipcet <at> gmail.com>, larsi <at> gnus.org, 46988 <at> debbugs.gnu.org,
>         monnier <at> iro.umontreal.ca
> 
> 23 juni 2022 kl. 18.08 skrev Eli Zaretskii <eliz <at> gnu.org>:
> 
> > Clang at least attempts to be GCC-compliant.  But there are other C
> > compilers we want to support which don't do even that.
> 
> Quite true. I don't think we should bother supporting them (at least not right away); just disable the feature for them.

Yes, that's what I meant: we shouldn't break compilation with those,
but we don't have to do more.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.