[9] RFR (S): 8071374: Native disassembler implementation may be not thread-safe

[Vladimir Kozlov]

Changes looks good.

Thanks,
Vladimir

On 12/15/15 4:30 AM, Vladimir Ivanov wrote:
> http://cr.openjdk.java.net/~vlivanov/8071374/webrev.00
> https://bugs.openjdk.java.net/browse/JDK-8071374
>
> Disassembler wraps some native disassembler, which is not necessarily thread-safe. It's not a problem for
> -XX:+PrintAssembly since access from compilers is serialized by Compile_lock.
>
> It is not the case anymore when there are calls from runtime (e.g., with -XX:+PrintSignatureHandlers). The problem can
> manifest as a failure to parse instruction stream.
>
> The fix is to serialize access to Disassembler on tty_lock.
>
> Considering most of the calls to Disassembler::decode are performed under tty_lock (which has the lowest rank), it's too
> burdensome to introduce a dedicated lock and a new rank to please deadlock detection logic.
>
> Also, some cleanups are included.
>
> Testing: failing test case from the report, JPRT.
>
> Thanks!
>
> Best regards,
> Vladimir Ivanov
>
> PS: I noted that the following code usually dumps some garbage at the end of the code block:
>
> src/share/vm/interpreter/interpreter.cpp:
> void SignatureHandlerLibrary::add(const methodHandle& method) {
> ...
>    tty->print_cr(" --- associated result handler ---");
>    address rh_end = rh_begin;
>    while (*(int*)rh_end != 0) {
>      rh_end += sizeof(int);
>    }
>    Disassembler::decode(rh_begin, rh_end);
>
> $ java -XX:+PrintSignatureHandlers ...
> ...
> argument handler #0 for: static java.lang.Object.registerNatives()V (fingerprint = 349, 11 bytes generated)
>    0x0000000106d55e60: movabs $0x106c1b118,%rax
>    0x0000000106d55e6a: retq
>   --- associated result handler ---
>    0x0000000106c1b118: retq  // T_VOID:   _native_abi_to_tosca[6]
>    0x0000000106c1b119: retq  // T_FLOAT:  _native_abi_to_tosca[7]
>    0x0000000106c1b11a: retq  // T_DOUBLE: _native_abi_to_tosca[8]
>    // T_OBJECT/T_ARRAY: _native_abi_to_tosca[9]
>    0x0000000106c1b11b: mov    0x10(%rbp),%rax
>    0x0000000106c1b11f: retq
>
> === end of AbstractInterpreter::_native_abi_to_tosca[]
>
> Garbage until (*(int*)rh_end) == 0:
>
>    0x0000000106c1b120: rex add %eax,(%rax)
>    0x0000000106c1b123: add    %cl,%ah
>    0x0000000106c1b125: int3
>    0x0000000106c1b126: int3
>    0x0000000106c1b127: int3
>    0x0000000106c1b128: insl   (%dx),%es:(%rdi)
>    0x0000000106c1b129: sub    $0x10521,%eax
>    0x0000000106c1b12e: add    %al,(%rax)
>    0x0000000106c1b130: (bad)
>    0x0000000106c1b131: (bad)
>    0x0000000106c1b132: (bad)
>    0x0000000106c1b133: dec    %esp
>    0x0000000106c1b135: int3
>    0x0000000106c1b136: int3
>    0x0000000106c1b137: int3
>
> Maybe add some padding in either CodeletMark::~CodeletMark or TemplateInterpreterGenerator::generate_all()?
>
>