On Tue, 2 May 2023 14:15:27 GMT, Adam Sotona <asot...@openjdk.org> wrote:
>> Classfile API didn't handle transformations of class files version 50 and >> below correctly. >> >> Proposed fix have two parts: >> 1. Inflation of branch targets does not depend on StackMapTable attribute >> presence for class file version 50 and below. Alternative fallback >> implementation is provided. >> 2. StackMapTable attribute is not generated for class file versions below 50. >> >> StackMapsTest is also extended to test this patch. >> >> Please review. >> >> Thanks, >> Adam > > Adam Sotona has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains 12 additional > commits since the last revision: > > - Merge branch 'master' into JDK-8305990-debug-info-strip-fail > - Update StackCounter.java > - added comments to StackCounter about maxStack upper bounds calculation for > JSR/RET instructions > - fixed stack counting of JSR instructions > - implemented StackCounter > - Making some BufWriter fields final > - Update > src/java.base/share/classes/jdk/internal/classfile/impl/AbstractInstruction.java > > Co-authored-by: liach <7806504+li...@users.noreply.github.com> > - Update src/java.base/share/classes/jdk/internal/classfile/Opcode.java > > Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com> > - fixed ClassPrinterImpl > - DiscontinuedInstruction implementation + test > - ... and 2 more: https://git.openjdk.org/jdk/compare/995f6608...f132f737 Also, has brian reviewed the DiscontinuedInstruction API changes? src/java.base/share/classes/jdk/internal/classfile/impl/StackCounter.java line 109: > 107: maxLocals = isStatic ? 0 : 1; > 108: for (var cd : methodDesc.parameterList()) { > 109: maxLocals += > TypeKind.fromDescriptor(cd.descriptorString()).slotSize(); Suggestion: maxLocals += TypeKind.from(cd).slotSize(); src/java.base/share/classes/jdk/internal/classfile/impl/StackCounter.java line 310: > 308: var mDesc = > MethodTypeDesc.ofDescriptor(nameAndType.type().stringValue()); > 309: for (var arg : mDesc.parameterList()) { > 310: > stack(-TypeKind.fromDescriptor(arg.descriptorString()).slotSize()); Suggestion: stack(-TypeKind.from(arg).slotSize()); src/java.base/share/classes/jdk/internal/classfile/impl/StackCounter.java line 315: > 313: stack(-1); > 314: } > 315: > stack(TypeKind.fromDescriptor(mDesc.returnType().descriptorString()).slotSize()); Suggestion: stack(TypeKind.from(mDesc.returnType()).slotSize()); src/java.base/share/classes/jdk/internal/classfile/impl/StackCounter.java line 378: > 376: bcs.bci, > 377: methodName, > 378: > methodDesc.parameterList().stream().map(ClassDesc::displayName).collect(Collectors.joining(",")))); Suggestion: methodDesc.displayDescriptor()); ------------- PR Review: https://git.openjdk.org/jdk/pull/13478#pullrequestreview-1413358082 PR Review Comment: https://git.openjdk.org/jdk/pull/13478#discussion_r1185172984 PR Review Comment: https://git.openjdk.org/jdk/pull/13478#discussion_r1185174342 PR Review Comment: https://git.openjdk.org/jdk/pull/13478#discussion_r1185174639 PR Review Comment: https://git.openjdk.org/jdk/pull/13478#discussion_r1185176343
