On Fri, 10 Mar 2023 22:14:00 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> Adam Sotona has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 208 commits: >> >> - Merge branch 'master' into JDK-8294972-jlink-plugins >> >> # Conflicts: >> # src/java.base/share/classes/module-info.java >> - Merge branch 'master' into JDK-8294972-jlink-plugins >> - fixed SystemModulesPlugin formatting >> - 8303624: The java.lang.Thread.FieldHolder can be null for JNI attaching >> threads >> >> Reviewed-by: alanb, dcubed >> - 8302360: Atomic*.compareAndExchange Javadoc unclear >> >> Reviewed-by: martin, dholmes >> - 8302779: HelidonAppTest.java fails with "assert(_cb == >> CodeCache::find_blob(pc())) failed: Must be the same" or SIGSEGV >> >> Reviewed-by: coleenp, sspitsyn >> - 8303691: Fedora based devkit build should load more packages from archive >> location >> >> Reviewed-by: mbaesken, erikj >> - 8303924: ProblemList serviceability/sa/UniqueVtableTest.java on Linux >> >> Reviewed-by: dcubed >> - 8303609: ProblemList serviceability/sa/TestSysProps.java with ZGC >> >> Reviewed-by: dcubed >> - 8289765: JDI EventSet/resume/resume008 failed with "ERROR: suspendCounts >> don't match for : VirtualThread-unparker" >> >> Reviewed-by: sspitsyn, kevinw >> - ... and 198 more: https://git.openjdk.org/jdk/compare/b1d89f30...cfc612ec > > src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/StripJavaDebugAttributesPlugin.java > line 62: > >> 60: // XXX. Do we have debug info? Is Asm ready for >> module-info? >> 61: } else { >> 62: byte[] content = newClassReader(path, resource, >> Classfile.Option.processDebug(false)) > > ASM `ClassReader.SKIP_DEBUG` flag indicates to skip the SourceFile, > SourceDebugExtension, LocalVariableTable, LocalVariableTypeTable, > LineNumberTable and MethodParameters attributes. > > `Classfile.Option.processDebug(false)` only skips LocalVariableTable, > LocalVariableTypeTable, CHARACTER_RANGE_TABLE attributes. To implement the > same behavior as `SKIP_DEBUG`, it needs > `Classfile.Option.processLineNumbers(false)` and skipping other attributes. Right, I'll fix it, thanks. > src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/VersionPropsPlugin.java > line 158: > >> 156: }); >> 157: } >> 158: }); > > Suggestion: > > }); > } else { > clb.with(cle); > > > Other `ClassElement`s need to be added to the builder; otherwise they are > dropped. > > One more thing I notice is that the attributes for example `LineNumberTable` > attribute in `<clinit>` method after transformed are dropped. But this > plugin should only transform the code and leave everything else touched. Yes, this transformation is incomplete and not using the latest Classfile API patterns. I'll update it, thanks. ------------- PR: https://git.openjdk.org/jdk/pull/12944
