RFR: 8133646: Internal Error: x86/vm/macroAssembler_x86.cpp:886 DEBUG MESSAGE: StubRoutines::call_stub: threads must correspond

[Daniel D. Daugherty]

On 8/25/15 8:17 PM, David Holmes wrote:
> Hi Coleen,
>
> Thanks for the review.
>
> On 26/08/2015 2:28 AM, Coleen Phillimore wrote:
>>
>> David,
>> Can you add this duplicate code into a verify_thread() function? This
>> would help if we are ever able to combine common code in
>> stubRoutines_x86_32/64.cpp
>
> I'm so grateful you posed that as a question not a request. :) Answer: 
> no.
>
> The duplicated structure is pre-existing, and the error messages 
> differ so the context would have to be passed in. I'm totally 
> unfamiliar with playing in the macroassembler so would be unsure how 
> to factor this out into a separate function without messing up 
> register usage, or using the correct "variables" from the "calling 
> context".
>
> Also note that for some reason these sanity checks don't exist on 
> x86_32, nor sparc (only aarch64 also has it presumably based on the 
> x86_64 version). I think the time to factor this out would be if/when 
> we do the code combining.

We do have a similar check on x86_32:

src/cpu/x86/vm/stubGenerator_x86_32.cpp:

    343    address generate_catch_exception() {
<snip>
    351  #ifdef ASSERT
    352      // verify that threads correspond
    353      { Label L;
    354        __ get_thread(rbx);
    355        __ cmpptr(rbx, rcx);
    356        __ jcc(Assembler::equal, L);
    357        __ stop("StubRoutines::catch_exception: threads must 
correspond");
    358        __ bind(L);
    359      }
    360  #endif

The check is much simpler because we don't have enough registers
on x86_32 to set aside one for the thread.

On SPARC we have this:

src/cpu/sparc/vm/stubGenerator_sparc.cpp:

    322    address generate_catch_exception() {
<snip>
    326      // verify that thread corresponds
    327      __ verify_thread();

There are lots of other calls to verify_thread() sprinkled
through the SPARC code...

src/cpu/sparc/vm/macroAssembler_sparc.cpp:

    398  void MacroAssembler::verify_thread() {
    399    if (VerifyThread) {
    400      // NOTE: this chops off the heads of the 64-bit O registers.
    401  #ifdef CC_INTERP
    402      save_frame(0);
    403  #else
    404      // make sure G2_thread contains the right value
    405      save_frame_and_mov(0, Lmethod, Lmethod);   // to avoid 
clobbering O0
  (and propagate Lmethod for -Xprof)
    406      mov(G1, L1);                // avoid clobbering G1
    407      // G2 saved below
    408      mov(G3, L3);                // avoid clobbering G3
    409      mov(G4, L4);                // avoid clobbering G4
    410      mov(G5_method, L5);         // avoid clobbering G5_method
    411  #endif /* CC_INTERP */
    412  #if defined(COMPILER2) && !defined(_LP64)
    413      // Save & restore possible 64-bit Long arguments in G-regs
    414      srlx(G1,32,L0);
    415      srlx(G4,32,L6);
    416  #endif
    417 call(CAST_FROM_FN_PTR(address,verify_thread_subroutine), relocInfo::
runtime_call_type);
    418      delayed()->mov(G2_thread, O0);
    419
    420      mov(L1, G1);                // Restore G1
    421      // G2 restored below
    422      mov(L3, G3);                // restore G3
    423      mov(L4, G4);                // restore G4
    424      mov(L5, G5_method);         // restore G5_method
    425  #if defined(COMPILER2) && !defined(_LP64)
    426      // Save & restore possible 64-bit Long arguments in G-regs
    427      sllx(L0,32,G2);             // Move old high G1 bits high in G2
    428      srl(G1, 0,G1);              // Clear current high G1 bits
    429      or3 (G1,G2,G1);             // Recover 64-bit G1
    430      sllx(L6,32,G2);             // Move old high G4 bits high in G2
    431      srl(G4, 0,G4);              // Clear current high G4 bits
    432      or3 (G4,G2,G4);             // Recover 64-bit G4
    433  #endif
    434      restore(O0, 0, G2_thread);
    435    }
    436  }

Does anyone else have a headache after reading verify_thread()? :-)

Dan


>
> Thanks,
> David
>
>> Thanks,
>> Coleen
>>
>> On 8/25/15 2:07 AM, David Holmes wrote:
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8133646
>>>
>>> webrev: http://cr.openjdk.java.net/~dholmes/8133646/webrev/
>>>
>>> The error raised in this bug was not reproducible - it was only seen
>>> on one machine which was subsequently re-imaged to run Windows. A
>>> similar Solaris system did not reproduce the bug.
>>>
>>> Rather than just close this as not-reproducible I want to push the
>>> enhanced diagnostics that Vladimir provided here:
>>>
>>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2015-August/019667.html 
>>>
>>>
>>>
>>> which I've added to the two chunks of code that perform this thread
>>> sanity check.
>>>
>>> Thanks,
>>> David
>>