RFR: 8261837: SIGSEGV in ciVirtualCallTypeData::translate_from [v2]

[Vladimir Ivanov]

On Tue, 21 Nov 2023 23:34:21 GMT, Dean Long <dlong at openjdk.org> wrote:

>> Type profiling code based on the x86 implementation uses XOR to check if the MDO value matches the klass, then later stores that XORed value into the MDO if the MDO value was 0.  However, there is a race here if we reload the MDO value to check for 0, resulting in storing OBJ_KLASS XOR MDO_KLASS back to the MDO.
>> 
>> I took a stab at riscv, but I don't have a way to test it.
>
> Dean Long has updated the pull request incrementally with one additional commit since the last revision:
> 
>   redo for x86

Nice catch, Dean!

src/hotspot/cpu/riscv/c1_LIRAssembler_riscv.cpp line 1689:

> 1687:     __ andi(tmp, tmp, TypeEntries::type_mask);
> 1688:     __ verify_klass_ptr(tmp);
> 1689:   }

Missing `#endif`?

src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp line 3634:

> 3632: #ifdef ASSERT
> 3633:   if (obj == tmp) {
> 3634:     LP64_ONLY(assert_different_registers(obj, rscratch1, mdo_addr.base(), mdo_addr.index());)

I mind `#ifdef _LP64` more readable than `LP64_ONLY`/`NOT_LP64` here.

src/hotspot/cpu/x86/interp_masm_x86.cpp line 58:

> 56: 
> 57: #ifdef _LP64
> 58:   assert_different_registers(obj, rscratch1, mdo_addr.base(), mdo_addr.index());

Same here: pass `rscratch1` unconditionally?

src/hotspot/cpu/x86/interp_masm_x86.cpp line 70:

> 68:   jccb(Assembler::notZero, next); // null already seen. Nothing to do anymore.
> 69:   // atomic update to prevent overwriting Klass* with 0
> 70:   lock();

Introduce `MacroAssembler::locked_orptr()`? There are multiple `MacroAssembler::atomic_*` and `MacroAssembler::locked_*` helper methods there.

-------------

Marked as reviewed by vlivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16750#pullrequestreview-1743343831
PR Review Comment: https://git.openjdk.org/jdk/pull/16750#discussion_r1401347956
PR Review Comment: https://git.openjdk.org/jdk/pull/16750#discussion_r1401349555
PR Review Comment: https://git.openjdk.org/jdk/pull/16750#discussion_r1401353163
PR Review Comment: https://git.openjdk.org/jdk/pull/16750#discussion_r1401361492