Face alias bugs (Re: abort() in keyboard.c)

Face alias bugs (Re: abort() in keyboard.c)

[Károly Lőrentey]

[-- Attachment #1.1.1: Type: text/plain, Size: 922 bytes --]

I have received a backtrace for an aborted multi-tty session from Ami
Fischman that is relevant to the trunk as well.

The abort was caused by Emacs entering the debugger during redisplay
because of a property list on a face symbol that was not a well-formed
Lisp list (see frames #23-26 in the backtrace at the bottom).

Beside the (rare) crash condition, there are other symptoms that are
easier to reproduce.  For example (in a recent CVS):

	emacs -q
	(setplist 'default (append (symbol-plist 'default) (cons 'foo 'bar)))
	C-h i
        <Redisplay loops>

Recursive aliases are fun as well:

        emacs -q
	(put 'foo 'face-alias 'default) 
        (put 'default 'face-alias 'foo)
        C-h i
        <unquitable loop>

The patch below fixes all these problems by protecting the call to
Fget inside resolve_face_name and limiting the number of indirections
during a single face lookup.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: face-crash.patch --]
[-- Type: text/x-patch, Size: 1673 bytes --]

*** orig/src/xfaces.c
--- mod/src/xfaces.c
***************
*** 3201,3206 ****
--- 3201,3220 ----
  
  \f
  
+ static Lisp_Object
+ internal_resolve_face_name (nargs, args)
+      int nargs;
+      Lisp_Object *args;
+ {
+   return Fget (args[0], args[1]);
+ }
+ 
+ static Lisp_Object
+ resolve_face_name_error (ignore)
+      Lisp_Object ignore;
+ {
+   return Qnil;
+ }
  
  /* Resolve face name FACE_NAME.  If FACE_NAME is a string, intern it
     to make it a symvol.  If FACE_NAME is an alias for another face,
***************
*** 3211,3227 ****
       Lisp_Object face_name;
  {
    Lisp_Object aliased;
  
    if (STRINGP (face_name))
      face_name = intern (SDATA (face_name));
  
!   while (SYMBOLP (face_name))
      {
!       aliased = Fget (face_name, Qface_alias);
        if (NILP (aliased))
  	break;
        else
  	face_name = aliased;
      }
  
    return face_name;
--- 3225,3249 ----
       Lisp_Object face_name;
  {
    Lisp_Object aliased;
+   Lisp_Object args[2];
+   int c = 0;
  
    if (STRINGP (face_name))
      face_name = intern (SDATA (face_name));
  
!   /* Protect against loops by limiting the number of indirections.  */
!   while (SYMBOLP (face_name) && c < 10)
      {
!       /* Fget can signal an error; just ignore it.  */
!       args[0] = face_name;
!       args[1] = Qface_alias;
!       aliased = internal_condition_case_2 (internal_resolve_face_name, 2, args, Qt,
!                                            resolve_face_name_error);
        if (NILP (aliased))
  	break;
        else
  	face_name = aliased;
+       c++;
      }
  
    return face_name;

[-- Attachment #1.1.3: Type: text/plain, Size: 6727 bytes --]


Ami Fischman <ami@fischman.org> writes:
> I just had an abort hit in an emacs session running under gdb under screen,
> with X frames open at work and home.  The backtrace is after my .sig.  Not
> really sure what if any useful info can be gleaned from it.  Does this ring
> any bells for anyone?
>
> #0  0x402feae1 in __kill () from /lib/i686/libc.so.6
> #1  0x080d8cfe in abort () at /workplace2/cvs/emacs/src/emacs.c:458
> #2  0x080dd8fd in read_char (commandflag=1, nmaps=5, maps=0xbfffbe30, prev_event=137388369, used_mouse_menu=0xbfffbe84)
>     at /workplace2/cvs/emacs/src/keyboard.c:2633
> #3  0x080e47ad in read_key_sequence (keybuf=0xbfffbf90, bufsize=30, prompt=137388369, dont_downcase_last=0, can_return_switch_frame=1, 
>     fix_current_buffer=1) at /workplace2/cvs/emacs/src/keyboard.c:8872
> #4  0x080dbc80 in command_loop_1 () at /workplace2/cvs/emacs/src/keyboard.c:1523
> #5  0x081344cd in internal_condition_case (bfun=0x80db964 <command_loop_1>, handlers=137449369, hfun=0x80db518 <cmd_error>)
>     at /workplace2/cvs/emacs/src/eval.c:1385
> #6  0x080db7d8 in command_loop_2 () at /workplace2/cvs/emacs/src/keyboard.c:1304
> #7  0x08134005 in internal_catch (tag=137484681, func=0x80db7b4 <command_loop_2>, arg=137388369) at /workplace2/cvs/emacs/src/eval.c:1144
> #8  0x080db73d in command_loop () at /workplace2/cvs/emacs/src/keyboard.c:1271
> #9  0x080db2a4 in recursive_edit_1 () at /workplace2/cvs/emacs/src/keyboard.c:980
> #10 0x080db3e0 in Frecursive_edit () at /workplace2/cvs/emacs/src/keyboard.c:1041
> #11 0x08135f05 in Ffuncall (nargs=1, args=0xbfffc2d0) at /workplace2/cvs/emacs/src/eval.c:2783
> #12 0x0815b61c in Fbyte_code (bytestr=180765107, vector=191568892, maxdepth=32) at /workplace2/cvs/emacs/src/bytecode.c:686
> #13 0x0813556a in Feval (form=197747269) at /workplace2/cvs/emacs/src/eval.c:2138
> #14 0x08133012 in Fprogn (args=197747109) at /workplace2/cvs/emacs/src/eval.c:408
> #15 0x0808b687 in Fsave_window_excursion (args=197747109) at /workplace2/cvs/emacs/src/window.c:6123
> #16 0x0815ba35 in Fbyte_code (bytestr=180765267, vector=177288436, maxdepth=208) at /workplace2/cvs/emacs/src/bytecode.c:835
> #17 0x08136455 in funcall_lambda (fun=200312492, nargs=2, arg_vector=0xbfffc5a4) at /workplace2/cvs/emacs/src/eval.c:2974
> #18 0x08136016 in Ffuncall (nargs=3, args=0xbfffc5a0) at /workplace2/cvs/emacs/src/eval.c:2843
> #19 0x08135909 in Fapply (nargs=2, args=0xbfffc620) at /workplace2/cvs/emacs/src/eval.c:2284
> #20 0x08135c11 in apply1 (fn=137633761, arg=193985885) at /workplace2/cvs/emacs/src/eval.c:2545
> #21 0x08132e2c in call_debugger (arg=193985885) at /workplace2/cvs/emacs/src/eval.c:265
> #22 0x08134bd5 in find_handler_clause (handlers=137449369, conditions=137417789, sig=137449393, data=137388369, debugger_value_ptr=0xbfffc704)
>     at /workplace2/cvs/emacs/src/eval.c:1728
> #23 0x0813485c in Fsignal (error_symbol=137449393, data=137388369) at /workplace2/cvs/emacs/src/eval.c:1559
> #24 0x0813c8df in Fplist_get (plist=137984389, prop=137437753) at /workplace2/cvs/emacs/src/fns.c:1993
> #25 0x0813c9c3 in Fget (symbol=137437321, propname=137437753) at /workplace2/cvs/emacs/src/fns.c:2038
> #26 0x080ad371 in resolve_face_name (face_name=137437321) at /workplace2/cvs/emacs/src/xfaces.c:3223
> #27 0x080ad39f in lface_from_face_name (f=0xa816d18, face_name=137437321, signal_p=0) at /workplace2/cvs/emacs/src/xfaces.c:3251
> #28 0x080ad412 in get_lface_attributes (f=0xa816d18, face_name=137437321, attrs=0xbfffc818, signal_p=0) at /workplace2/cvs/emacs/src/xfaces.c:3284
> #29 0x080ada54 in merge_named_face (f=0xa816d18, face_name=137437321, to=0xbfffc9a4, named_merge_points=0xbfffc810)
>     at /workplace2/cvs/emacs/src/xfaces.c:3553
> #30 0x080ade7f in merge_face_ref (f=0xa816d18, face_ref=137437321, to=0xbfffc9a4, err_msgs=1, named_merge_points=0x0)
>     at /workplace2/cvs/emacs/src/xfaces.c:3790
> #31 0x080b2db2 in face_at_buffer_position (w=0xc1a03d8, pos=215, region_beg=-1, region_end=-1, endptr=0xbfffca34, limit=315, mouse=0)
>     at /workplace2/cvs/emacs/src/xfaces.c:7578
> #32 0x08061bd3 in handle_face_prop (it=0xbfffcdf8) at /workplace2/cvs/emacs/src/xdisp.c:2968
> #33 0x08061741 in handle_stop (it=0xbfffcdf8) at /workplace2/cvs/emacs/src/xdisp.c:2703
> #34 0x0806476d in reseat (it=0xbfffcdf8, pos={charpos = 215, bytepos = 215}, force_p=1) at /workplace2/cvs/emacs/src/xdisp.c:4828
> #35 0x080611d7 in init_iterator (it=0xbfffcdf8, w=0xc1a03d8, charpos=215, bytepos=215, row=0xc0e6618, base_face_id=DEFAULT_FACE_ID)
>     at /workplace2/cvs/emacs/src/xdisp.c:2378
> #36 0x08061286 in start_display (it=0xbfffcdf8, w=0xc1a03d8, pos={charpos = 215, bytepos = 215}) at /workplace2/cvs/emacs/src/xdisp.c:2397
> #37 0x0806b59e in redisplay_internal (preserve_echo_area=1) at /workplace2/cvs/emacs/src/xdisp.c:10204
> #38 0x0806beab in redisplay_preserve_echo_area (from_where=8) at /workplace2/cvs/emacs/src/xdisp.c:10649
> #39 0x080e61f8 in detect_input_pending_run_timers (do_display=1) at /workplace2/cvs/emacs/src/keyboard.c:9966
> #40 0x08161c30 in wait_reading_process_output (time_limit=30, microsecs=0, read_kbd=-1, do_display=1, wait_for_cell=137388369, wait_proc=0x0, 
>     just_wait_proc=0) at /workplace2/cvs/emacs/src/process.c:4451
> #41 0x08056d3b in sit_for (sec=30, usec=0, reading=1, display=1, initial_display=0) at /workplace2/cvs/emacs/src/dispnew.c:6393
> #42 0x080ddce1 in read_char (commandflag=1, nmaps=3, maps=0xbfffd5d0, prev_event=137388369, used_mouse_menu=0xbfffd614)
>     at /workplace2/cvs/emacs/src/keyboard.c:2763
> #43 0x080e47ad in read_key_sequence (keybuf=0xbfffd720, bufsize=30, prompt=137388369, dont_downcase_last=0, can_return_switch_frame=1, 
>     fix_current_buffer=1) at /workplace2/cvs/emacs/src/keyboard.c:8872
> #44 0x080dbc80 in command_loop_1 () at /workplace2/cvs/emacs/src/keyboard.c:1523
> #45 0x081344cd in internal_condition_case (bfun=0x80db964 <command_loop_1>, handlers=137449369, hfun=0x80db518 <cmd_error>)
>     at /workplace2/cvs/emacs/src/eval.c:1385
> #46 0x080db7d8 in command_loop_2 () at /workplace2/cvs/emacs/src/keyboard.c:1304
> #47 0x08134005 in internal_catch (tag=137443353, func=0x80db7b4 <command_loop_2>, arg=137388369) at /workplace2/cvs/emacs/src/eval.c:1144
> #48 0x080db787 in command_loop () at /workplace2/cvs/emacs/src/keyboard.c:1283
> #49 0x080db2a4 in recursive_edit_1 () at /workplace2/cvs/emacs/src/keyboard.c:980
> #50 0x080db3e0 in Frecursive_edit () at /workplace2/cvs/emacs/src/keyboard.c:1041
> #51 0x080da0ff in main (argc=1, argv=0xbfffdcf4) at /workplace2/cvs/emacs/src/emacs.c:1767
> The program is running.  Exit anyway? (y or n) 

-- 
Károly

[-- Attachment #1.2: Type: application/pgp-signature, Size: 188 bytes --]

[-- Attachment #2: Type: text/plain, Size: 145 bytes --]

_______________________________________________
multi-tty mailing list
multi-tty@lists.fnord.hu
http://lists.fnord.hu/mailman/listinfo/multi-tty

Re: Face alias bugs (Re: abort() in keyboard.c)

[Kim F. Storm]

lorentey@elte.hu (Károly Lőrentey) writes:

> I have received a backtrace for an aborted multi-tty session from Ami
> Fischman that is relevant to the trunk as well.

Thanks for the report and analysis.

I have installed a different change for this problem which uses a
new safe-get function for the face lookup.  I also added a check
for alias loops.

>
> The abort was caused by Emacs entering the debugger during redisplay
> because of a property list on a face symbol that was not a well-formed
> Lisp list (see frames #23-26 in the backtrace at the bottom).
>
> Beside the (rare) crash condition, there are other symptoms that are
> easier to reproduce.  For example (in a recent CVS):
>
> 	emacs -q
> 	(setplist 'default (append (symbol-plist 'default) (cons 'foo 'bar)))
> 	C-h i
>         <Redisplay loops>
>
> Recursive aliases are fun as well:
>
>         emacs -q
> 	(put 'foo 'face-alias 'default) 
>         (put 'default 'face-alias 'foo)
>         C-h i
>         <unquitable loop>
>
> The patch below fixes all these problems by protecting the call to
> Fget inside resolve_face_name and limiting the number of indirections
> during a single face lookup.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk