bug#18438: 24.4.50; assertion failed in bidi.c

[Eli Zaretskii <eliz@gnu.org>]

> Date: Fri, 10 Oct 2014 21:57:24 -0400
> From: Ken Brown <kbrown@cornell.edu>
> CC: aidalgol@amuri.net, 18438@debbugs.gnu.org
> 
> On 10/10/2014 11:12 AM, Eli Zaretskii wrote:
> > Something vague about the upper 32 bits of the 64-bit registers.
> > (Yes, I'm desperate.)
> 
> I'm desperate too.  Here's another thought: Suppose this really is a 
> thread-safety issue in some way that we don't understand.  Then maybe 
> the problem is that the test 'type <= 23' is not atomic in the 
> compilation that Aidan and I have been doing.  First 'type' is copied 
> from ECX to RBP+0x10, then the latter is tested.

That's true; but note that the value at RBP+0x10 is the one passed to
fprintf (by pushing it on the stack via EDX), and it printed correctly.

> We could make it 
> atomic by forcing GCC to directly test ECX <= 23.  We can do this by 
> compiling with -Og instead of -O0. (Aidan and I have both been using -O0.)
> 
> The resulting disassembly (based on your earlier patch, in 
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=18438#103) is
> 
> Dump of assembler code for function bidi_check_type:
>     0x00000001004ee9db <+0>:     push   %rbx
>     0x00000001004ee9dc <+1>:     sub    $0x40,%rsp
>     0x00000001004ee9e0 <+5>:     mov    %ecx,%ebx
>     0x00000001004ee9e2 <+7>:     mov    0x543027(%rip),%rax        # 
> 0x100a31a10 <.refptr.suppress_checking>
>     0x00000001004ee9e9 <+14>:    cmpb   $0x0,(%rax)
>     0x00000001004ee9ec <+17>:    jne    0x1004eea2f <bidi_check_type+84>
>     0x00000001004ee9ee <+19>:    cmp    $0x17,%ecx
>     0x00000001004ee9f1 <+22>:    jbe    0x1004eea2f <bidi_check_type+84>
>     0x00000001004ee9f3 <+24>:    callq  0x10069fd40 <__getreent>
>     0x00000001004ee9f8 <+29>:    mov    0x18(%rax),%rcx
>     0x00000001004ee9fc <+33>:    movl   $0x17,0x30(%rsp)
>     0x00000001004eea04 <+41>:    movl   $0x0,0x28(%rsp)
>     0x00000001004eea0c <+49>:    mov    %ebx,0x20(%rsp)
>     0x00000001004eea10 <+53>:    mov    $0x14c,%r9d
>     0x00000001004eea16 <+59>:    lea    0x51e713(%rip),%r8        # 
> 0x100a0d130 <chartab_size+112>
>     0x00000001004eea1d <+66>:    lea    0x51e814(%rip),%rdx        # 
> 0x100a0d238 <chartab_size+376>
>     0x00000001004eea24 <+73>:    callq  0x1006a0040 <fprintf>
>     0x00000001004eea29 <+78>:    callq  0x10065227d <emacs_abort>
>     0x00000001004eea2e <+83>:    nop
>     0x00000001004eea2f <+84>:    add    $0x40,%rsp
>     0x00000001004eea33 <+88>:    pop    %rbx
>     0x00000001004eea34 <+89>:    retq
> End of assembler dump.
> 
> Do you think this is worth trying (perhaps after Aidan tries your other 
> suggestion, involving 64-bit registers)?

Holding a value in a register AFAIU actually makes the probability of
a clobber by another thread higher than keeping it on the stack.

But I think any idea is worth trying at this time, certainly including
yours.  Thanks.

Btw, note that the above version copies the argument into EBX, which
is then pushed onto the stack (10 instructions later) before calling
fprintf.  This is somewhat different from the original code, which
held the value in a temporary variable on the stack instead of in EBX.
Not sure this matters, just mentioning it for the record.