On Thu, 10 Apr 2025 18:26:39 GMT, Vladimir Kozlov <k...@openjdk.org> wrote:
>> 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
I think it would be "easier" to shift `@IntrinsicCandidate` to a private
`Reference.getImpl` method, and intrinsify that instead. Pretty much like
current `clear` and `refersTo` are doing it now.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24315#discussion_r2039019037
