Subject: Re: bug#60237: 30.0.50; tree sitter core dumps when I edebug view a node

From: Eli Zaretskii <[email-masked]>
Date: Sat, 04 Mar 2023 14:21:41 +0200

> From: Stefan Monnier <[email-masked]>
> Cc: [email-masked],  [email-masked],  [email-masked],
>   [email-masked]
> Date: Wed, 01 Mar 2023 12:39:03 -0500
> 
> >> For `emacs-29` I suggest we just use the patch below which should
> >> circumvent the problem.
> >
> > Fine with me, please install, and thanks.
> 
> Thanks, pushed.  Hopefully we can do a bit better on `master`, but
> I don't have time for it right now.  Maybe someone else?

I tried cargo-culting the cpu_gc_count stuff for the memory profiler,
see the patch below.  However, something is amiss: this assertion in
profiler.el sometimes triggers:

    (maphash
     (lambda (backtrace _count)
       (let* ((max (1- (length backtrace)))
              (head (aref backtrace max))
              (best-parent nil)
              (best-match (1+ max))
              (parents (gethash head fun-map)))
         (pcase-dolist (`(,i . ,parent) parents)
           (when t ;; (<= (- max i) best-match) ;Else, it can't be better.
             (let ((match max)
                   (imatch i))
               (cl-assert (>= match imatch))  <<<<<<<<<<<<<<<<<<<<<<<<<<<<
               (cl-assert (function-equal (aref backtrace max)
                                          (aref parent i)))

I cannot reliably reproduce this, and don't understand what causes the
assertion.  Any hints?

Here's the patch:

diff --git a/src/profiler.c b/src/profiler.c
index 8247b2e..92d8a0a 100644
--- a/src/profiler.c
+++ b/src/profiler.c
@@ -227,6 +227,9 @@ record_backtrace (log_t *log, EMACS_INT count)
 /* Separate counter for the time spent in the GC.  */
 static EMACS_INT cpu_gc_count;
 
+/* Separate counter for the memory allocations during GC.  */
+static EMACS_INT mem_gc_count;
+
 /* The current sampling interval in nanoseconds.  */
 static EMACS_INT current_sampling_interval;
 
@@ -451,7 +454,10 @@ DEFUN ("profiler-memory-start", Fprofiler_memory_start, Sprofiler_memory_start,
     error ("Memory profiler is already running");
 
   if (NILP (memory_log))
-    memory_log = make_log ();
+    {
+      mem_gc_count = 0;
+      memory_log = make_log ();
+    }
 
   profiler_memory_running = true;
 
@@ -495,6 +501,10 @@ DEFUN ("profiler-memory-log",
      more for our use afterwards since we can't rely on its special
      pre-allocated keys anymore.  So we have to allocate a new one.  */
   memory_log = profiler_memory_running ? make_log () : Qnil;
+  Fputhash (make_vector (1, QAutomatic_GC),
+	    make_fixnum (mem_gc_count),
+	    result);
+  mem_gc_count = 0;
   return result;
 }
 
@@ -506,10 +516,19 @@ DEFUN ("profiler-memory-log",
 malloc_probe (size_t size)
 {
   if (EQ (backtrace_top_function (), QAutomatic_GC)) /* bug#60237 */
-    /* FIXME: We should do something like what we did with `cpu_gc_count`.  */
-    return;
-  eassert (HASH_TABLE_P (memory_log));
-  record_backtrace (XHASH_TABLE (memory_log), min (size, MOST_POSITIVE_FIXNUM));
+    /* Special case the malloc-count inside GC because the hash-table
+       code is not prepared to be used while the GC is running.
+       More specifically it uses ASIZE at many places where it does
+       not expect the ARRAY_MARK_FLAG to be set.  We could try and
+       harden the hash-table code, but it doesn't seem worth the
+       effort.  */
+    mem_gc_count = saturated_add (mem_gc_count, 1);
+  else
+    {
+      eassert (HASH_TABLE_P (memory_log));
+      record_backtrace (XHASH_TABLE (memory_log),
+			min (size, MOST_POSITIVE_FIXNUM));
+    }
 }
 
 DEFUN ("function-equal", Ffunction_equal, Sfunction_equal, 2, 2, 0,