Current trunk aborts with MinGW

Current trunk aborts with MinGW

[martin rudalics]

A MinGW build of current trunk aborts here as follows:


Breakpoint 1, terminate_due_to_signal (sig=22, backtrace_limit=2147483647) at emacs.c:361
361	  signal (sig, SIG_DFL);
(gdb) bt
#0  terminate_due_to_signal (sig=22, backtrace_limit=2147483647) at emacs.c:361
#1  0x01173d6b in die (msg=0x14bb004 "XTYPE (a) == type && XUNTAG (a, type) == ptr", file=0x14baf34 "lisp.h", line=926) at alloc.c:7111
#2  0x010f9651 in make_lisp_ptr (ptr=0x4a7fe0c, type=Lisp_String) at lisp.h:926
#3  0x0101b2a2 in x_get_arg (dpyinfo=0x206d800, alist=..., param=..., attribute=0x14de8e4 "left", class=0x14de8df "Left", type=RES_TYPE_NUMBER) at frame.c:4152
#4  0x01202cc9 in w32_createwindow (f=0x16c70b8) at w32fns.c:1987
#5  0x01203b0e in w32_msg_pump (msg_buf=0x4a7ff54) at w32fns.c:2519
#6  0x01204067 in w32_msg_worker@4 (arg=0x0) at w32fns.c:2724
#7  0x7c80b683 in KERNEL32!GetModuleFileNameA () from C:\WINDOWS\system32\kernel32.dll
#8  0x00000000 in ?? ()

Lisp Backtrace:
"x-create-frame" (0x82e7c8)
"x-create-frame-with-faces" (0x82eb18)
"make-frame" (0x82ee68)
"frame-initialize" (0x82f1b8)
"command-line" (0x82f53c)
"normal-top-level" (0x82f800)


Help welcome, martin

Re: Current trunk aborts with MinGW

[Andy Moreton]

On Tue 30 Sep 2014, martin rudalics wrote:

> A MinGW build of current trunk aborts here as follows:

You have not given enough details:
1) which revision of trunk ?
2) which mingw toolchain and gcc version ?
3) which build options ?
4) what did you do to provoke the bug ?

mingw32 32bit and mingw64 64bit builds work for me at trunk r117982.

    AndyM

Re: Current trunk aborts with MinGW

[martin rudalics]

 > 1) which revision of trunk ?

I tried revisions 117981 and now 117983.

 > 2) which mingw toolchain and gcc version ?

gcc version 4.6.2.  I have no idea what a mingw toolchain version is.

 > 3) which build options ?

CPPFLAGS='-DGLYPH_DEBUG=1' CFLAGS='-O0 -g3' ./configure --prefix=/c/emacs/quickfixes --enable-checking=yes --enable-check-lisp-object-type=yes

 > 4) what did you do to provoke the bug ?

I invoked emacs -Q.

The problem is in this part of frame.c

	  tem = display_x_get_resource
	    (dpyinfo, SCOPED_STRING (attribute),
	     SCOPED_STRING (class), Qnil, Qnil);

but I have no idea how to investigate this.

martin

Re: Current trunk aborts with MinGW

[Andy Moreton]

On Tue 30 Sep 2014, martin rudalics wrote:

>> 1) which revision of trunk ?
>
> I tried revisions 117981 and now 117983.
>
>> 2) which mingw toolchain and gcc version ?
>
> gcc version 4.6.2.  I have no idea what a mingw toolchain version is.

There are several toolchains:

 a) The Mingw project (www.mingw.org):
    - 32bit gcc (i686-pc-mingw32)

 b) The Mingw64 project (mingw-w64.sourceforge.net):
    - 32bit gcc (i686-w64-mingw32)
    - 64bit gcc (x86_64-w64-mingw32)

It helps to know which one you are using as well as the gcc version.

> The problem is in this part of frame.c
>
> 	  tem = display_x_get_resource
> 	    (dpyinfo, SCOPED_STRING (attribute),
> 	     SCOPED_STRING (class), Qnil, Qnil);
>
> but I have no idea how to investigate this.

Eli has analysed the cause of this, and his patch will hopefully fix
your problem (and several other obscure crashes seen recently).

    AndyM

Re: Current trunk aborts with MinGW

[martin rudalics]

 >> I have no idea what a mingw toolchain version is.
 >
 > There are several toolchains:
 >
 >   a) The Mingw project (www.mingw.org):
 >      - 32bit gcc (i686-pc-mingw32)
 >
 >   b) The Mingw64 project (mingw-w64.sourceforge.net):
 >      - 32bit gcc (i686-w64-mingw32)
 >      - 64bit gcc (x86_64-w64-mingw32)

Aha ... that thing printed by `emacs-version' or `system-configuration'.
It's "i686-pc-mingw32" here.  Couldn't you try to write a function that
provides more meaningful information here?  I always consider w32 bug
reports inferior to the ones for GNU/Linux which IMHO are not entirely
optimal either.  Basically, it would be fine to have one function that
gets me, in a nicely formatted way,

- the operating system,
- the revision number,
- the build options,
- the build environment,
- the date of the build

that is, most of the things you asked for.

martin

Re: Current trunk aborts with MinGW

[Eli Zaretskii]

> Date: Tue, 30 Sep 2014 15:51:55 +0200
> From: martin rudalics <rudalics@gmx.at>
> 
> The problem is in this part of frame.c
> 
> 	  tem = display_x_get_resource
> 	    (dpyinfo, SCOPED_STRING (attribute),
> 	     SCOPED_STRING (class), Qnil, Qnil);
> 
> but I have no idea how to investigate this.

When one of the functions that create "scoped" Lisp objects aborts,
the first thing to look at is the addresses of the stack variables:
if they are not 8-byte aligned, that's the reason.  For example:

> #1  0x01173d6b in die (msg=0x14bb004 "XTYPE (a) == type && XUNTAG (a, type) == ptr", file=0x14baf34 "lisp.h", line=926) at alloc.c:7111

See the address of 'msg'?  It's clearly aligned on a 4-byte boundary.

Re: Current trunk aborts with MinGW

[martin rudalics]

 > When one of the functions that create "scoped" Lisp objects aborts,
 > the first thing to look at is the addresses of the stack variables:
 > if they are not 8-byte aligned, that's the reason.  For example:
 >
 >> #1  0x01173d6b in die (msg=0x14bb004 "XTYPE (a) == type && XUNTAG (a, type) == ptr", file=0x14baf34 "lisp.h", line=926) at alloc.c:7111
 >
 > See the address of 'msg'?  It's clearly aligned on a 4-byte boundary.

I wouldn't even have known that 0x14bb004 denotes a stack address.  And
then look whether it ends with "4" or "C" ...

Thanks, martin

Re: Current trunk aborts with MinGW

[Eli Zaretskii]

> Date: Tue, 30 Sep 2014 17:58:18 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: andrewjmoreton@gmail.com, emacs-devel@gnu.org
> 
>  > When one of the functions that create "scoped" Lisp objects aborts,
>  > the first thing to look at is the addresses of the stack variables:
>  > if they are not 8-byte aligned, that's the reason.  For example:
>  >
>  >> #1  0x01173d6b in die (msg=0x14bb004 "XTYPE (a) == type && XUNTAG (a, type) == ptr", file=0x14baf34 "lisp.h", line=926) at alloc.c:7111
>  >
>  > See the address of 'msg'?  It's clearly aligned on a 4-byte boundary.
> 
> I wouldn't even have known that 0x14bb004 denotes a stack address.

You know that by looking at the source, where 'die' is called.  The
variable that is passed as the 1st argument to 'die' is a local
variable in the function that calls 'die', so it is on the stack.

> And then look whether it ends with "4" or "C" ...

I didn't mean to say you should have known.  I explained this so you
could do that in the future.

Re: Current trunk aborts with MinGW

[martin rudalics]

> I didn't mean to say you should have known.  I explained this so you
> could do that in the future.

OK.  I'll try to do that from now on.

Thanks again, martin

Re: Current trunk aborts with MinGW

[Eli Zaretskii]

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Tue, 30 Sep 2014 14:01:46 +0100
> 
> mingw32 32bit and mingw64 64bit builds work for me at trunk r117982.

For me as well, but that's by sheer luck.

The immediate cause of the assertion violation is that we try to
create a Lisp object on the stack, and the stack is not 8-byte
aligned, because we are in a separate thread started by CreateThread,
where the x86 32-bit ABI does not guarantee more than 4-byte
alignment.

But the real problem is that we shouldn't be creating Lisp objects at
all in any thread but the main one, because that's non-reentrant.

Re: Current trunk aborts with MinGW

[Eli Zaretskii]

> Date: Tue, 30 Sep 2014 10:25:49 +0200
> From: martin rudalics <rudalics@gmx.at>
> 
> A MinGW build of current trunk aborts here as follows:
> 
> 
> Breakpoint 1, terminate_due_to_signal (sig=22, backtrace_limit=2147483647) at emacs.c:361
> 361	  signal (sig, SIG_DFL);
> (gdb) bt
> #0  terminate_due_to_signal (sig=22, backtrace_limit=2147483647) at emacs.c:361
> #1  0x01173d6b in die (msg=0x14bb004 "XTYPE (a) == type && XUNTAG (a, type) == ptr", file=0x14baf34 "lisp.h", line=926) at alloc.c:7111
> #2  0x010f9651 in make_lisp_ptr (ptr=0x4a7fe0c, type=Lisp_String) at lisp.h:926
> #3  0x0101b2a2 in x_get_arg (dpyinfo=0x206d800, alist=..., param=..., attribute=0x14de8e4 "left", class=0x14de8df "Left", type=RES_TYPE_NUMBER) at frame.c:4152
> #4  0x01202cc9 in w32_createwindow (f=0x16c70b8) at w32fns.c:1987
> #5  0x01203b0e in w32_msg_pump (msg_buf=0x4a7ff54) at w32fns.c:2519
> #6  0x01204067 in w32_msg_worker@4 (arg=0x0) at w32fns.c:2724
> #7  0x7c80b683 in KERNEL32!GetModuleFileNameA () from C:\WINDOWS\system32\kernel32.dll
> #8  0x00000000 in ?? ()
> 
> Lisp Backtrace:
> "x-create-frame" (0x82e7c8)
> "x-create-frame-with-faces" (0x82eb18)
> "make-frame" (0x82ee68)
> "frame-initialize" (0x82f1b8)
> "command-line" (0x82f53c)
> "normal-top-level" (0x82f800)
> 
> 
> Help welcome, martin

So I think we are very lucky to have this exposed before the release
of 24.4, because this reveals a fatal flaw in our code for the last 6
years: we are consing Lisp objects in a thread other than the main
(a.k.a. "Lisp") thread.  This is an absolute no-no, and can explain
any number of weird crash reports we had in the meantime, especially
from users whose Emacs usage patterns create frames a lot.

I fixed this in the emacs-24 branch (r117524).  The patch is below for
those who need this for the trunk and cannot wait for the merge.

=== modified file 'src/w32fns.c'
--- src/w32fns.c	2014-07-12 09:25:29 +0000
+++ src/w32fns.c	2014-09-30 13:53:24 +0000
@@ -1911,13 +1911,12 @@ w32_createscrollbar (struct frame *f, st
 }
 
 static void
-w32_createwindow (struct frame *f)
+w32_createwindow (struct frame *f, int *coords)
 {
   HWND hwnd;
   RECT rect;
-  Lisp_Object top = Qunbound;
-  Lisp_Object left = Qunbound;
-  struct w32_display_info *dpyinfo = &one_w32_display_info;
+  int top;
+  int left;
 
   rect.left = rect.top = 0;
   rect.right = FRAME_PIXEL_WIDTH (f);
@@ -1932,25 +1931,21 @@ w32_createwindow (struct frame *f)
 
   if (f->size_hint_flags & USPosition || f->size_hint_flags & PPosition)
     {
-      XSETINT (left, f->left_pos);
-      XSETINT (top, f->top_pos);
+      left = f->left_pos;
+      top = f->top_pos;
     }
-  else if (EQ (left, Qunbound) && EQ (top, Qunbound))
+  else
     {
-      /* When called with RES_TYPE_NUMBER, w32_get_arg will return zero
-	 for anything that is not a number and is not Qunbound.  */
-      left = x_get_arg (dpyinfo, Qnil, Qleft, "left", "Left", RES_TYPE_NUMBER);
-      top = x_get_arg (dpyinfo, Qnil, Qtop, "top", "Top", RES_TYPE_NUMBER);
+      left = coords[0];
+      top = coords[1];
     }
 
   FRAME_W32_WINDOW (f) = hwnd
     = CreateWindow (EMACS_CLASS,
 		    f->namebuf,
 		    f->output_data.w32->dwStyle | WS_CLIPCHILDREN,
-		    EQ (left, Qunbound) ? CW_USEDEFAULT : XINT (left),
-		    EQ (top, Qunbound) ? CW_USEDEFAULT : XINT (top),
-		    rect.right - rect.left,
-		    rect.bottom - rect.top,
+		    left, top,
+		    rect.right - rect.left, rect.bottom - rect.top,
 		    NULL,
 		    NULL,
 		    hinst,
@@ -2468,7 +2463,8 @@ w32_msg_pump (deferred_msg * msg_buf)
                  the patch for XP is not publicly available until XP SP3,
                  and older versions will never be patched.  */
               CoInitialize (NULL);
-	      w32_createwindow ((struct frame *) msg.wParam);
+	      w32_createwindow ((struct frame *) msg.wParam,
+				(int *) msg.lParam);
 	      if (!PostThreadMessage (dwMainThreadId, WM_EMACS_DONE, 0, 0))
 		emacs_abort ();
 	      break;
@@ -4069,8 +4065,25 @@ static void
 my_create_window (struct frame * f)
 {
   MSG msg;
+  static int coords[2];
+  Lisp_Object left, top;
+  struct w32_display_info *dpyinfo = &one_w32_display_info;
+
+  /* When called with RES_TYPE_NUMBER, x_get_arg will return zero for
+     anything that is not a number and is not Qunbound.  */
+  left = x_get_arg (dpyinfo, Qnil, Qleft, "left", "Left", RES_TYPE_NUMBER);
+  top = x_get_arg (dpyinfo, Qnil, Qtop, "top", "Top", RES_TYPE_NUMBER);
+  if (EQ (left, Qunbound))
+    coords[0] = CW_USEDEFAULT;
+  else
+    coords[0] = XINT (left);
+  if (EQ (top, Qunbound))
+    coords[1] = CW_USEDEFAULT;
+  else
+    coords[1] = XINT (top);
 
-  if (!PostThreadMessage (dwWindowsThreadId, WM_EMACS_CREATEWINDOW, (WPARAM)f, 0))
+  if (!PostThreadMessage (dwWindowsThreadId, WM_EMACS_CREATEWINDOW,
+			  (WPARAM)f, (LPARAM)coords))
     emacs_abort ();
   GetMessage (&msg, NULL, WM_EMACS_DONE, WM_EMACS_DONE);
 }

Re: Current trunk aborts with MinGW

[martin rudalics]

> I fixed this in the emacs-24 branch (r117524).  The patch is below for
> those who need this for the trunk and cannot wait for the merge.

Thank you, works now.

martin

Re: Current trunk aborts with MinGW

[Eli Zaretskii]

> Date: Tue, 30 Sep 2014 16:11:56 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: emacs-devel@gnu.org
> 
> > I fixed this in the emacs-24 branch (r117524).  The patch is below for
> > those who need this for the trunk and cannot wait for the merge.
> 
> Thank you, works now.

Thanks for testing (and we are lucky you saw this in the first place).

Re: Current trunk aborts with MinGW

[Stefan Monnier]

> A MinGW build of current trunk aborts here as follows:

So all that work on optimizing the code did pay off, in the end.


        Stefan

Re: Current trunk aborts with MinGW

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Tue, 30 Sep 2014 10:33:35 -0400
> Cc: emacs-devel <emacs-devel@gnu.org>
> 
> > A MinGW build of current trunk aborts here as follows:
> 
> So all that work on optimizing the code did pay off, in the end.

Yep, definitely.  (I just couldn't believe my eyes when I saw in
Martin's backtrace make_lisp_ptr being called from w32_msg_pump.)

We could also add a test to functions which allocate memory for Lisp
objects that they are called in the main thread.  Patches for that are
welcome.

Re: Current trunk aborts with MinGW

[martin rudalics]

 > We could also add a test to functions which allocate memory for Lisp
 > objects that they are called in the main thread.  Patches for that are
 > welcome.

Very welcome, indeed.

martin

Re: Current trunk aborts with MinGW

[Eli Zaretskii]

> Date: Tue, 30 Sep 2014 17:57:46 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: emacs-devel@gnu.org
> 
>  > We could also add a test to functions which allocate memory for Lisp
>  > objects that they are called in the main thread.  Patches for that are
>  > welcome.
> 
> Very welcome, indeed.

The thread ID of the main thread is recorded in the variable
dwMainThreadId, and the ID of the current thread can be obtained by
calling GetCurrentThreadId.  If dwMainThreadId is zero, it either was
not yet initialized, or we are running in batch mode, where this
variable is never set (because Emacs runs single-threaded in batch
mode); in both cases, the test should be bypassed.

I think using the above info, adding such a test should be easy.
Volunteers are welcome.