RFR: 8378792: ObjectMethods.bootstrap missing getter validation
During review of Valhalla lworld vs mainline patch, I noticed Valhalla has a fix for a mainline issue where there is no getter validation for ObjectMethods.bootstrap. We should upstream this fix to mainline to make lworld cleaner and make mainline more robust. Before this fix, the invalid getters caused weird exceptions in method handle construction. Now they consistently throw IllegalArgumentException as specified. ------------- Commit messages: - 8378792: ObjectMethods.bootstrap missing getter validation Changes: https://git.openjdk.org/jdk/pull/29941/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=29941&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8378792 Stats: 68 lines in 2 files changed: 43 ins; 4 del; 21 mod Patch: https://git.openjdk.org/jdk/pull/29941.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/29941/head:pull/29941 PR: https://git.openjdk.org/jdk/pull/29941
On Thu, 26 Feb 2026 19:15:14 GMT, Chen Liang <liach@openjdk.org> wrote:
During review of Valhalla lworld vs mainline patch, I noticed Valhalla has a fix for a mainline issue where there is no getter validation for ObjectMethods.bootstrap. We should upstream this fix to mainline to make lworld cleaner and make mainline more robust.
Before this fix, the invalid getters caused weird exceptions in method handle construction. Now they consistently throw IllegalArgumentException as specified.
Marked as reviewed by rriggs (Reviewer). test/jdk/java/lang/runtime/ObjectMethodsTest.java line 162:
160: } 161: 162: record NamePlusType(String mn, MethodType mt) {}
Parameter/fields names of `name`, and `type` would make the uses below more readable. ------------- PR Review: https://git.openjdk.org/jdk/pull/29941#pullrequestreview-3863131791 PR Review Comment: https://git.openjdk.org/jdk/pull/29941#discussion_r2860912640
On Thu, 26 Feb 2026 19:30:21 GMT, Roger Riggs <rriggs@openjdk.org> wrote:
Chen Liang has updated the pull request incrementally with one additional commit since the last revision:
Name change as review recommended
test/jdk/java/lang/runtime/ObjectMethodsTest.java line 162:
160: } 161: 162: record NamePlusType(String mn, MethodType mt) {}
Parameter/fields names of `name`, and `type` would make the uses below more readable.
I have renamed these two components, and extracted them in the test method to local variables early on to make the test code more concise. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29941#discussion_r2860948595
On Thu, 26 Feb 2026 19:15:14 GMT, Chen Liang <liach@openjdk.org> wrote:
During review of Valhalla lworld vs mainline patch, I noticed Valhalla has a fix for a mainline issue where there is no getter validation for ObjectMethods.bootstrap. We should upstream this fix to mainline to make lworld cleaner and make mainline more robust.
Before this fix, the invalid getters caused weird exceptions in method handle construction. Now they consistently throw IllegalArgumentException as specified.
Marked as reviewed by jvernee (Reviewer). ------------- PR Review: https://git.openjdk.org/jdk/pull/29941#pullrequestreview-3863134702
During review of Valhalla lworld vs mainline patch, I noticed Valhalla has a fix for a mainline issue where there is no getter validation for ObjectMethods.bootstrap. We should upstream this fix to mainline to make lworld cleaner and make mainline more robust.
Before this fix, the invalid getters caused weird exceptions in method handle construction. Now they consistently throw IllegalArgumentException as specified.
Chen Liang has updated the pull request incrementally with one additional commit since the last revision: Name change as review recommended ------------- Changes: - all: https://git.openjdk.org/jdk/pull/29941/files - new: https://git.openjdk.org/jdk/pull/29941/files/63c94366..880e110a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=29941&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=29941&range=00-01 Stats: 15 lines in 1 file changed: 3 ins; 0 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/29941.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/29941/head:pull/29941 PR: https://git.openjdk.org/jdk/pull/29941
On Thu, 26 Feb 2026 19:42:24 GMT, Chen Liang <liach@openjdk.org> wrote:
During review of Valhalla lworld vs mainline patch, I noticed Valhalla has a fix for a mainline issue where there is no getter validation for ObjectMethods.bootstrap. We should upstream this fix to mainline to make lworld cleaner and make mainline more robust.
Before this fix, the invalid getters caused weird exceptions in method handle construction. Now they consistently throw IllegalArgumentException as specified.
Chen Liang has updated the pull request incrementally with one additional commit since the last revision:
Name change as review recommended
Looks good. ------------- Marked as reviewed by rriggs (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/29941#pullrequestreview-3863821297
On Thu, 26 Feb 2026 19:42:24 GMT, Chen Liang <liach@openjdk.org> wrote:
During review of Valhalla lworld vs mainline patch, I noticed Valhalla has a fix for a mainline issue where there is no getter validation for ObjectMethods.bootstrap. We should upstream this fix to mainline to make lworld cleaner and make mainline more robust.
Before this fix, the invalid getters caused weird exceptions in method handle construction. Now they consistently throw IllegalArgumentException as specified.
Chen Liang has updated the pull request incrementally with one additional commit since the last revision:
Name change as review recommended
Thanks for the reviews! This will facilitate Valhalla cleanups. ------------- PR Comment: https://git.openjdk.org/jdk/pull/29941#issuecomment-3974245446
On Thu, 26 Feb 2026 19:15:14 GMT, Chen Liang <liach@openjdk.org> wrote:
During review of Valhalla lworld vs mainline patch, I noticed Valhalla has a fix for a mainline issue where there is no getter validation for ObjectMethods.bootstrap. We should upstream this fix to mainline to make lworld cleaner and make mainline more robust.
Before this fix, the invalid getters caused weird exceptions in method handle construction. Now they consistently throw IllegalArgumentException as specified.
This pull request has now been integrated. Changeset: 1fb608e1 Author: Chen Liang <liach@openjdk.org> URL: https://git.openjdk.org/jdk/commit/1fb608e1bcbbc3fd68205ea168f10584cc5c2a62 Stats: 71 lines in 2 files changed: 46 ins; 4 del; 21 mod 8378792: ObjectMethods.bootstrap missing getter validation Reviewed-by: rriggs, jvernee ------------- PR: https://git.openjdk.org/jdk/pull/29941
participants (3)
-
Chen Liang
-
Jorn Vernee
-
Roger Riggs