Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
But, in emacs-unicode-2, x_encode_text uses encode_coding_object, and that function does call Lisp for pre-write-conversion. Would you please install in emacs-unicode-2 whatever GCPROs are needed in the callers of x_encode_text? ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
If my change broke something, then please undo it; but in that case, please put in whatever GCPROs are needed for x_encode_text. The GCPRO you added in x_encode_text was not deleted. I know that, but we are miscommunicating. There needs to be a GCPRO in x_set_name_internal around the calls to x_encode_text, at least to protect the variable encoded_name. Or else the code needs to be rearranged so encoded_name is not used after the calls to x_encode_text. ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
Ah, now I understand what you are saying. But even if your change had been left in place, wouldn't these GCPRO would have been needed anyway (for the case where HAVE_GTK is false)? I don't think there is any variable that needed protecting in the non-HAVE_GTK case in my version of the code. ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
In article [EMAIL PROTECTED], YAMAMOTO Mitsuharu [EMAIL PROTECTED] writes: On Sun, 03 Jun 2007 12:52:26 -0400, Richard Stallman [EMAIL PROTECTED] said: There needs to be a GCPRO in x_set_name_internal around the calls to x_encode_text, at least to protect the variable encoded_name. Or else the code needs to be rearranged so encoded_name is not used after the calls to x_encode_text. Such GCPROs are not necessary because x_encode_text is called with the third arg SELECTIONP == 0. Actually, there's no SELECTIONP != 0 case in the current Emacs code. I remember that x_encode_text had been used to encode X selection data, but now it is not in Emacs 22. But, in emacs-unicode-2, x_encode_text uses encode_coding_object, and that function does call Lisp for pre-write-conversion. --- Kenichi Handa [EMAIL PROTECTED] ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
On Mon, 04 Jun 2007 09:38:14 +0900, Kenichi Handa [EMAIL PROTECTED] said: Such GCPROs are not necessary because x_encode_text is called with the third arg SELECTIONP == 0. Actually, there's no SELECTIONP != 0 case in the current Emacs code. I remember that x_encode_text had been used to encode X selection data, but now it is not in Emacs 22. But, in emacs-unicode-2, x_encode_text uses encode_coding_object, and that function does call Lisp for pre-write-conversion. That seems to mean there's another string data relocation problem that `text.value' might get corrupted by the subsequent x_encode_text call for `icon.value' in emacs-unicode-2 even on non-GTK+ builds. YAMAMOTO Mitsuharu [EMAIL PROTECTED] ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
Such GCPROs are not necessary because x_encode_text is called with the third arg SELECTIONP == 0. Actually, there's no SELECTIONP != 0 case in the current Emacs code. Ok. How about deleting the arg SELECTIONP in the trunk, to avoid confusion? ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
YAMAMOTO Mitsuharu skrev: On Fri, 01 Jun 2007 18:42:57 -0400, Chong Yidong [EMAIL PROTECTED] said: I looked at the GTK+ sources. Apparently, if we don't set WM_ICON_NAME ourselves, gtk_window_set_icon_name sets WM_ICON_NAME for us. So I think we are perfectly safe. No, I don't think it does, from the Gtk+ documentation: Sets the icon for the window from a named themed icon. See the docs for GtkIconTheme for more details. Note that this has nothing to do with the WM_ICON_NAME property which is mentioned in the ICCCM. What we could use is gdk_window_set_icon_name (Note gdk_, not gtk_). It is available in Gtk+ 2.4 and onwards, 2.4 is the minimum Emacs require. But this function blindly assumes the name passed is in UTF-8 and sets both _NET_WM_ICON_NAME and WM_ICON_NAME to that. x_set_name_internal at least tries to decode things correctly for the window manager. But now we can't set the icon title separately from the frame title using the `icon-name' frame parameter on GTK+ builds. `icon-name' The name to use in the icon for this frame, when and if the icon appears. If this is `nil', the frame's title is used. Even with non-GTK+ builds, setting this parameter might not affect the icon title for some window managers. But at least, CDE on Solaris respects this. Maybe we can use gtk_window_set_icon_name for this purpose, but it is available only on GTK+ 2.6 and later, so some check will be necessary and it is definitely after-the-release matter. So I propose undoing the latest change for x_set_name_internal for now. The reason for using gtk functions (gtk_window_set_title) is that the information is saved in Gtk+, and also that the function sets _NET_WM_ICON_NAME. But it also sets WM_ICON_NAME, but with a possible wrong encoding, so we call XSetWMIconName to fix that. Come to think of it, all builds should set _NET_WM_ICON_NAME to the UTF-8 encoded icon name. Jan D. ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
So I propose undoing the latest change for x_set_name_internal for now. If my change broke something, then please undo it; but in that case, please put in whatever GCPROs are needed for x_encode_text. ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
Richard Stallman [EMAIL PROTECTED] writes: So I propose undoing the latest change for x_set_name_internal for now. If my change broke something, then please undo it; but in that case, please put in whatever GCPROs are needed for x_encode_text. The GCPRO you added in x_encode_text was not deleted. ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
YAMAMOTO Mitsuharu [EMAIL PROTECTED] writes: I think it crashed while creating the new frame; I don't recall seeing the frame appear, but I'm not absolutely sure. I think this is due to string data relocation caused by ENCODE_UTF_8 in x_set_name_internal (GTK+ only). Amazing! Thanks for fixing this. -- Kim F. Storm [EMAIL PROTECTED] http://www.cua.dk ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
Thanks for guessing this. After your change, there was another similar bug: x_encode_text can call Lisp code, which would have needed a GCPRO. After looking at it, I realized there was no need to call x_encode_text in the GTK case. So I separated the two cases completely, which should improve things. Would people please test setting the name, in both GTK and non-GTK builds? ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
Richard Stallman [EMAIL PROTECTED] writes: Thanks for guessing this. After your change, there was another similar bug: x_encode_text can call Lisp code, which would have needed a GCPRO. After looking at it, I realized there was no need to call x_encode_text in the GTK case. So I separated the two cases completely, which should improve things. Would people please test setting the name, in both GTK and non-GTK builds? Setting the frame title works fine on both GTK and non-GTK builds. ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
On Fri, 01 Jun 2007 10:51:12 -0400, Richard Stallman [EMAIL PROTECTED] said: After looking at it, I realized there was no need to call x_encode_text in the GTK case. So I separated the two cases completely, which should improve things. I'm not so familiar with GTK+, but is the call to XSetWMIconName unnecessary in the GTK+ case? Previously, it was called in both cases and that was the reason for x_encode_text in the GTK+ case. YAMAMOTO Mitsuharu [EMAIL PROTECTED] ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
YAMAMOTO Mitsuharu [EMAIL PROTECTED] writes: On Fri, 01 Jun 2007 10:51:12 -0400, Richard Stallman [EMAIL PROTECTED] said: After looking at it, I realized there was no need to call x_encode_text in the GTK case. So I separated the two cases completely, which should improve things. I'm not so familiar with GTK+, but is the call to XSetWMIconName unnecessary in the GTK+ case? Previously, it was called in both cases and that was the reason for x_encode_text in the GTK+ case. I believe this was an oversight. However, on my Gnome desktop, it makes no observable difference. According to the Xlib specs, XSetWMIconName sets the WM_ICON_NAME window property, which is a string that the client wants to be displayed in association with the window when it is iconified (for example, in an icon label). However, I see the same iconfied window names for GTK and non-GTK builds, both before and after RMS's change. If we want to be safe, we can undo RMS's refactoring on the branch, going back to YAMAMOTO Mitsuharu's patch plus an additional GCPRO for x_encode_text. In either case, we should be getting on with the release! ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
Setting the frame title works fine on both GTK and non-GTK builds. That is good. Please make the release tomorrow if no reason comes up why not. ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
Chong Yidong [EMAIL PROTECTED] writes: YAMAMOTO Mitsuharu [EMAIL PROTECTED] writes: After looking at it, I realized there was no need to call x_encode_text in the GTK case. So I separated the two cases completely, which should improve things. I'm not so familiar with GTK+, but is the call to XSetWMIconName unnecessary in the GTK+ case? Previously, it was called in both cases and that was the reason for x_encode_text in the GTK+ case. I believe this was an oversight. However, on my Gnome desktop, it makes no observable difference. According to the Xlib specs, XSetWMIconName sets the WM_ICON_NAME window property, which is a string that the client wants to be displayed in association with the window when it is iconified (for example, in an icon label). However, I see the same iconfied window names for GTK and non-GTK builds, both before and after RMS's change. I looked at the GTK+ sources. Apparently, if we don't set WM_ICON_NAME ourselves, gtk_window_set_icon_name sets WM_ICON_NAME for us. So I think we are perfectly safe. ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
On Fri, 01 Jun 2007 18:42:57 -0400, Chong Yidong [EMAIL PROTECTED] said: I looked at the GTK+ sources. Apparently, if we don't set WM_ICON_NAME ourselves, gtk_window_set_icon_name sets WM_ICON_NAME for us. So I think we are perfectly safe. But now we can't set the icon title separately from the frame title using the `icon-name' frame parameter on GTK+ builds. `icon-name' The name to use in the icon for this frame, when and if the icon appears. If this is `nil', the frame's title is used. Even with non-GTK+ builds, setting this parameter might not affect the icon title for some window managers. But at least, CDE on Solaris respects this. Maybe we can use gtk_window_set_icon_name for this purpose, but it is available only on GTK+ 2.6 and later, so some check will be necessary and it is definitely after-the-release matter. So I propose undoing the latest change for x_set_name_internal for now. YAMAMOTO Mitsuharu [EMAIL PROTECTED] ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
YAMAMOTO Mitsuharu [EMAIL PROTECTED] writes: I looked at the GTK+ sources. Apparently, if we don't set WM_ICON_NAME ourselves, gtk_window_set_icon_name sets WM_ICON_NAME for us. So I think we are perfectly safe. But now we can't set the icon title separately from the frame title using the `icon-name' frame parameter on GTK+ builds. So I propose undoing the latest change for x_set_name_internal for now. I have checked in a patch to revert x_set_name_internal to the version before the latest RMS change. The GCPRO added to x_encode_text has been kept. ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
Reiner Steib [EMAIL PROTECTED] writes: report-emacs-bug wrote: Please describe exactly what actions triggered the bug and the precise symptoms of the bug: My Emacs session which has been already running for several days (notebook with suspend to disk) crashed. I don't recall which command triggered the bug. (gdb) bt #0 0xb74fc4ca in memcpy () from /lib/libc.so.6 #1 0xb77a049e in XChangeProperty () from /usr/lib/libX11.so.6 #2 0xb77c0711 in XSetTextProperty () from /usr/lib/libX11.so.6 #3 0xb77c0793 in XSetWMIconName () from /usr/lib/libX11.so.6 #4 0x080d550a in x_set_name_internal (f=0xa4dd960, name=222515027) at ~/.../cvs-HEAD/emacs/src/xfns.c:1653 #5 0x0807efec in prepare_menu_bars () at ~/.../cvs-HEAD/emacs/src/xdisp.c:9015 ... I'll dig around and see what I can find, but OTOH this looks like a crash in the X libraries, probably not related to Emacs. ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
Chong Yidong [EMAIL PROTECTED] writes: Reiner Steib [EMAIL PROTECTED] writes: report-emacs-bug wrote: Please describe exactly what actions triggered the bug and the precise symptoms of the bug: My Emacs session which has been already running for several days (notebook with suspend to disk) crashed. I don't recall which command triggered the bug. (gdb) bt #0 0xb74fc4ca in memcpy () from /lib/libc.so.6 #1 0xb77a049e in XChangeProperty () from /usr/lib/libX11.so.6 #2 0xb77c0711 in XSetTextProperty () from /usr/lib/libX11.so.6 #3 0xb77c0793 in XSetWMIconName () from /usr/lib/libX11.so.6 #4 0x080d550a in x_set_name_internal (f=0xa4dd960, name=222515027) at ~/.../cvs-HEAD/emacs/src/xfns.c:1653 #5 0x0807efec in prepare_menu_bars () at ~/.../cvs-HEAD/emacs/src/xdisp.c:9015 ... I'll dig around and see what I can find, but OTOH this looks like a crash in the X libraries, probably not related to Emacs. I had a crash yesterday with the emacs 22 branch (not fully up-to-date, but close): GNU Emacs 22.0.990.1 (i686-pc-linux-gnu, GTK+ Version 2.8.20) of 2007-05-23 on kfs-lx It is built on an up-to-date Debian Stable system. Emacs had probably been running for approx. 20 hours, and the last command I executed which triggered the crash was C-x 5 b (that's ido-switch-buffer-other-frame), selected a buffer from the list, and hit RET. I think it crashed while creating the new frame; I don't recall seeing the frame appear, but I'm not absolutely sure. The value of default-frame-alist is: ((vertical-scroll-bars . right) (left . 10) (top . 50) (font . -Misc-Fixed-Medium-R-Normal--13-120-75-75-C-80-ISO8859-1) (wait-for-wm) (tool-bar-lines . 1) (menu-bar-lines . 1) (right-fringe) (left-fringe)) My window manager is KDE Unfortunately, it happened while I was installing Debian on a new PC, so I wasn't running under gdb at the time. I've tried to reproduce the crash at least a 100 times, but haven't been able to repeat it. -- Kim F. Storm [EMAIL PROTECTED] http://www.cua.dk ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
On Fri, 01 Jun 2007 00:35:33 +0200, [EMAIL PROTECTED] (Kim F. Storm) said: I had a crash yesterday with the emacs 22 branch (not fully up-to-date, but close): GNU Emacs 22.0.990.1 (i686-pc-linux-gnu, GTK+ Version 2.8.20) of 2007-05-23 on kfs-lx It is built on an up-to-date Debian Stable system. Emacs had probably been running for approx. 20 hours, and the last command I executed which triggered the crash was C-x 5 b (that's ido-switch-buffer-other-frame), selected a buffer from the list, and hit RET. I think it crashed while creating the new frame; I don't recall seeing the frame appear, but I'm not absolutely sure. I think this is due to string data relocation caused by ENCODE_UTF_8 in x_set_name_internal (GTK+ only). YAMAMOTO Mitsuharu [EMAIL PROTECTED] Index: src/xfns.c === RCS file: /sources/emacs/emacs/src/xfns.c,v retrieving revision 1.681 diff -c -p -r1.681 xfns.c *** src/xfns.c 24 Mar 2007 15:40:38 - 1.681 --- src/xfns.c 1 Jun 2007 01:00:51 - *** x_set_name_internal (f, name) *** 1605,1610 --- 1605,1615 int bytes, stringp; int do_free_icon_value = 0, do_free_text_value = 0; Lisp_Object coding_system; + #ifdef USE_GTK + /* As ENCODE_UTF_8 may cause GC and relocation of string data, + we use it before x_encode_text that may return string data. */ + Lisp_Object encoded_name = ENCODE_UTF_8 (name); + #endif coding_system = Qcompound_text; /* Note: Encoding strategy *** x_set_name_internal (f, name) *** 1645,1651 #ifdef USE_GTK gtk_window_set_title (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)), ! (char *) SDATA (ENCODE_UTF_8 (name))); #else /* not USE_GTK */ XSetWMName (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f), text); #endif /* not USE_GTK */ --- 1650,1656 #ifdef USE_GTK gtk_window_set_title (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)), ! (char *) SDATA (encoded_name)); #else /* not USE_GTK */ XSetWMName (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f), text); #endif /* not USE_GTK */ ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
On Fri, 01 Jun 2007 10:05:41 +0900, YAMAMOTO Mitsuharu [EMAIL PROTECTED] said: I think this is due to string data relocation caused by ENCODE_UTF_8 in x_set_name_internal (GTK+ only). GCPRO was missing in the previous patch. Below is a revised one. YAMAMOTO Mitsuharu [EMAIL PROTECTED] Index: src/xfns.c === RCS file: /sources/emacs/emacs/src/xfns.c,v retrieving revision 1.681 diff -c -p -r1.681 xfns.c *** src/xfns.c 24 Mar 2007 15:40:38 - 1.681 --- src/xfns.c 1 Jun 2007 03:24:43 - *** x_set_name_internal (f, name) *** 1605,1610 --- 1605,1620 int bytes, stringp; int do_free_icon_value = 0, do_free_text_value = 0; Lisp_Object coding_system; + #ifdef USE_GTK + Lisp_Object encoded_name; + struct gcpro gcpro1; + + /* As ENCODE_UTF_8 may cause GC and relocation of string data, + we use it before x_encode_text that may return string data. */ + GCPRO1 (name); + encoded_name = ENCODE_UTF_8 (name); + UNGCPRO; + #endif coding_system = Qcompound_text; /* Note: Encoding strategy *** x_set_name_internal (f, name) *** 1645,1651 #ifdef USE_GTK gtk_window_set_title (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)), ! (char *) SDATA (ENCODE_UTF_8 (name))); #else /* not USE_GTK */ XSetWMName (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f), text); #endif /* not USE_GTK */ --- 1655,1661 #ifdef USE_GTK gtk_window_set_title (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)), ! (char *) SDATA (encoded_name)); #else /* not USE_GTK */ XSetWMName (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f), text); #endif /* not USE_GTK */ ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
YAMAMOTO Mitsuharu [EMAIL PROTECTED] writes: On Fri, 01 Jun 2007 10:05:41 +0900, YAMAMOTO Mitsuharu [EMAIL PROTECTED] said: I think this is due to string data relocation caused by ENCODE_UTF_8 in x_set_name_internal (GTK+ only). GCPRO was missing in the previous patch. Below is a revised one. Yes. I dunno how you managed to deduce that this is the reason for the reported bug, but the patch looks like it DTRT. I've checked it into the branch and trunk for you. ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
What version are you running? If it is from CVS, is it the trunk or which branch? ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
Re: Crash: XSetWMIconName, x_set_name_internal, prepare_menu_bars
On Fri, Jun 01 2007, YAMAMOTO Mitsuharu wrote: I think it crashed while creating the new frame; I don't recall seeing the frame appear, but I'm not absolutely sure. I think Emacs was creating a frame while/before my crash as well. I think this is due to string data relocation caused by ENCODE_UTF_8 in x_set_name_internal (GTK+ only). Thanks for analyzing the problem and providing the patch. Bye, Reiner. -- ,,, (o o) ---ooO-(_)-Ooo--- | PGP key available | http://rsteib.home.pages.de/ ___ emacs-pretest-bug mailing list emacs-pretest-bug@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
