bug#32038: 27.0.50; Emacs hangs when using :propertize mode line construct and not providing a property value

[Jonathan Kyle Mitchell <kyle@jonathanmitchell.org>]

[-- Attachment #1: Type: text/plain, Size: 1789 bytes --]

On Wed, 2018-07-04 at 18:07 +0300, Eli Zaretskii wrote:
> > From: Jonathan Kyle Mitchell <kyle@jonathanmitchell.org>
> > Date: Tue, 03 Jul 2018 22:12:57 -0500
> > Cc: 32038@debbugs.gnu.org
> > 
> > I think I found a way to make redisplay ignore any malformed
> > property list by
> > putting a single check around Fset_text_properties in xdisp.c. The
> > text of the
> > modeline is still set according to the provided string, but the
> > property list
> > is ignored if it doesn't have an even number of elements. It
> > doesn't
> > infinitely loop anymore given a malformed property list.
> 
> Thanks, but I think we should log the error in *Messages*, because
> otherwise the error will go unnoticed.
> 
> > +		    if (EQ (Fmod (Flength (props), make_number
> > (2)),
> > +			    make_number (0)))
> 
> We are on the C level, so it is easier/simpler to do this instead:
> 
>   ptrdiff_t seqlen = XFASTINT (Flength (props));
>   if (seqlen % 2 == 0)
>     Fset_text_properties (...);
> 
> More importantly, Flength can signal an error if PROPS is too long,
> so
> I'm not sure the idea of your patch is 100% correct, because the code
> you propose can still signal an error.  An alternative would be to
> call Fset_text_properties via internal_condition_case_n, like we do
> in
> safe__call.

That makes sense. I added one function to call Fset_text_properties
through internal_condition_case_n in the attached revised patch. The
error gets caught and safe_eval_handler appends an error message to the
*Messages* buffer.

The error message is put in *Messages* on the first time only though,
subsequent (force-mode-line-update) calls just append nil. I don't know
if that's expected for redisplay's internal messaging routines.

Thanks for reviewing the patch.

--
Jonathan Kyle Mitchell

[-- Attachment #2: 0001-Call-Fset_text_properties-through-internal_condition.patch --]
[-- Type: text/x-patch, Size: 1706 bytes --]

From 621bc3df19eaf2258c9a2ec0c72004ea8336ce0f Mon Sep 17 00:00:00 2001
From: Jonathan Kyle Mitchell <kyle@jonathanmitchell.org>
Date: Wed, 4 Jul 2018 22:38:29 -0500
Subject: [PATCH] Call Fset_text_properties through internal_condition_case_n

* xdisp.c (display_mode_element, safe_set_text_properties):
---
 src/xdisp.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/xdisp.c b/src/xdisp.c
index 9b4febdd61..fb8fc905b9 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -23516,6 +23516,17 @@ move_elt_to_front (Lisp_Object elt, Lisp_Object list)
   return list;
 }
 
+/* Subroutine to call Fset_text_properties through
+   internal_condition_case_n.  ARGS are the arguments of
+   Fset_text_properties, in order. */
+
+static Lisp_Object
+safe_set_text_properties (ptrdiff_t nargs, Lisp_Object *args)
+{
+  eassert (nargs == 4);
+  return Fset_text_properties (args[0], args[1], args[2], args[3]);
+}
+
 /* Contribute ELT to the mode line for window IT->w.  How it
    translates into text depends on its data type.
 
@@ -23610,8 +23621,14 @@ display_mode_element (struct it *it, int depth, int field_width, int precision,
 			= Fdelq (aelt, mode_line_proptrans_alist);
 
 		    elt = Fcopy_sequence (elt);
-		    Fset_text_properties (make_number (0), Flength (elt),
-					  props, elt);
+		    internal_condition_case_n (safe_set_text_properties,
+					       4,
+					       ((Lisp_Object [])
+					       {make_number (0),
+						   Flength (elt),
+						   props,
+						   elt}),
+					       Qt, safe_eval_handler);
 		    /* Add this item to mode_line_proptrans_alist.  */
 		    mode_line_proptrans_alist
 		      = Fcons (Fcons (elt, props),
-- 
2.17.1