[jdk17] RFR: 8269697: JNI_GetPrimitiveArrayCritical() should not accept object array [v2]

[David Holmes]

On 1/07/2021 6:49 pm, Aleksey Shipilev wrote:
> On Thu, 1 Jul 2021 00:34:26 GMT, Zhengyu Gu <zgu at openjdk.org> wrote:
> 
>>> GetPrimitiveArrayCritical() is supposed to only be used with primitive array types, but nothing prevents current implementation from accepting object arrays (please see attached test case in bug).
>>>
>>> My purposed fix is not very friendly, it crashes JVM if a none primitive array is passed in, but I am sure what to expect in this scenario.
>>>
>>> Specification people, please comment. Thanks!
>>
>> Zhengyu Gu has updated the pull request incrementally with one additional commit since the last revision:
>>
>>    Kim's comment
> 
> Hold on. This introduces the behavioral change _for the worse_, right? More specifically, the current callers-with-object-array would now get `TypeArrayKlass::cast()`-ed to improper class with unknown consequences. _Hopefully_ it would be a JVM crash, but it might as well be incorrect execution. Old code would execute fine with most GCs (= the ones using GCLocker). Would it be saner to do a copy of object array if such a request is detected?

Making a copy seems a rather bizarre way to legitimize something invalid.

I agree that it might be better to detect and return NULL rather than 
hoping it might crash. But the same can be said for passing bad 
parameters to any JNI method - we don't guarantee fail-fast.

> I believe it warrants a CSR discussion, and therefore postponement to JDK 18.

It had skipped my notice that this was targeted to 17. I do not agree 
with that. This is day one behaviour so not a P3+ bug, so not suitable 
for changing in RDP1. This has to be targeted to 18 and yes a CSR 
request could be worthwhile - though unclear how to handle such a case 
when the spec itself fails to handle it. Perhaps for 18 we need to 
adjust the spec as well to explicitly reject a non-primitive array and 
return NULL.

David

> -------------
> 
> PR: https://git.openjdk.java.net/jdk17/pull/185
>