Subject: bug#40897: Negation of pixel-width :width expressions does not work

From: Pip Cet <[email-masked]>
Date: Mon, 27 Apr 2020 08:47:50 +0000

In looking over the xdisp.c code, I noticed that
calc_pixel_width_or_height has at least three problems:

1. negation of width specifiers doesn't work
2. a circular list passed to (+) will hang Emacs
3. a circular list passed directly will crash Emacs

(1) is a significant bug and should be fixed. (2) is easy to fix, and
we should probably do so on the master branch. (3) is difficult to
fix, but I'd like to fix all these, and undiscovered similar issues,
by converting calc_pixel_width_or_height to using the Lisp
interpreter.

The underlying problem is we're writing yet another mini-interpreter
for a limited Lisp-like language, in C. That means no quitting, no
protection against stack overflows, and getting tricky semantics (such
as the different behavior of (- A) and (- A B ...)) wrong.

My proposal is to move the somewhat complex logic of
calc_pixel_width_or_height to Lisp code, run when the display spec is
actually evaluated (i.e. when "(when ...)" display specs run their
Lisp code). This could be achieved by a method similar to the patch
for bug#40845 in
https://lists.gnu.org/archive/html/bug-gnu-emacs/2020-04/msg01096.html.

This would also resolve bug#40856 by allowing more complicated space
calculations at a time when the font metrics are known.

I'm attaching a patch, against master, for (1) only. I haven't tried
to restore the previous "optimization" of avoiding (very cheap) calls
to EQ for all but the first argument.

Recipes:

(1):

(insert (propertize " " 'display '(space :width (+ 50 (- 50)))))

Expected result: a zero-width space
Actual result: a space of width 100

(2):
(let ((l (cons 0 nil)))
  (setcdr l l)
  (insert (propertize " " 'display `(space :width (+ . ,l)))))

Expected result: a quittable infinite loop
Actual result: a non-quittable infinite loop

(3):
(let ((l (cons 0 nil)))
  (setcdr l l)
  (insert (propertize " " 'display `(space :width ,l))))

Expected result: a Lisp stack overflow error
Actual result: a C stack overflow, Emacs crashes
From b7493ff934553a2d008cce4fb8844d25fd2df998 Mon Sep 17 00:00:00 2001
From: Pip Cet <[email-masked]>
Date: Mon, 27 Apr 2020 08:41:52 +0000
Subject: [PATCH] Fix pixel-width evaluation of negated specs

* src/xdisp.c (calc_pixel_width_or_height): Fix evaluation of (- X)
SPEC.
---
 src/xdisp.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/xdisp.c b/src/xdisp.c
index 140d134572..1aba4c8901 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -27278,7 +27278,7 @@ calc_pixel_width_or_height (double *res, struct it *it, Lisp_Object prop,
 	     recursively calculated values.  */
 	  if (EQ (car, Qplus) || EQ (car, Qminus))
 	    {
-	      bool first = true;
+	      ptrdiff_t count = 0;
 	      double px;
 
 	      pixels = 0;
@@ -27287,13 +27287,13 @@ calc_pixel_width_or_height (double *res, struct it *it, Lisp_Object prop,
 		  if (!calc_pixel_width_or_height (&px, it, XCAR (cdr),
 						   font, width_p, align_to))
 		    return false;
-		  if (first)
-		    pixels = (EQ (car, Qplus) ? px : -px), first = false;
-		  else
+		  if (count++ == 0)
 		    pixels += px;
+		  else
+		    pixels += (EQ (car, Qplus) ? px : -px);
 		  cdr = XCDR (cdr);
 		}
-	      if (EQ (car, Qminus))
+	      if (EQ (car, Qminus) && count == 1)
 		pixels = -pixels;
 	      return OK_PIXELS (pixels);
 	    }
-- 
2.26.2