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 Thanks for the feedback, I've moved the check into sharedGetFieldValues. Unfortunately, this requires an allocation, as we need to check the fields before the length is written meaning we need to read the field IDs into an array so we have access to them after checking them. My initial change was also not suitable as writeStaticFieldValue and writeFieldValue were called after the length had been written. I've added some additional JDWP tests based on the two tests you suggested above. They use the exact same test classes but with all static fields converted to instance fields and vice versa. They also send a command for each field, to ensure the correct error is returned for all field types. I also added an additional helper method to verify that the expected error is present on ReplyPacket. I've also verified that all of the tests you listed above pass on my local machine (Linux) ------------- PR Comment: https://git.openjdk.org/jdk/pull/22280#issuecomment-2492380425
