RFR: 8318737: Fallback linker passes bad JNI handle

[Jorn Vernee]

On Wed, 25 Oct 2023 10:01:29 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Not sure what you're saying here. As far as I understand the intent of this code is to check whether the handle is of a certain type, and if it's not recognized, return `JNIInvalidRefType`. So, I'm not sure there should be any `ShouldNotReachHere()` in this code.
>> 
>> We can add an `else` branch that returns `JNIInvalidRefType` though.
>
> The old code would throw `ShouldNotReachHere()` if we did not recognize the handle type:
> https://github.com/openjdk/jdk/blob/d2d1592dd94e897fae6fc4098e43b4fffb6d6750/src/hotspot/share/runtime/jniHandles.cpp#L207
> 
> I think the new code should still keep it, like so:
> 
> 
>  } else if (is_local_handle(thread, handle) || is_frame_handle(thread, handle)) {
>    ...
>  } else {
>    ShouldNotReachHere();
>  }
> 
> 
> That way, if we ever have another handle type, we would hit the `else` branch instead of silently returning `JNIInvalidRefType`. That is, our condition chain would still be exhaustive, catching unexpected values explicitly.

But that just re-introduces the bug? I don't see how we can have both `ShouldNotReachHere()` in the 'else branch' and return `JNIInvalidRefType` for unrecognized handles at the same time.

Not that this function is used to check potentially stale/invalid/corrupt JNI handles, so the input handle can be complete garbage.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16349#discussion_r1371485862