On Thu, 21 Nov 2024 21:26:01 GMT, Adam Bruce <d...@openjdk.org> wrote:

>> This PR fixes a long-standing bug in JDWP where the access flags of a field 
>> are not checked before attempting to read it's value. 
>> 
>> Prior to this change, attempting to read a non-static field would cause a 
>> JVM crash, this change corrects that behaviour by returning 
>> `INVALID_FIELDID` instead.
>> 
>> This is my first PR to OpenJDK, so please let me know if I've made any 
>> mistakes in the process.
>> 
>> Cheers,
>> Adam
>
> Adam Bruce has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Move static check to sharedGetFieldValues, add JDWP tests

I mentioned the following above:

> I think JDI might need some fixes to the exception handling. Currently in 
> ReferenceTypeImpl.getValues(), any JDWP exception goes through 
> toJDIException() to convert it to a JDI exception, and it does not handle 
> INVALID_FRAMEID. You'll need to special case it to throw 
> IllegalArgumentException. Same is true in ObjectReferenceImpl.getValues().

This still needs to be addressed and will also need a JDI test case. Note 
ObjectReferenceImpl.getValues() probably doesn't need this fix as stated, since 
it separates the fields into separate arrays of static and non-static, so It 
should never see INVALID_FRAMEID. So I think maybe nothing needs to be done for 
it, but ReferenceTypeImpl.getValues() does need to be fixed and needs a test 
case. You might be able to modify 
nsk/jdi/ReferenceType/getValue/getvalue004.java to do this, or use it as a 
starting point for a new test.

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

PR Comment: https://git.openjdk.org/jdk/pull/22280#issuecomment-2499098017

Reply via email to