bug#36312: 27.0.50; (message) in display spec condition causes emacs_abort()

bug#36312: 27.0.50; (message) in display spec condition causes emacs_abort()

[Pip Cet]

In emacs -Q, evaluate

(let ((o (make-overlay (point) (point))))
  (overlay-put o 'after-string (propertize " " 'display
                       '(when (message "a")
  . "b"))))

This causes a SIGABRT in the bidi stack code.

Backtrace:

#0  0x00007ffff4b4d5cb in raise (sig=sig@entry=6)
    at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x000055555559573b in terminate_due_to_signal
    (sig=sig@entry=6, backtrace_limit=backtrace_limit@entry=40) at emacs.c:406
#2  0x0000555555595b37 in emacs_abort () at sysdep.c:2453
#3  0x00005555555942e3 in bidi_pop_it (bidi_it=bidi_it@entry=0x7fffffff99b0)
    at bidi.c:947
#4  0x00005555555b2174 in pop_it (it=0x7fffffff8fc0) at xdisp.c:6356
#5  0x00005555555c3594 in next_overlay_string (it=it@entry=0x7fffffff8fc0)
    at xdisp.c:5754
#6  0x00005555555c8edc in set_iterator_to_next
    (it=0x7fffffff8fc0, reseat_p=<optimized out>) at xdisp.c:7871
#7  0x00005555555cf9ba in display_line
    (it=0x7fffffff8fc0, cursor_vpos=<optimized out>) at xdisp.c:22296
#8  0x00005555555d491b in try_window
    (window=window@entry=0x555555f99fa5, pos=..., flags=flags@entry=1)
    at xdisp.c:17863
#9  0x00005555555e80df in redisplay_window
    (window=0x555555f99fa5, just_this_one_p=<optimized out>) at xdisp.c:17311
#10 0x00005555555eae1e in redisplay_window_1
(window=window@entry=0x555555f99fa5)
    at xdisp.c:15044
#11 0x00005555556eb3ca in internal_condition_case_1
    (bfun=bfun@entry=0x5555555eadf0 <redisplay_window_1>,
arg=0x555555f99fa5, handlers=<optimized out>,
hfun=hfun@entry=0x5555555afd00 <redisplay_window_error>)
    at eval.c:1376
#12 0x00005555555d834f in redisplay_internal () at xdisp.c:14614
#13 0x000055555567b5d7 in read_char
    (commandflag=1, map=0x555556856f83, prev_event=0x0,
used_mouse_menu=0x7fffffffe4db, end_time=0x0) at keyboard.c:2474
#14 0x000055555567df1e in read_key_sequence
    (keybuf=0x7fffffffe5e0, prompt=0x0, dont_downcase_last=<optimized
out>, can_return_switch_frame=true, fix_current_buffer=true,
prevent_redisplay=<optimized out>)
    at keyboard.c:9111
#15 0x000055555567f6fc in command_loop_1 () at lisp.h:1045
#16 0x00005555556eb332 in internal_condition_case
    (bfun=bfun@entry=0x55555567f520 <command_loop_1>,
handlers=handlers@entry=0x55b0, hfun=hfun@entry=0x555555676d20
<cmd_error>) at eval.c:1352
#17 0x0000555555671d14 in command_loop_2 (ignore=ignore@entry=0x0) at
lisp.h:1045
#18 0x00005555556eb2b1 in internal_catch
    (tag=tag@entry=0xcc30, func=func@entry=0x555555671cf0
<command_loop_2>, arg=arg@entry=0x0) at eval.c:1113
#19 0x0000555555671cbb in command_loop () at lisp.h:1045
#20 0x0000555555676926 in recursive_edit_1 () at keyboard.c:714
#21 0x0000555555676c45 in Frecursive_edit () at keyboard.c:786
#22 0x000055555559b227 in main (argc=2, argv=0x7fffffffe978) at emacs.c:1962

It's reproducible here, so I can provide more debugging information if
required.

I'm aware that doing complicated stuff in a display spec condition is
a bad idea, but I think debug messages are such an important special
case that we ought, at least, not to abort completely for those.

Setting redisplay--bidi-inhibit to t works around the problem.

bug#36312: 27.0.50; (message) in display spec condition causes emacs_abort()

[Eli Zaretskii]

> From: Pip Cet <pipcet@gmail.com>
> Date: Thu, 20 Jun 2019 16:06:25 +0000
> 
> In emacs -Q, evaluate
> 
> (let ((o (make-overlay (point) (point))))
>   (overlay-put o 'after-string (propertize " " 'display
>                        '(when (message "a")
>   . "b"))))
> 
> This causes a SIGABRT in the bidi stack code.

Thanks.

You do realize that calling 'message' in a display-spec form causes us
to re-enter redisplay in the middle of redisplay?  I didn't even know
we allowed that, but there's something new about our display code to
learn every day.

Should be fixed now.

P.S. Is there a chance fontification-functions will also call
'message'?  Because I think we have the same problem there.

bug#36312: 27.0.50; (message) in display spec condition causes emacs_abort()

[Pip Cet]

On Thu, Jun 20, 2019 at 6:12 PM Eli Zaretskii <eliz@gnu.org> wrote:
> > From: Pip Cet <pipcet@gmail.com>
> > Date: Thu, 20 Jun 2019 16:06:25 +0000
> >
> > In emacs -Q, evaluate
> >
> > (let ((o (make-overlay (point) (point))))
> >   (overlay-put o 'after-string (propertize " " 'display
> >                        '(when (message "a")
> >   . "b"))))
> >
> > This causes a SIGABRT in the bidi stack code.
>
> Thanks.
>
> You do realize that calling 'message' in a display-spec form causes us
> to re-enter redisplay in the middle of redisplay?  I didn't even know
> we allowed that, but there's something new about our display code to
> learn every day.

I did learn that today, and it's very puzzling. For starters, why do
we allow '(when ...), but not '(eval ...)? And is it really a good
idea to call code from redisplay?

The reason I'm looking at this is that I wanted to define an image
that changes color based on the face properties at point. It turns out
you can do that by abusing a (when ...) spec (which I use to call real
code in a (run-with-timer 0 nil ...), to avoid the issue of recursive
redisplays etc.).

It's very ugly, and I'm not sure what the best way to handle things
would be; luckily, I'm not dependent on running on older or official
Emacs versions, so I'm free to experiment.

So far, what I'm thinking about is a hook that's run _after_ redisplay
to let an overlay know that redisplay just happened and the face used
for the text around the overlay changed. It's soon enough to change
things and trigger another redisplay then, as far as I'm concerned.
(There's some flickering, but for my application that's okay).

But I'd appreciate any suggestions (the immediate application is to
define character-like image-based glyphs that "look like text", but
there might be others).

bug#36312: 27.0.50; (message) in display spec condition causes emacs_abort()

[Eli Zaretskii]

> From: Pip Cet <pipcet@gmail.com>
> Date: Thu, 20 Jun 2019 19:32:43 +0000
> Cc: 36312@debbugs.gnu.org
> 
> > You do realize that calling 'message' in a display-spec form causes us
> > to re-enter redisplay in the middle of redisplay?  I didn't even know
> > we allowed that, but there's something new about our display code to
> > learn every day.
> 
> I did learn that today, and it's very puzzling. For starters, why do
> we allow '(when ...), but not '(eval ...)?

Because the idea was that the result of the evaluation should be used
to affect the 'display' property in whose spec it is included, not to
run some arbitrary unrelated code.  With the intended use, 'when' is
more useful than 'eval', because 'when' can do everything 'eval' can
do, but not the other way around.

> And is it really a good idea to call code from redisplay?

That ship has sailed long ago, with Emacs 21.1.  We have
fontification-functions which are called from within redisplay, we
have (height SOMETHING) in display specs, where SOMETHING can be a
function or an arbitrary Lisp form which returns the height to be used
to display text, we have 'eval' in the mode line format, etc. etc.
Just search xdisp.c for safe_call and safe_eval, and you will see how
many of them are there.

It is true that running expensive Lisp from these hooks is a bad idea,
because it will slow down redisplay.  But Emacs gives you enough rope
to hang yourself, and then trusts you not to do that.

> The reason I'm looking at this is that I wanted to define an image
> that changes color based on the face properties at point. It turns out
> you can do that by abusing a (when ...) spec (which I use to call real
> code in a (run-with-timer 0 nil ...), to avoid the issue of recursive
> redisplays etc.).
> 
> It's very ugly, and I'm not sure what the best way to handle things
> would be; luckily, I'm not dependent on running on older or official
> Emacs versions, so I'm free to experiment.

Why not do something like that in a post-command-hook instead?  The
position of point is already up to date then, and retrieving face
properties at point is trivial.  What am I missing?

There's also pre-redisplay-hook.

> So far, what I'm thinking about is a hook that's run _after_ redisplay
> to let an overlay know that redisplay just happened and the face used
> for the text around the overlay changed.

I don't understand why you think you must run after redisplay.  If
redisplay changes the faces (probably via JIT font-lock?), and you
must have the corresponding change in your image without any delays
(though if you use a timer, you seem to not mind a delay), then why
not register a function with jit-lock-register, and do that from
there?  And if you indeed don't mind a short delay, then I think
post-command-hook should be your friend.

> But I'd appreciate any suggestions (the immediate application is to
> define character-like image-based glyphs that "look like text", but
> there might be others).

You seem to be thinking about font-lock like use cases, so plugging
yourself into jit-lock would be my first suggestion.

In any case, (ab)using 'when' in display specs for running code that
doesn't affect the value of that same display spec is to be avoided,
IMO, as that is not what these features is for.

Can we close this bug?  Is the crash fixed in your real-life code as
well?

Thanks.

bug#36312: 27.0.50; (message) in display spec condition causes emacs_abort()

[Pip Cet]

On Fri, Jun 21, 2019 at 7:34 AM Eli Zaretskii <eliz@gnu.org> wrote:
> > From: Pip Cet <pipcet@gmail.com>
> > Date: Thu, 20 Jun 2019 19:32:43 +0000
> > Cc: 36312@debbugs.gnu.org
> >
> Can we close this bug?  Is the crash fixed in your real-life code as
> well?

Thank you very much, it is!

bug#36312: 27.0.50; (message) in display spec condition causes emacs_abort()

[Eli Zaretskii]

> Date: Fri, 21 Jun 2019 10:34:26 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 36312@debbugs.gnu.org
> 
> Why not do something like that in a post-command-hook instead?  The
> position of point is already up to date then, and retrieving face
> properties at point is trivial.  What am I missing?
> 
> There's also pre-redisplay-hook.
> 
> > So far, what I'm thinking about is a hook that's run _after_ redisplay
> > to let an overlay know that redisplay just happened and the face used
> > for the text around the overlay changed.
> 
> I don't understand why you think you must run after redisplay.  If
> redisplay changes the faces (probably via JIT font-lock?), and you
> must have the corresponding change in your image without any delays
> (though if you use a timer, you seem to not mind a delay), then why
> not register a function with jit-lock-register, and do that from
> there?  And if you indeed don't mind a short delay, then I think
> post-command-hook should be your friend.
> 
> > But I'd appreciate any suggestions (the immediate application is to
> > define character-like image-based glyphs that "look like text", but
> > there might be others).
> 
> You seem to be thinking about font-lock like use cases, so plugging
> yourself into jit-lock would be my first suggestion.
> 
> In any case, (ab)using 'when' in display specs for running code that
> doesn't affect the value of that same display spec is to be avoided,
> IMO, as that is not what these features is for.

FTR, I wanted to draw your attention to some problematic aspects of
using 'when' in display specs as a kind of "redisplay hook".

First aspect that you need to be aware of is that when the display
spec is being evaluated, there's no 100% guarantee that the value of
point is what will eventually be displayed.  In some, admittedly rare,
situations, when redisplay is finished, it turns out that point is not
in a fully visible screen line.  In such cases, the display engine can
decide to move point to bring it back into the viewport.  It then
performs another redisplay cycle, but that is not guaranteed to
re-evaluate your display spec, because Emacs might find a redisplay
optimization which avoids that (typically, moving point only needs to
consider point's line).

More generally, if a window is redrawn, the display engine might
decide not to redisplay the line where you have your display spec, but
instead either scroll the display (i.e. reuse the results of the
previous redisplay cycle), or even bypass that line altogether,
because it decides that this line is outside of the region affected by
the last command.

Another problematic aspect is that the display specs are evaluated for
purposes other than displaying the results.  There are features in
Emacs that need to perform text layout calculations without displaying
the text whose layout they consider.  One popular example is C-n/C-p,
which calls vertical-motion internally.  vertical-motion then invokes
the display engine in a special mode which performs text layout
calculations and measures the text dimensions without displaying that
text, just because it needs to figure out, for example, what character
or buffer position is on display directly below or above the given
screen coordinates.  The result of this is that your 'when' form will
be called in contexts other than redisplay, and if you fire timers or
other functions inside that form, you will find that your code is
invoked more than once per command, in different contexts you didn't
necessarily expect, and with point whose value may not be up to date.
To continue the same example, vertical-motion invokes the display
routine before it moves point, precisely _because_ it needs to perform
layout calculations to decide where to move point.