Re: Emacs-26 threads problem [win64]

[Eli Zaretskii]

> From: Fabrice Popineau <address@hidden>
> Date: Tue, 10 Oct 2017 22:14:58 +0200
> Cc: Emacs developers <address@hidden>
> 
>  >  Does the problem go away if you insert at the beginning of each test a
>  >  loop like this:
>  >
>  >  (while (> (length (all-threads)) 1)
>  >  (sleep-for 0.1))
>  >
>  > This makes all the 28 tests pass.
> 
>  And the address of stack_pos doesn't grow anymore?
> 
> Yes, it still does. Apparently not out of bounds, because I have run the 
> tests several times. 
> 
>  > A bit scary though.
> 
>  Why scary?
> 
> I don't like to be at the whim of such a waiting loop before being able to 
> run another thread.

We've discussed this issue off-line with Fabrice, and the results of
the investigation seem to point to a GCC bug.  Here are the details:

The faulting instruction is this:

  706       self->m_stack_bottom = (char *) &stack_pos;
  => 0x4001ec91e <run_thread+158>:        movaps %xmm0,0x40(%rbx)

The faulting address:

  (gdb) p/x $rbx
  $1 = 0x400ae5e78
  (gdb) p/x self
  $2 = 0x400ae5e78
  (gdb) p/x $rbx+0x40
  $3 = 0x400ae5eb8
  (gdb) x/x $rbx+0x40
  0x400ae5eb8 <dumped_data+4670424>:      0x00000000

Note that GDB has no problems accessing that address, whereas Emacs
segfaulted in an instruction that accessed it.

The documentation of the MOVAPS instruction says this:

  When the source or destination operand is a memory operand, the
  operand must be aligned on a 16-byte (128-bit version) or 32-byte
  (VEX.256 encoded version) boundary or a general-protection exception
  (#GP) will be generated.

And sure enough, 0x400ae5eb8 is not 16-byte aligned.  Fabrice then
added a dummy member to the tread_state struct:

  diff --git a/src/thread.h b/src/thread.h
  index cb2133d72d..fd06a03071 100644
  --- a/src/thread.h
  +++ b/src/thread.h
  @@ -56,6 +56,8 @@ struct thread_state
        waiting on.  */
     Lisp_Object event_object;

  +  Lisp_Object foobar;
  +
     /* m_stack_bottom must be the first non-Lisp field.  */
     /* An address near the bottom of the stack.
        Tells GC how to save a copy of the stack.  */

and the problem went away.

Further investigation shows that the unaligned address comes from this
fragment of make-thread:

  new_thread = ALLOCATE_PSEUDOVECTOR (struct thread_state, m_stack_bottom,
                                      PVEC_THREAD);

The address of the new_thread structure is then passed as argument to
run_thread a few lines below:

  if (! sys_thread_create (&thr, c_name, run_thread, new_thread))

So it looks like new_thread could be aligned at just 8 bytes, but GCC
assumes for some reason that it's 16-byte aligned, and issues an
instruction that requires such an alignment.  That sounds like a GCC
bug, doesn't it?  Or do heap allocation functions on Posix hosts
return buffers that are always 16-byte aligned?

Note that ALLOCATE_PSEUDOVECTOR could sub-allocate via
allocate_vector_from_block: is this guaranteed to be 16-byte aligned?

I'm not sure how to resolve this issue, so ideas and comments on the
analysis are welcome.