On Thu, 10 Apr 2025 08:08:32 GMT, Aleksey Shipilev <[email protected]> wrote:
>> Kim Barrett has updated the pull request incrementally with three additional
>> commits since the last revision:
>>
>> - remove timeout by using waitForReferenceProcessing
>> - make ill-timed gc in non-concurrent case less likely
>> - fix test package use
>
> src/java.base/share/classes/java/lang/ref/Reference.java line 357:
>
>> 355: @IntrinsicCandidate
>> 356: public T get() {
>> 357: return get0();
>
> I am looking at this now and wondering how current intrinsics matchers work
> in case of virtual calls.
>
> For example, when type information/profile tells us the receiver is generic
> `Reference`, but in reality it is a `PhantomReference` subclass, would the
> call to `Reference.get()` -- which is actually `PhantomReference.get()`! --
> match accidentally to `Reference.get` intrinsic, and thus enter Access API
> with `ON_WEAK_REF`? Looks pre-existing, and I would have expected native code
> to assert, but I also think at least C2 intrinsics do not check the reference
> type.
>
> It seems both `clear` and `refersTo` side-step all this by: a) not
> intrinsifying the virtual methods; b) doing `AS_NO_KEEPALIVE` -- so they are
> not as exposed. It might be another reason to do this change: to clearly
> separate `Reference.get` intrinsic and `PhantomReference.get` non-intrinsic...
You need to use intrinsic predicates to add runtime check for receiver. See:
https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/library_call.cpp#L167
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24315#discussion_r2038013798
