Need some help: 8133646: x86/vm/macroAssembler_x86.cpp:886 DEBUG MESSAGE: StubRoutines::call_stub: threads must correspond

[Vladimir Kozlov]

Additional check:

      Label L1, L2, L3;
       __ cmpptr(r15_thread, thread);
       __ jcc(Assembler::equal, L1);
       __ stop("StubRoutines::call_stub: r15_thread is corrupted");
       __ bind(L1);
       __ get_thread(rbx);
       __ cmpptr(r15_thread, thread);
       __ jcc(Assembler::equal, L2);
       __ stop("StubRoutines::call_stub: r15_thread is modified by call");
       __ bind(L2);
       __ cmpptr(r15_thread, rbx);
       __ jcc(Assembler::equal, L3);
       __ stop("StubRoutines::call_stub: threads must correspond");
       __ bind(L3);

Vladimir

On 8/19/15 11:49 AM, Vladimir Kozlov wrote:
> First, there copy-paste problem in verification code, jcc after bind is
> not needed:
>
>  >     375 __ jcc(Assembler::equal, L);
>  >     376 __ bind(S);
>  >     377 __ jcc(Assembler::equal, L);
>
> Yes, the code assumes that get_thread(rbx); returns correct value and
> will not modify r15. r15 is save-on-entry so the C code should save it.
>
> Is it possible that Sun's C++ compiler has a bug? So that _thr_current
> has wrong value? Or does not save r15?
>
> To make sure that it is the cause of problem I would suggest to modify
> verification code as next:
>
>        Label L1, L2;
>        __ cmpptr(r15_thread, thread);
>        __ jcc(Assembler::equal, L1);
>        __ stop("StubRoutines::call_stub: r15_thread is corrupted");
>        __ bind(L1);
>        __ get_thread(rbx);
>        __ cmpptr(r15_thread, rbx);
>        __ jcc(Assembler::equal, L2);
>        __ stop("StubRoutines::call_stub: threads must correspond");
>        __ bind(L2);
>
>
> Thanks,
> Vladimir
>
> On 8/17/15 8:05 PM, David Holmes wrote:
>> Hi Compiler and x86_64 code experts, :)
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8133646
>>
>> There's a chance this failure is related to my recent changes under
>> 8130212:
>>
>> http://hg.openjdk.java.net/jdk9/hs-rt/hotspot/rev/d5b328043c10
>>
>> Given the failing debug code:
>>
>>     367 #ifdef ASSERT
>>     368 // verify that threads correspond
>>     369 {
>>     370 Label L, S;
>>     371 __ cmpptr(r15_thread, thread);
>>     372 __ jcc(Assembler::notEqual, S);
>>     373 __ get_thread(rbx);
>>     374 __ cmpptr(r15_thread, rbx);
>>     375 __ jcc(Assembler::equal, L);
>>     376 __ bind(S);
>>     377 __ jcc(Assembler::equal, L);
>>     378 __ stop("StubRoutines::call_stub: threads must correspond");
>>     379 __ bind(L);
>>     380 }
>>     381 #endif
>>
>> and given the definition of get_thread:
>>
>> // This is simply a call to ThreadLocalStorage::thread()
>> void MacroAssembler::get_thread(Register thread) {
>>    if (thread != rax) {
>>      push(rax);
>>    }
>>    push(rdi);
>>    push(rsi);
>>    push(rdx);
>>    push(rcx);
>>    push(r8);
>>    push(r9);
>>    push(r10);
>>    push(r11);
>>
>>    call(RuntimeAddress(CAST_FROM_FN_PTR(address,
>> ThreadLocalStorage::thread)));
>>
>>    pop(r11);
>>    pop(r10);
>>    pop(r9);
>>    pop(r8);
>>    pop(rcx);
>>    pop(rdx);
>>    pop(rsi);
>>    pop(rdi);
>>
>>    if (thread != rax) {
>>      movl(thread, rax);
>>      pop(rax);
>>    }
>> }
>>
>> where ThreadLocalStorage::thread() is simply:
>>
>> inline Thread* ThreadLocalStorage::thread()  {
>>    return _thr_current;
>> }
>>
>> and _thr_current is a compiler-defined thread-local variable:
>>
>> static __thread Thread * _thr_current;
>>
>> Can anyone see anything that might cause an issue? I was wondering
>> whether I have to verify that r15 is preserved by the actual
>> thread-local access? IIUC normally the get_thread slow-path would be
>> used to set r15_thread and so we wouldn't notice if it got trashed under
>> some circumstances. It obviously doesn't always get trashed because the
>> bug has only been seen to occur once. And of course the failure could be
>> completely incidental to my changes. :)
>>
>> Thanks,
>> David