Subject: bug#76180: [feature/igc] Remaining known tracing issues

From: Pip Cet <[email-masked]>
Date: Tue, 11 Feb 2025 20:16:21 +0000

"Stefan Kangas" <[email-masked]> writes:

> Pip Cet via "Bug reports for GNU Emacs, the Swiss army knife of text
> editors" <[email-masked]> writes:
>
>> My hope is that once these stop-gap patches are installed, it's a good
>> point to call again for general testing of the feature/igc branch, under
>> certain restrictions and with specific instructions to reduce the
>> possibility of data loss due to crashes (e.g. run in GDB, set a
>> breakpoint on kill, don't use TTY signals, xwidgets, unusual toolkits).
>> I hope that "PGTK" and "WIDE_EMACS_INT" can be removed from the list of
>> build constellations expected to be unstable ASAP.
>>
>> The branch will continue to contain bugs and we can't take any
>> responsibility, but at least avoiding those known bugs seems to result
>> in a usable work environment here, and it might be a good opportunity to
>> regain some of the early testers who were too frustrated by stability
>> issues to let them know that we've fixed some of them.
>>
>> Thoughts?
>
> I don't have anything specific to add, but the above sounds like a good
> plan to me.  Meanwhile, we also continue seeing other useful bug reports
> trickling in.

Well, here are the patches.  I tried for a while to actually produce the
bugs they attempt to fix, but those efforts weren't successful.  As
these bugs would be a bit subtle, that's not overly surprising.

I've decided against putting HORRIBLE_ESTIMATE in lisp.h because then
we'd probably have to explain what it is :-)

>From 3cc41ab8ba5dda51774157dea5a976b931bf6a90 Mon Sep 17 00:00:00 2001
From: Pip Cet <[email-masked]>
Subject: [PATCH 1/3] [MPS] Force nativecomp code to be compiled with frame
 pointers

If nativecomp code keeps a reference to an MPS object in %rbp and
calls setjmp, the reference will be scrambled and cause crashes if MPS
moves or frees it.

* lisp/emacs-lisp/comp.el (native-comp-compiler-options) [HAVE_MPS]:
Add "-fno-omit-frame-pointer".
---
 lisp/emacs-lisp/comp.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/comp.el b/lisp/emacs-lisp/comp.el
index 20b9c140228..86944b1a645 100644
--- a/lisp/emacs-lisp/comp.el
+++ b/lisp/emacs-lisp/comp.el
@@ -91,7 +91,8 @@ native-comp-bootstrap-deny-list
   :type '(repeat regexp)
   :version "28.1")
 
-(defcustom native-comp-compiler-options nil
+(defcustom native-comp-compiler-options
+  (cond ((featurep 'mps) (list "-fno-omit-frame-pointer")))
   "Command line options passed verbatim to GCC compiler.
 Note that not all options are meaningful and some options might even
 break your Emacs.  Use at your own risk.
-- 
2.48.1

>From 2c8c6f8013aaf6a80ba0e97b99e8bc156c31b3ff Mon Sep 17 00:00:00 2001
From: Pip Cet <[email-masked]>
Subject: [PATCH 2/3] [MPS] Refuse to read byte-code objects with deep stacks

HORRIBLE_ESTIMATE in igc.c provides an upper limit on how deep a byte
code object's stack can be and still be handled appropriately by igc.
Enforce that.

* src/alloc.c (Fmake_byte_code):
* src/lread.c (bytecode_from_rev_list): Error out rather than creating
a crashable byte-code object.-
---
 src/alloc.c | 5 +++++
 src/lread.c | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/src/alloc.c b/src/alloc.c
index 7bad029b858..f8bcb1ef717 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -3820,6 +3820,11 @@ and (optional) INTERACTIVE-SPEC.
 	 && FIXNATP (args[CLOSURE_STACK_DEPTH])))
     error ("Invalid byte-code object");
 
+#ifdef HAVE_MPS
+  if (XFIXNAT (args[CLOSURE_STACK_DEPTH]) >= 1022)
+    error ("Byte-code object uses too much stack, increase HORRIBLE_ESTIMATE!");
+#endif
+
 #ifndef HAVE_MPS
   /* Bytecode must be immovable.  */
   pin_string (args[CLOSURE_CODE]);
diff --git a/src/lread.c b/src/lread.c
index e8cd689d794..67c1bbe5f7b 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -3483,6 +3483,11 @@ bytecode_from_rev_list (Lisp_Object elems, Lisp_Object readcharfun)
 	            || NILP (vec[CLOSURE_CONSTANTS]))))))
     invalid_syntax ("Invalid byte-code object", readcharfun);
 
+#ifdef HAVE_MPS
+  if (XFIXNAT (vec[CLOSURE_STACK_DEPTH]) >= 1022)
+    error ("Byte-code object uses too much stack, increase HORRIBLE_ESTIMATE!");
+#endif
+
   if (STRINGP (vec[CLOSURE_CODE]))
     {
       if (STRING_MULTIBYTE (vec[CLOSURE_CODE]))
-- 
2.48.1

>From ad35aecca5c6270faeaff03cae4ff682d7503b35 Mon Sep 17 00:00:00 2001
From: Pip Cet <[email-masked]>
Subject: [PATCH 3/3] [MPS] Ensure sys_jmp buffers are scanned ambiguously

* src/igc.c (igc_alloc_handler): Use 'igc_xzalloc_ambig', not 'alloc'.
---
 src/igc.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/igc.c b/src/igc.c
index 9ed5c983a5d..17816c3a74c 100644
--- a/src/igc.c
+++ b/src/igc.c
@@ -4491,7 +4491,14 @@ igc_alloc_blv (void)
 void *
 igc_alloc_handler (void)
 {
-  struct handler *h = alloc (sizeof *h, IGC_OBJ_HANDLER);
+  /* struct handler contains a jmp_buf, which means it must be scanned
+     ambiguously in case one of the registers stored in the jmp_buf
+     refers to an MPS object.
+
+     On Windows 64, jmp_buf is also 16-byte aligned, which causes
+     trouble with the MPS default alignment.
+   */
+  struct handler *h = igc_xzalloc_ambig (sizeof *h);
   return h;
 }
 
-- 
2.48.1