Re: editfns.c (Fformat): fix for segfault

Re: editfns.c (Fformat): fix for segfault

[Kenichi Handa]

In article <851xzo9f2l.fsf@pi.meyering.net>, Jim Meyering <jim@meyering.net> writes:

> I tried using a snapshot from today's CVS repository with Gnus.
> I got a segfault within a minute or two:

>   pi$ gdb --args  /p/bin/emacs -f gnus-no-server
>   (gdb) r
>   Starting program: /u/p/bin/emacs -f gnus-no-server

>   Program received signal SIGSEGV, Segmentation fault.
>   0x081718e2 in Fformat (nargs=4, args=0xbfffe194)
>       at /mirror/d/emacs/src/editfns.c:3486
>   3486              discarded[format - format_start] = 1;
>   (gdb) p format
>   $1 = (unsigned char *) 0x91788b4 "%S %d %d y\n"
>   (gdb) p format_start
>   $2 = (unsigned char *) 0x90cff34 "%s %d %d y\n"

Thank you for the report.  I found a problem in the code of
Fformat.

> This patch seems to have fixed it, at least to the extent
> that with it, I can no longer reproduce the problem.
[...]
>    /* Scan the format and store result in BUF.  */
>    format = SDATA (args[0]);
> +  format_start = format;
> +  end = format + SBYTES (args[0]);
>    maybe_combine_byte = 0;
>    while (format != end)
>      {

I think it's not enough.

This is my analysis.  In the first scan, Fchar_to_string or
Fprint1_to_string are called, and they will relocate a data
of a Lisp string (in the current case, args[0]).  When that
happens, the area pointed to by `format' becomes invalid.
So, after each call of them, we must update format,
format_start, and end.  Another way is to copy the byte
sequence of args[0] into some safe area alloced or malloced.

Could some Emacs developper confirm my analysis and, if it
is correct, fix the code along this line.  Currently, I
don't have a time to do that by myself.

---
Ken'ichi HANDA
handa@m17n.org

Re: editfns.c (Fformat): fix for segfault

[Richard Stallman]

The only way that the value of format should be so different
from format_start, and yet with the same text, is if
there was a GC and it moved the string.

I don't see what could possibly have caused a GC there.
It would have to be something that calls eval.

If it was really due to a GC, I have doubts that this change is
enough, because the old location of the string could get overwritten
with some other string.  Perhaps it will happen just a minority of the
time, but it can happen.

I added a facility to make Emacs abort if it GCs in the middle
of that code.  That way we will really get to the bottom of this.

    This is my analysis.  In the first scan, Fchar_to_string or
    Fprint1_to_string are called, and they will relocate a data
    of a Lisp string (in the current case, args[0]).

How can either of these functions cause a GC?

Re: editfns.c (Fformat): fix for segfault

[Kenichi Handa]

In article <E19AICm-0005Qs-00@fencepost.gnu.org>, Richard Stallman <rms@gnu.org> writes:
>     This is my analysis.  In the first scan, Fchar_to_string or
>     Fprint1_to_string are called, and they will relocate a data
>     of a Lisp string (in the current case, args[0]).

> How can either of these functions cause a GC?

As I'm not familiar with Emacs' memory allocation, my guess
may be wrong.  What I thought was that buffers and strings
may be relocated without GC if Emacs is using a relocatable
allocator for them.  Only that explains why this bug happens.

---
Ken'ichi HANDA
handa@m17n.org

Re: editfns.c (Fformat): fix for segfault

[Richard Stallman]

    > How can either of these functions cause a GC?

    As I'm not familiar with Emacs' memory allocation, my guess
    may be wrong.  What I thought was that buffers and strings
    may be relocated without GC if Emacs is using a relocatable
    allocator for them.  Only that explains why this bug happens.

Only GC relocates strings.

Re: editfns.c (Fformat): fix for segfault

[Jim Meyering]

Richard Stallman <rms@gnu.org> wrote:
> The only way that the value of format should be so different
> from format_start, and yet with the same text, is if
> there was a GC and it moved the string.
>
> I don't see what could possibly have caused a GC there.
> It would have to be something that calls eval.
>
> If it was really due to a GC, I have doubts that this change is
> enough, because the old location of the string could get overwritten
> with some other string.  Perhaps it will happen just a minority of the
> time, but it can happen.
>
> I added a facility to make Emacs abort if it GCs in the middle
> of that code.  That way we will really get to the bottom of this.
>
>     This is my analysis.  In the first scan, Fchar_to_string or
>     Fprint1_to_string are called, and they will relocate a data
>     of a Lisp string (in the current case, args[0]).
>
> How can either of these functions cause a GC?

Using emacs checked out and built this morning along
with Oort gnus 0.20, that new abort was triggered.

Do you need any more information than the following?

(gdb) r
Starting program: /t/emacs/src/emacs -f gnus-no-server

Program received signal SIGABRT, Aborted.
0x402e2a41 in kill () from /lib/libc.so.6
(gdb) xbacktrace
"format"
"nnmail-generate-active"
"nnmail-save-active"
"nnmail-get-new-mail"
"nnml-request-scan"
"gnus-request-scan"
"gnus-activate-group"
"gnus-get-unread-articles"
"gnus-group-get-new-news"
"call-interactively"
(gdb) w
#0  0x402e2a41 in kill () from /lib/libc.so.6
#1  0x080fefdd in abort () at /mirror/d/emacs/src/emacs.c:412
#2  0x0815f135 in Fgarbage_collect () at /mirror/d/emacs/src/alloc.c:4103
#3  0x08179a9e in Ffuncall (nargs=3, args=0xbfffd910)
    at /mirror/d/emacs/src/eval.c:2664
#4  0x081798a6 in run_hook_list_with_args (funlist=1490478012, nargs=3,
    args=0xbfffd910) at /mirror/d/emacs/src/eval.c:2446
#5  0x0812ad24 in signal_before_change (start_int=1, end_int=1,
    preserve_ptr=0x0) at /mirror/d/emacs/src/insdel.c:2058
#6  0x0812aa5a in prepare_to_modify_buffer (start=1, end=1, preserve_ptr=0x0)
    at /mirror/d/emacs/src/insdel.c:1956
#7  0x081286f8 in insert_1_both (
    string=0x9271ba0 "u\\.seq<@eailebsdÌ=ø\bÀc\0378¬\e'\tÀc\0378´\e'\tÀc\0378¼\e'
\tÀc\0378Ä\e'\tÀc\0378Ì\e'\tÀc\0378Ô\e'\tÀc\0378Ü\e'\tÀc\0378ä\e'\tÀc\0378ì\e'\tÀ
c\0378ô\e'\tÀc\0378ü\e'\tÀc\0378\004\034'\tÀc\0378\f\034'\tÀc\0378\024\034'\tÀc\0
378\034\034'\tÀc\0378$\034'\tÀc\0378,\034'\tÀc\03784\034'\tÀc\0378<\034'\tÀc\0378
D\034'\tÀc\0378L\034'\tÀc\0378T\034'\tÀc\0378"..., nchars=6, nbytes=6,
    inherit=0, prepare=1, before_markers=0) at /mirror/d/emacs/src/insdel.c:1001
#8  0x0818c010 in Fprin1_to_string (object=411432292, noescape=405896804)
    at /mirror/d/emacs/src/print.c:775
#9  0x08171690 in Fformat (nargs=4, args=0xbfffdc24)
    at /mirror/d/emacs/src/editfns.c:3353
#10 0x08179c3c in Ffuncall (nargs=5, args=0xbfffdc20)
    at /mirror/d/emacs/src/eval.c:2705
#11 0x081b1b68 in Fbyte_code (bytestr=948814908, vector=1215031136, maxdepth=6)
    at /mirror/d/emacs/src/bytecode.c:709
#12 0x0817a494 in funcall_lambda (fun=1220009168, nargs=1,
    arg_vector=0xbfffdde4) at /mirror/d/emacs/src/eval.c:2911
#13 0x08179edd in Ffuncall (nargs=2, args=0xbfffdde0)
    at /mirror/d/emacs/src/eval.c:2772
#14 0x081b1b68 in Fbyte_code (bytestr=948817620, vector=1215289248, maxdepth=7)
    at /mirror/d/emacs/src/bytecode.c:709
#15 0x0817a494 in funcall_lambda (fun=1220005768, nargs=2,
    arg_vector=0xbfffdfa4) at /mirror/d/emacs/src/eval.c:2911
#16 0x08179edd in Ffuncall (nargs=3, args=0xbfffdfa0)
    at /mirror/d/emacs/src/eval.c:2772
#17 0x081b1b68 in Fbyte_code (bytestr=948766804, vector=1215174384, maxdepth=17)
    at /mirror/d/emacs/src/bytecode.c:709
#18 0x0817a494 in funcall_lambda (fun=1214748208, nargs=4,
    arg_vector=0xbfffe194) at /mirror/d/emacs/src/eval.c:2911
#19 0x08179edd in Ffuncall (nargs=5, args=0xbfffe190)
    at /mirror/d/emacs/src/eval.c:2772
#20 0x081b1b68 in Fbyte_code (bytestr=952458740, vector=1220896832, maxdepth=5)
    at /mirror/d/emacs/src/bytecode.c:709
#21 0x0817a494 in funcall_lambda (fun=1220896976, nargs=2,
    arg_vector=0xbfffe354) at /mirror/d/emacs/src/eval.c:2911
#22 0x08179edd in Ffuncall (nargs=3, args=0xbfffe350)
    at /mirror/d/emacs/src/eval.c:2772
#23 0x081b1b68 in Fbyte_code (bytestr=951599396, vector=1220038872, maxdepth=4)
    at /mirror/d/emacs/src/bytecode.c:709
#24 0x0817a494 in funcall_lambda (fun=1220039048, nargs=2,
    arg_vector=0xbfffe504) at /mirror/d/emacs/src/eval.c:2911
#25 0x08179edd in Ffuncall (nargs=3, args=0xbfffe500)
    at /mirror/d/emacs/src/eval.c:2772
#26 0x081b1b68 in Fbyte_code (bytestr=949718684, vector=1216787184, maxdepth=10)
    at /mirror/d/emacs/src/bytecode.c:709
#27 0x0817a494 in funcall_lambda (fun=1214237616, nargs=2,
    arg_vector=0xbfffe6d4) at /mirror/d/emacs/src/eval.c:2911
#28 0x08179edd in Ffuncall (nargs=3, args=0xbfffe6d0)
    at /mirror/d/emacs/src/eval.c:2772
#29 0x081b1b68 in Fbyte_code (bytestr=949686140, vector=1213293104, maxdepth=9)
    at /mirror/d/emacs/src/bytecode.c:709
#30 0x0817a494 in funcall_lambda (fun=1214365864, nargs=1,
    arg_vector=0xbfffe8a4) at /mirror/d/emacs/src/eval.c:2911
#31 0x08179edd in Ffuncall (nargs=2, args=0xbfffe8a0)
    at /mirror/d/emacs/src/eval.c:2772
#32 0x081b1b68 in Fbyte_code (bytestr=948486652, vector=1219379032, maxdepth=3)
    at /mirror/d/emacs/src/bytecode.c:709
#33 0x0817a494 in funcall_lambda (fun=1219952584, nargs=1,
    arg_vector=0xbfffea84) at /mirror/d/emacs/src/eval.c:2911
#34 0x08179edd in Ffuncall (nargs=2, args=0xbfffea80)
    at /mirror/d/emacs/src/eval.c:2772
#35 0x08175bfd in Fcall_interactively (function=413867092,
    record_flag=405896804, keys=1222607368) at /mirror/d/emacs/src/callint.c:850
#36 0x08110736 in Fcommand_execute (cmd=413867092, record_flag=405896804,
    keys=405896804, special=405896804) at /mirror/d/emacs/src/keyboard.c:9641
---Type <return> to continue, or q <return> to quit---q

Re: editfns.c (Fformat): fix for segfault

[Richard Stallman]

Does this solve the problem?

*** print.c.~1.185.~	Fri Mar 14 16:00:39 2003
--- print.c	Sat May  3 18:09:00 2003
***************
*** 759,772 ****
  {
    PRINTDECLARE;
    Lisp_Object printcharfun;
!   struct gcpro gcpro1, gcpro2;
!   Lisp_Object tem;
  
    /* Save and restore this--we are altering a buffer
       but we don't want to deactivate the mark just for that.
       No need for specbind, since errors deactivate the mark.  */
!   tem = Vdeactivate_mark;
!   GCPRO2 (object, tem);
  
    printcharfun = Vprin1_to_string_buffer;
    PRINTPREPARE;
--- 759,776 ----
  {
    PRINTDECLARE;
    Lisp_Object printcharfun;
!   /* struct gcpro gcpro1, gcpro2; */
!   Lisp_Object save_deactivate_mark;
!   int count = specpdl_ptr - specpdl;
! 
!   specbind (Qinhibit_modification_hooks, Qt);
  
    /* Save and restore this--we are altering a buffer
       but we don't want to deactivate the mark just for that.
       No need for specbind, since errors deactivate the mark.  */
!   save_deactivate_mark = Vdeactivate_mark;
!   /* GCPRO2 (object, save_deactivate_mark); */
!   abort_on_gc++;
  
    printcharfun = Vprin1_to_string_buffer;
    PRINTPREPARE;
***************
*** 781,790 ****
    Ferase_buffer ();
    set_buffer_internal (old);
  
!   Vdeactivate_mark = tem;
!   UNGCPRO;
  
!   return object;
  }
  
  DEFUN ("princ", Fprinc, Sprinc, 1, 2, 0,
--- 785,795 ----
    Ferase_buffer ();
    set_buffer_internal (old);
  
!   Vdeactivate_mark = save_deactivate_mark;
!   /* UNGCPRO; */
  
!   abort_on_gc--;
!   return unbind_to (count, object);
  }
  
  DEFUN ("princ", Fprinc, Sprinc, 1, 2, 0,

Re: editfns.c (Fformat): fix for segfault

[Jim Meyering]

Richard Stallman <rms@gnu.org> wrote:
> Does this solve the problem?

> *** print.c.~1.185.~	Fri Mar 14 16:00:39 2003
> --- print.c	Sat May  3 18:09:00 2003
...

I applied that patch and rebuilt yesterday.
It just aborted:

Program received signal SIGABRT, Aborted.
0x402e2a41 in kill () from /lib/libc.so.6
(gdb) w
#0  0x402e2a41 in kill () from /lib/libc.so.6
#1  0x080fef29 in abort () at /mirror/d/emacs/src/emacs.c:412
#2  0x0815f2bd in Fgarbage_collect () at /mirror/d/emacs/src/alloc.c:4103
#3  0x08179c26 in Ffuncall (nargs=3, args=0xbfffd8e0)
    at /mirror/d/emacs/src/eval.c:2664
#4  0x08179a2e in run_hook_list_with_args (funlist=1492461932, nargs=3,
    args=0xbfffd8e0) at /mirror/d/emacs/src/eval.c:2446
#5  0x0812aeac in signal_before_change (start_int=1, end_int=11,
    preserve_ptr=0xbfffd9e0) at /mirror/d/emacs/src/insdel.c:2058
#6  0x0812abe2 in prepare_to_modify_buffer (start=1, end=11,
    preserve_ptr=0xbfffd9e0) at /mirror/d/emacs/src/insdel.c:1956
#7  0x0812a2fb in del_range_1 (from=1, to=11, prepare=1, ret_string=0)
    at /mirror/d/emacs/src/insdel.c:1689
#8  0x0812a286 in del_range (from=154412777, to=0)
    at /mirror/d/emacs/src/insdel.c:1663
#9  0x0811f6f5 in Ferase_buffer () at /mirror/d/emacs/src/buffer.c:1975
#10 0x0818c307 in Fprin1_to_string (object=956063660, noescape=405897284)
    at /mirror/d/emacs/src/print.c:785
#11 0x08171818 in Fformat (nargs=4, args=0xbfffdc24)
    at /mirror/d/emacs/src/editfns.c:3353
#12 0x08179dc4 in Ffuncall (nargs=5, args=0xbfffdc20)
    at /mirror/d/emacs/src/eval.c:2705
#13 0x081b1d34 in Fbyte_code (bytestr=949038292, vector=1215229440, maxdepth=6)
    at /mirror/d/emacs/src/bytecode.c:709
#14 0x0817a61c in funcall_lambda (fun=1220041448, nargs=1,
    arg_vector=0xbfffdde4) at /mirror/d/emacs/src/eval.c:2911
#15 0x0817a065 in Ffuncall (nargs=2, args=0xbfffdde0)
    at /mirror/d/emacs/src/eval.c:2772
#16 0x081b1d34 in Fbyte_code (bytestr=949038404, vector=1214792128, maxdepth=7)
    at /mirror/d/emacs/src/bytecode.c:709
...
(gdb) xbacktrace
"format"
"nnmail-generate-active"
"nnmail-save-active"
"nnmail-get-new-mail"
"nnml-request-scan"
"gnus-request-scan"
"gnus-activate-group"
"gnus-get-unread-articles"
"gnus-group-get-new-news"
"call-interactively"

Re: editfns.c (Fformat): fix for segfault

[Richard Stallman]

    I applied that patch and rebuilt yesterday.
    It just aborted:

    Program received signal SIGABRT, Aborted.
    0x402e2a41 in kill () from /lib/libc.so.6
    (gdb) w
    #0  0x402e2a41 in kill () from /lib/libc.so.6
    #1  0x080fef29 in abort () at /mirror/d/emacs/src/emacs.c:412
    #2  0x0815f2bd in Fgarbage_collect () at /mirror/d/emacs/src/alloc.c:4103
    #3  0x08179c26 in Ffuncall (nargs=3, args=0xbfffd8e0)
	at /mirror/d/emacs/src/eval.c:2664
    #4  0x08179a2e in run_hook_list_with_args (funlist=1492461932, nargs=3,
	args=0xbfffd8e0) at /mirror/d/emacs/src/eval.c:2446
    #5  0x0812aeac in signal_before_change (start_int=1, end_int=11,
	preserve_ptr=0xbfffd9e0) at /mirror/d/emacs/src/insdel.c:2058

Binding Qinhibit_modification_hooks to t should have made
signal_before_change return near the beginning.  Can you find
out why this did not work?

Re: editfns.c (Fformat): fix for segfault

[Jim Meyering]

Richard Stallman <rms@gnu.org> wrote:
>     I applied that patch and rebuilt yesterday.
>     It just aborted:
>
>     Program received signal SIGABRT, Aborted.
>     0x402e2a41 in kill () from /lib/libc.so.6
>     (gdb) w
>     #0  0x402e2a41 in kill () from /lib/libc.so.6
>     #1  0x080fef29 in abort () at /mirror/d/emacs/src/emacs.c:412
>     #2  0x0815f2bd in Fgarbage_collect () at /mirror/d/emacs/src/alloc.c:4103
>     #3  0x08179c26 in Ffuncall (nargs=3, args=0xbfffd8e0)
> 	at /mirror/d/emacs/src/eval.c:2664
>     #4  0x08179a2e in run_hook_list_with_args (funlist=1492461932, nargs=3,
> 	args=0xbfffd8e0) at /mirror/d/emacs/src/eval.c:2446
>     #5  0x0812aeac in signal_before_change (start_int=1, end_int=11,
> 	preserve_ptr=0xbfffd9e0) at /mirror/d/emacs/src/insdel.c:2058
>
> Binding Qinhibit_modification_hooks to t should have made
> signal_before_change return near the beginning.  Can you find
> out why this did not work?

I set a watchpoint for inhibit_modification_hooks, and found that
it changes very frequently.  Is that expected?
In fact, it changes so frequently, that attaching a useful action
(like `where' or xbacktrace) to it makes emacs+Gnus nearly unusable.

I presume the goal is to find out which code sets it to 0
just before the abort.

I could endure that if there were some way to trigger the abort
more quickly.  I can't use such an impaired program for the entire
day or so it usually seems to take.

For example, after stopping at a breakpoint in Fgarbage_collect,
I enabled the inhibit_modification_hooks watchpoint and `finish'ed
that function.  The watchpoint was hit *many* times:

  Hardware watchpoint 5: inhibit_modification_hooks

  Old value = 1
  New value = 0
  store_symval_forwarding (symbol=406073020, valcontents=674503112,
      newval=405897284, buf=0x8578e28) at /mirror/d/emacs/src/data.c:889
  889               break;
  (gdb) w
  #0  store_symval_forwarding (symbol=406073020, valcontents=674503112,
      newval=405897284, buf=0x8578e28) at /mirror/d/emacs/src/data.c:889
  #1  0x081645ab in set_internal (symbol=406073020, newval=405897284,
      buf=0x8578e28, bindflag=1) at /mirror/d/emacs/src/data.c:1243
  #2  0x0817ad54 in unbind_to (count=2, value=405897284)
      at /mirror/d/emacs/src/eval.c:3106
  #3  0x080705c0 in with_echo_area_buffer (w=0x0, which=0,
      fn=0x8071296 <current_message_1>, a1=-1073747996, a2=405897284, a3=0, a4=0)
      at /mirror/d/emacs/src/xdisp.c:6809
  #4  0x08071279 in current_message () at /mirror/d/emacs/src/xdisp.c:7227
  #5  0x080712ef in push_message () at /mirror/d/emacs/src/xdisp.c:7262
  #6  0x0815f2f3 in Fgarbage_collect () at /mirror/d/emacs/src/alloc.c:4117
  #7  0x0810510d in read_char (commandflag=1, nmaps=8, maps=0xbfffeaf0,
      prev_event=405897284, used_mouse_menu=0xbfffebdc)
      at /mirror/d/emacs/src/keyboard.c:2717
  #8  0x0810e1a0 in read_key_sequence (keybuf=0xbfffed50, bufsize=30,
      prompt=405897284, dont_downcase_last=0, can_return_switch_frame=1,
      fix_current_buffer=1) at /mirror/d/emacs/src/keyboard.c:8581
  #9  0x0810242b in command_loop_1 () at /mirror/d/emacs/src/keyboard.c:1507
  #10 0x08177cd8 in internal_condition_case (bfun=0x810209a <command_loop_1>,
      handlers=405993916, hfun=0x8101c37 <cmd_error>)
      at /mirror/d/emacs/src/eval.c:1333
  #11 0x08101f5e in command_loop_2 () at /mirror/d/emacs/src/keyboard.c:1295
  #12 0x08177789 in internal_catch (tag=405955220,
      func=0x8101f3f <command_loop_2>, arg=405897284)
      at /mirror/d/emacs/src/eval.c:1094
  #13 0x08101f11 in command_loop () at /mirror/d/emacs/src/keyboard.c:1274
  #14 0x081019de in recursive_edit_1 () at /mirror/d/emacs/src/keyboard.c:990
  #15 0x08101b0d in Frecursive_edit () at /mirror/d/emacs/src/keyboard.c:1046
  #16 0x08100505 in main (argc=3, argv=0xbffff364)
      at /mirror/d/emacs/src/emacs.c:1659