From b2e91060db82a13e993227f538c8d54b41a9796b Mon Sep 17 00:00:00 2001 From: Adam Sotona Date: Thu, 14 Sep 2023 18:52:51 +0000 Subject: [PATCH] 8313452: Improve Classfile API attributes handling safety Reviewed-by: briangoetz --- .../internal/classfile/AttributeMapper.java | 40 +++ .../jdk/internal/classfile/Attributes.java | 270 +++++++++++++++--- .../jdk/internal/classfile/Classfile.java | 21 +- .../classfile/impl/AbstractDirectBuilder.java | 4 +- .../classfile/impl/AnnotationReader.java | 12 +- .../classfile/impl/BoundAttribute.java | 53 ++-- .../internal/classfile/impl/ClassImpl.java | 2 +- .../classfile/impl/ClassfileImpl.java | 10 +- .../jdk/internal/classfile/impl/Util.java | 19 +- test/jdk/jdk/classfile/OptionsTest.java | 133 +++++++++ 10 files changed, 457 insertions(+), 107 deletions(-) create mode 100644 test/jdk/jdk/classfile/OptionsTest.java diff --git a/src/java.base/share/classes/jdk/internal/classfile/AttributeMapper.java b/src/java.base/share/classes/jdk/internal/classfile/AttributeMapper.java index 8892b56ed60..2914a9d5110 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/AttributeMapper.java +++ b/src/java.base/share/classes/jdk/internal/classfile/AttributeMapper.java @@ -37,6 +37,41 @@ package jdk.internal.classfile; */ public interface AttributeMapper { + /** + * Attribute stability indicator + */ + enum AttributeStability { + + /** + * The attribute contains only pure data, such as timestamps, and can always be bulk-copied. + */ + STATELESS, + + /** + * The attribute contains only pure data and CP refs, so can be bulk-copied when CP sharing is in effect, + * and need to be exploded and rewritten when CP sharing is not in effect. + */ + CP_REFS, + + /** + * The attribute may contain labels, so need to be exploded and rewritten when the Code array is perturbed. + */ + LABELS, + + /** + * The attribute may contain indexes into structured not managed by the library (type variable lists, etc) + * and so we consult the {@link Classfile.AttributesProcessingOption} option to determine whether to preserve + * or drop it during transformation. + */ + UNSTABLE, + + /** + * The attribute is completely unknown and so we consult the {@link Classfile.AttributesProcessingOption} option + * to determine whether to preserve or drop it during transformation. + */ + UNKNOWN + } + /** * {@return the name of the attribute} */ @@ -75,4 +110,9 @@ public interface AttributeMapper { default boolean allowMultiple() { return false; } + + /** + * {@return attribute stability indicator} + */ + AttributeStability stability(); } diff --git a/src/java.base/share/classes/jdk/internal/classfile/Attributes.java b/src/java.base/share/classes/jdk/internal/classfile/Attributes.java index 699da664a9b..4e230a161ac 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/Attributes.java +++ b/src/java.base/share/classes/jdk/internal/classfile/Attributes.java @@ -215,6 +215,11 @@ public class Attributes { protected void writeBody(BufWriter buf, AnnotationDefaultAttribute attr) { attr.defaultValue().writeTo(buf); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; /** Attribute mapper for the {@code BootstrapMethods} attribute */ @@ -229,6 +234,11 @@ public class Attributes { protected void writeBody(BufWriter buf, BootstrapMethodsAttribute attr) { buf.writeList(attr.bootstrapMethods()); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; /** Attribute mapper for the {@code CharacterRangeTable} attribute */ @@ -251,6 +261,11 @@ public class Attributes { buf.writeU2(info.flags()); } } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.LABELS; + } }; /** Attribute mapper for the {@code Code} attribute */ @@ -265,6 +280,11 @@ public class Attributes { protected void writeBody(BufWriter buf, CodeAttribute attr) { throw new UnsupportedOperationException("Code attribute does not support direct write"); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; @@ -280,6 +300,11 @@ public class Attributes { protected void writeBody(BufWriter buf, CompilationIDAttribute attr) { buf.writeIndex(attr.compilationId()); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; /** Attribute mapper for the {@code ConstantValue} attribute */ @@ -294,6 +319,11 @@ public class Attributes { protected void writeBody(BufWriter buf, ConstantValueAttribute attr) { buf.writeIndex(attr.constant()); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; /** Attribute mapper for the {@code Deprecated} attribute */ @@ -308,6 +338,11 @@ public class Attributes { protected void writeBody(BufWriter buf, DeprecatedAttribute attr) { // empty } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.STATELESS; + } }; /** Attribute mapper for the {@code EnclosingMethod} attribute */ @@ -323,6 +358,11 @@ public class Attributes { buf.writeIndex(attr.enclosingClass()); buf.writeIndexOrZero(attr.enclosingMethod().orElse(null)); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; /** Attribute mapper for the {@code Exceptions} attribute */ @@ -337,6 +377,11 @@ public class Attributes { protected void writeBody(BufWriter buf, ExceptionsAttribute attr) { buf.writeListIndices(attr.exceptions()); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; /** Attribute mapper for the {@code InnerClasses} attribute */ @@ -358,6 +403,11 @@ public class Attributes { buf.writeU2(ic.flagsMask()); } } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; /** Attribute mapper for the {@code LineNumberTable} attribute */ @@ -377,6 +427,11 @@ public class Attributes { buf.writeU2(line.lineNumber()); } } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.LABELS; + } }; /** Attribute mapper for the {@code LocalVariableTable} attribute */ @@ -399,6 +454,11 @@ public class Attributes { buf.writeU2(info.slot()); } } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.LABELS; + } }; /** Attribute mapper for the {@code LocalVariableTypeTable} attribute */ @@ -421,6 +481,11 @@ public class Attributes { buf.writeU2(info.slot()); } } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.LABELS; + } }; /** Attribute mapper for the {@code MethodParameters} attribute */ @@ -440,47 +505,57 @@ public class Attributes { buf.writeU2(info.flagsMask()); } } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; /** Attribute mapper for the {@code Module} attribute */ public static final AttributeMapper MODULE = new AbstractAttributeMapper<>(NAME_MODULE, Classfile.JAVA_9_VERSION) { - @Override - public ModuleAttribute readAttribute(AttributedElement e, ClassReader cf, int p) { - return new BoundAttribute.BoundModuleAttribute(cf, this, p); - } + @Override + public ModuleAttribute readAttribute(AttributedElement e, ClassReader cf, int p) { + return new BoundAttribute.BoundModuleAttribute(cf, this, p); + } - @Override - protected void writeBody(BufWriter buf, ModuleAttribute attr) { - buf.writeIndex(attr.moduleName()); - buf.writeU2(attr.moduleFlagsMask()); - buf.writeIndexOrZero(attr.moduleVersion().orElse(null)); - buf.writeU2(attr.requires().size()); - for (ModuleRequireInfo require : attr.requires()) { - buf.writeIndex(require.requires()); - buf.writeU2(require.requiresFlagsMask()); - buf.writeIndexOrZero(require.requiresVersion().orElse(null)); - } - buf.writeU2(attr.exports().size()); - for (ModuleExportInfo export : attr.exports()) { - buf.writeIndex(export.exportedPackage()); - buf.writeU2(export.exportsFlagsMask()); - buf.writeListIndices(export.exportsTo()); - } - buf.writeU2(attr.opens().size()); - for (ModuleOpenInfo open : attr.opens()) { - buf.writeIndex(open.openedPackage()); - buf.writeU2(open.opensFlagsMask()); - buf.writeListIndices(open.opensTo()); - } - buf.writeListIndices(attr.uses()); - buf.writeU2(attr.provides().size()); - for (ModuleProvideInfo provide : attr.provides()) { - buf.writeIndex(provide.provides()); - buf.writeListIndices(provide.providesWith()); - } - } - }; + @Override + protected void writeBody(BufWriter buf, ModuleAttribute attr) { + buf.writeIndex(attr.moduleName()); + buf.writeU2(attr.moduleFlagsMask()); + buf.writeIndexOrZero(attr.moduleVersion().orElse(null)); + buf.writeU2(attr.requires().size()); + for (ModuleRequireInfo require : attr.requires()) { + buf.writeIndex(require.requires()); + buf.writeU2(require.requiresFlagsMask()); + buf.writeIndexOrZero(require.requiresVersion().orElse(null)); + } + buf.writeU2(attr.exports().size()); + for (ModuleExportInfo export : attr.exports()) { + buf.writeIndex(export.exportedPackage()); + buf.writeU2(export.exportsFlagsMask()); + buf.writeListIndices(export.exportsTo()); + } + buf.writeU2(attr.opens().size()); + for (ModuleOpenInfo open : attr.opens()) { + buf.writeIndex(open.openedPackage()); + buf.writeU2(open.opensFlagsMask()); + buf.writeListIndices(open.opensTo()); + } + buf.writeListIndices(attr.uses()); + buf.writeU2(attr.provides().size()); + for (ModuleProvideInfo provide : attr.provides()) { + buf.writeIndex(provide.provides()); + buf.writeListIndices(provide.providesWith()); + } + } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } + }; /** Attribute mapper for the {@code ModuleHashes} attribute */ public static final AttributeMapper @@ -501,6 +576,11 @@ public class Attributes { buf.writeBytes(hash.hash()); } } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; /** Attribute mapper for the {@code ModuleMainClass} attribute */ @@ -515,6 +595,11 @@ public class Attributes { protected void writeBody(BufWriter buf, ModuleMainClassAttribute attr) { buf.writeIndex(attr.mainClass()); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; /** Attribute mapper for the {@code ModulePackages} attribute */ @@ -529,6 +614,11 @@ public class Attributes { protected void writeBody(BufWriter buf, ModulePackagesAttribute attr) { buf.writeListIndices(attr.packages()); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; /** Attribute mapper for the {@code ModuleResolution} attribute */ @@ -543,6 +633,11 @@ public class Attributes { protected void writeBody(BufWriter buf, ModuleResolutionAttribute attr) { buf.writeU2(attr.resolutionFlags()); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.STATELESS; + } }; /** Attribute mapper for the {@code ModuleTarget} attribute */ @@ -557,6 +652,11 @@ public class Attributes { protected void writeBody(BufWriter buf, ModuleTargetAttribute attr) { buf.writeIndex(attr.targetPlatform()); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; /** Attribute mapper for the {@code NestHost} attribute */ @@ -571,6 +671,11 @@ public class Attributes { protected void writeBody(BufWriter buf, NestHostAttribute attr) { buf.writeIndex(attr.nestHost()); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; /** Attribute mapper for the {@code NestMembers} attribute */ @@ -585,6 +690,11 @@ public class Attributes { protected void writeBody(BufWriter buf, NestMembersAttribute attr) { buf.writeListIndices(attr.nestMembers()); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; /** Attribute mapper for the {@code PermittedSubclasses} attribute */ @@ -599,6 +709,11 @@ public class Attributes { protected void writeBody(BufWriter buf, PermittedSubclassesAttribute attr) { buf.writeListIndices(attr.permittedSubclasses()); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; /** Attribute mapper for the {@code Record} attribute */ @@ -619,6 +734,11 @@ public class Attributes { buf.writeList(info.attributes()); } } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; /** Attribute mapper for the {@code RuntimeInvisibleAnnotations} attribute */ @@ -633,7 +753,12 @@ public class Attributes { protected void writeBody(BufWriter buf, RuntimeInvisibleAnnotationsAttribute attr) { buf.writeList(attr.annotations()); } - }; + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } + }; /** Attribute mapper for the {@code RuntimeInvisibleParameterAnnotations} attribute */ public static final AttributeMapper @@ -650,6 +775,11 @@ public class Attributes { for (List list : lists) buf.writeList(list); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; /** Attribute mapper for the {@code RuntimeInvisibleTypeAnnotations} attribute */ @@ -664,21 +794,31 @@ public class Attributes { protected void writeBody(BufWriter buf, RuntimeInvisibleTypeAnnotationsAttribute attr) { buf.writeList(attr.annotations()); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.UNSTABLE; + } }; /** Attribute mapper for the {@code RuntimeVisibleAnnotations} attribute */ public static final AttributeMapper RUNTIME_VISIBLE_ANNOTATIONS = new AbstractAttributeMapper<>(NAME_RUNTIME_VISIBLE_ANNOTATIONS, Classfile.JAVA_5_VERSION) { - @Override - public RuntimeVisibleAnnotationsAttribute readAttribute(AttributedElement enclosing, ClassReader cf, int pos) { - return new BoundAttribute.BoundRuntimeVisibleAnnotationsAttribute(cf, pos); - } + @Override + public RuntimeVisibleAnnotationsAttribute readAttribute(AttributedElement enclosing, ClassReader cf, int pos) { + return new BoundAttribute.BoundRuntimeVisibleAnnotationsAttribute(cf, pos); + } - @Override - protected void writeBody(BufWriter buf, RuntimeVisibleAnnotationsAttribute attr) { - buf.writeList(attr.annotations()); - } - }; + @Override + protected void writeBody(BufWriter buf, RuntimeVisibleAnnotationsAttribute attr) { + buf.writeList(attr.annotations()); + } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } + }; /** Attribute mapper for the {@code RuntimeVisibleParameterAnnotations} attribute */ public static final AttributeMapper @@ -695,6 +835,11 @@ public class Attributes { for (List list : lists) buf.writeList(list); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; /** Attribute mapper for the {@code RuntimeVisibleTypeAnnotations} attribute */ @@ -709,6 +854,11 @@ public class Attributes { protected void writeBody(BufWriter buf, RuntimeVisibleTypeAnnotationsAttribute attr) { buf.writeList(attr.annotations()); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.UNSTABLE; + } }; /** Attribute mapper for the {@code Signature} attribute */ @@ -723,6 +873,11 @@ public class Attributes { protected void writeBody(BufWriter buf, SignatureAttribute attr) { buf.writeIndex(attr.signature()); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; /** Attribute mapper for the {@code SourceDebugExtension} attribute */ @@ -737,6 +892,11 @@ public class Attributes { protected void writeBody(BufWriter buf, SourceDebugExtensionAttribute attr) { buf.writeBytes(attr.contents()); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.STATELESS; + } }; /** Attribute mapper for the {@code SourceFile} attribute */ @@ -751,6 +911,11 @@ public class Attributes { protected void writeBody(BufWriter buf, SourceFileAttribute attr) { buf.writeIndex(attr.sourceFile()); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; /** Attribute mapper for the {@code SourceID} attribute */ @@ -765,6 +930,11 @@ public class Attributes { protected void writeBody(BufWriter buf, SourceIDAttribute attr) { buf.writeIndex(attr.sourceId()); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.CP_REFS; + } }; /** Attribute mapper for the {@code StackMapTable} attribute */ @@ -779,6 +949,11 @@ public class Attributes { protected void writeBody(BufWriter b, StackMapTableAttribute attr) { StackMapDecoder.writeFrames(b, attr.entries()); } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.LABELS; + } }; @@ -794,6 +969,11 @@ public class Attributes { protected void writeBody(BufWriter buf, SyntheticAttribute attr) { // empty } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.STATELESS; + } }; /** diff --git a/src/java.base/share/classes/jdk/internal/classfile/Classfile.java b/src/java.base/share/classes/jdk/internal/classfile/Classfile.java index f8913f99b51..7ee9a8299ca 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/Classfile.java +++ b/src/java.base/share/classes/jdk/internal/classfile/Classfile.java @@ -241,17 +241,22 @@ public sealed interface Classfile } /** - * Option describing whether to process or discard unrecognized attributes. - * Default is {@code PASS_UNKNOWN_ATTRIBUTES} to process unrecognized - * attributes, and deliver as instances of {@link UnknownAttribute}. + * Option describing whether to process or discard unrecognized or problematic + * original attributes when a class, record component, field, method or code is + * transformed in its exploded form. + * Default is {@code PASS_ALL_ATTRIBUTES} to process all original attributes. + * @see AttributeMapper.AttributeStability */ - enum UnknownAttributesOption implements Option { + enum AttributesProcessingOption implements Option { - /** Process unknown attributes */ - PASS_UNKNOWN_ATTRIBUTES, + /** Process all original attributes during transformation */ + PASS_ALL_ATTRIBUTES, - /** Drop unknown attributes */ - DROP_UNKNOWN_ATTRIBUTES + /** Drop unknown attributes during transformation */ + DROP_UNKNOWN_ATTRIBUTES, + + /** Drop unknown and unstable original attributes during transformation */ + DROP_UNSTABLE_ATRIBUTES; } /** diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/AbstractDirectBuilder.java b/src/java.base/share/classes/jdk/internal/classfile/impl/AbstractDirectBuilder.java index ed9268f5372..4707b4bc46a 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/AbstractDirectBuilder.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/AbstractDirectBuilder.java @@ -52,6 +52,8 @@ public class AbstractDirectBuilder { } public void writeAttribute(Attribute a) { - attributes.withAttribute(a); + if (Util.isAttributeAllowed(a, context.attributesProcessingOption())) { + attributes.withAttribute(a); + } } } diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/AnnotationReader.java b/src/java.base/share/classes/jdk/internal/classfile/impl/AnnotationReader.java index ca4e0b1228b..ae80c1ced00 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/AnnotationReader.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/AnnotationReader.java @@ -51,7 +51,7 @@ class AnnotationReader { annos[i] = readAnnotation(classReader, pos); pos = skipAnnotation(classReader, pos); } - return SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArrayNullsAllowed(annos); + return SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArray(annos); } public static AnnotationValue readElementValue(ClassReader classReader, int p) { @@ -78,7 +78,7 @@ class AnnotationReader { values[i] = readElementValue(classReader, p); p = skipElementValue(classReader, p); } - yield new AnnotationImpl.OfArrayImpl(SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArrayNullsAllowed(values)); + yield new AnnotationImpl.OfArrayImpl(SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArray(values)); } default -> throw new IllegalArgumentException( "Unexpected tag '%s' in AnnotationValue, pos = %d".formatted(tag, p - 1)); @@ -93,7 +93,7 @@ class AnnotationReader { annotations[i] = readTypeAnnotation(classReader, p, lc); p = skipTypeAnnotation(classReader, p); } - return SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArrayNullsAllowed(annotations); + return SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArray(annotations); } public static List> readParameterAnnotations(ClassReader classReader, int p) { @@ -103,7 +103,7 @@ class AnnotationReader { pas[i] = readAnnotations(classReader, p); p = skipAnnotations(classReader, p); } - return SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArrayNullsAllowed(pas); + return SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArray(pas); } private static int skipElementValue(ClassReader classReader, int p) { @@ -156,7 +156,7 @@ class AnnotationReader { annotationElements[i] = new AnnotationImpl.AnnotationElementImpl(elementName, value); p = skipElementValue(classReader, p); } - return SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArrayNullsAllowed(annotationElements); + return SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArray(annotationElements); } private static int skipElementValuePairs(ClassReader classReader, int p) { @@ -257,7 +257,7 @@ class AnnotationReader { classReader.readU2(p + 4)); p += 6; } - return SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArrayNullsAllowed(entries); + return SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArray(entries); } private static int skipTypeAnnotation(ClassReader classReader, int p) { diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/BoundAttribute.java b/src/java.base/share/classes/jdk/internal/classfile/impl/BoundAttribute.java index 9b8c579e9e2..1a518b10f4d 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/BoundAttribute.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/BoundAttribute.java @@ -26,6 +26,7 @@ package jdk.internal.classfile.impl; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Optional; import java.util.function.Function; @@ -121,15 +122,16 @@ public abstract sealed class BoundAttribute> for (int i = 0; p < end; i++, p += 2) { entries[i] = classReader.readEntry(p); } - return SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArrayNullsAllowed(entries); + return SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArray(entries); } public static List> readAttributes(AttributedElement enclosing, ClassReader reader, int pos, Function> customAttributes) { int size = reader.readU2(pos); - var filled = new Object[size]; + var filled = new ArrayList>(size); int p = pos + 2; int cfLen = reader.classfileLength(); + var apo = ((ClassReaderImpl)reader).context().attributesProcessingOption(); for (int i = 0; i < size; ++i) { Utf8Entry name = reader.readUtf8Entry(p); int len = reader.readInt(p + 2); @@ -143,8 +145,8 @@ public abstract sealed class BoundAttribute> mapper = customAttributes.apply(name); } if (mapper != null) { - filled[i] = mapper.readAttribute(enclosing, reader, p); - } else if (((ClassReaderImpl)reader).context().unknownAttributesOption() == Classfile.UnknownAttributesOption.PASS_UNKNOWN_ATTRIBUTES) { + filled.add((Attribute)mapper.readAttribute(enclosing, reader, p)); + } else { AttributeMapper fakeMapper = new AttributeMapper<>() { @Override public String name() { @@ -159,19 +161,27 @@ public abstract sealed class BoundAttribute> @Override public void writeAttribute(BufWriter buf, UnknownAttribute attr) { - throw new UnsupportedOperationException("Write of unknown attribute " + name() + " not supported"); + buf.writeIndex(name); + var cont = attr.contents(); + buf.writeInt(cont.length); + buf.writeBytes(cont); } @Override public boolean allowMultiple() { return true; } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeStability.UNKNOWN; + } }; - filled[i] = new BoundUnknownAttribute(reader, fakeMapper, p); + filled.add(new BoundUnknownAttribute(reader, fakeMapper, p)); } p += len; } - return SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArrayNullsAllowed(filled); + return Collections.unmodifiableList(filled); } public static final class BoundUnknownAttribute extends BoundAttribute @@ -179,35 +189,6 @@ public abstract sealed class BoundAttribute> public BoundUnknownAttribute(ClassReader cf, AttributeMapper mapper, int pos) { super(cf, mapper, pos); } - - @Override - public void writeTo(DirectClassBuilder builder) { - checkWriteSupported(builder::canWriteDirect); - super.writeTo(builder); - } - - @Override - public void writeTo(DirectMethodBuilder builder) { - checkWriteSupported(builder::canWriteDirect); - super.writeTo(builder); - } - - @Override - public void writeTo(DirectFieldBuilder builder) { - checkWriteSupported(builder::canWriteDirect); - super.writeTo(builder); - } - - @Override - public void writeTo(BufWriter buf) { - checkWriteSupported(buf::canWriteDirect); - super.writeTo(buf); - } - - private void checkWriteSupported(Function condition) { - if (!condition.apply(classReader)) - throw new UnsupportedOperationException("Write of unknown attribute " + attributeName() + " not supported to alien constant pool"); - } } public static final class BoundStackMapTableAttribute diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/ClassImpl.java b/src/java.base/share/classes/jdk/internal/classfile/impl/ClassImpl.java index 455ad35b526..97ecda3f381 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/ClassImpl.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/ClassImpl.java @@ -138,7 +138,7 @@ public final class ClassImpl arr[i] = reader.readClassEntry(pos); pos += 2; } - this.interfaces = SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArrayNullsAllowed(arr); + this.interfaces = SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArray(arr); } return interfaces; } diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/ClassfileImpl.java b/src/java.base/share/classes/jdk/internal/classfile/impl/ClassfileImpl.java index 6610de76659..a3c6e6b1b65 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/ClassfileImpl.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/ClassfileImpl.java @@ -43,7 +43,7 @@ import jdk.internal.classfile.constantpool.Utf8Entry; public record ClassfileImpl(StackMapsOption stackMapsOption, DebugElementsOption debugElementsOption, LineNumbersOption lineNumbersOption, - UnknownAttributesOption unknownAttributesOption, + AttributesProcessingOption attributesProcessingOption, ConstantPoolSharingOption constantPoolSharingOption, ShortJumpsOption shortJumpsOption, DeadCodeOption deadCodeOption, @@ -55,7 +55,7 @@ public record ClassfileImpl(StackMapsOption stackMapsOption, StackMapsOption.STACK_MAPS_WHEN_REQUIRED, DebugElementsOption.PASS_DEBUG, LineNumbersOption.PASS_LINE_NUMBERS, - UnknownAttributesOption.PASS_UNKNOWN_ATTRIBUTES, + AttributesProcessingOption.PASS_ALL_ATTRIBUTES, ConstantPoolSharingOption.SHARED_POOL, ShortJumpsOption.FIX_SHORT_JUMPS, DeadCodeOption.PATCH_DEAD_CODE, @@ -74,7 +74,7 @@ public record ClassfileImpl(StackMapsOption stackMapsOption, var smo = stackMapsOption; var deo = debugElementsOption; var lno = lineNumbersOption; - var uao = unknownAttributesOption; + var apo = attributesProcessingOption; var cpso = constantPoolSharingOption; var sjo = shortJumpsOption; var dco = deadCodeOption; @@ -86,7 +86,7 @@ public record ClassfileImpl(StackMapsOption stackMapsOption, case StackMapsOption oo -> smo = oo; case DebugElementsOption oo -> deo = oo; case LineNumbersOption oo -> lno = oo; - case UnknownAttributesOption oo -> uao = oo; + case AttributesProcessingOption oo -> apo = oo; case ConstantPoolSharingOption oo -> cpso = oo; case ShortJumpsOption oo -> sjo = oo; case DeadCodeOption oo -> dco = oo; @@ -95,7 +95,7 @@ public record ClassfileImpl(StackMapsOption stackMapsOption, case AttributeMapperOption oo -> amo = oo; } } - return new ClassfileImpl(smo, deo, lno, uao, cpso, sjo, dco, dlo, chro, amo); + return new ClassfileImpl(smo, deo, lno, apo, cpso, sjo, dco, dlo, chro, amo); } @Override diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/Util.java b/src/java.base/share/classes/jdk/internal/classfile/impl/Util.java index b757749b498..cd7a00b547e 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/Util.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/Util.java @@ -27,9 +27,7 @@ package jdk.internal.classfile.impl; import java.lang.constant.ClassDesc; import java.lang.constant.MethodTypeDesc; import java.util.AbstractList; -import java.util.BitSet; import java.util.Collection; -import java.util.Iterator; import java.util.List; import java.util.function.Function; @@ -38,11 +36,13 @@ import jdk.internal.classfile.constantpool.ClassEntry; import jdk.internal.classfile.constantpool.ModuleEntry; import jdk.internal.classfile.constantpool.NameAndTypeEntry; import java.lang.constant.ModuleDesc; -import jdk.internal.classfile.impl.TemporaryConstantPool; import java.lang.reflect.AccessFlag; import static jdk.internal.classfile.Classfile.ACC_STATIC; import jdk.internal.access.SharedSecrets; +import jdk.internal.classfile.Attribute; +import jdk.internal.classfile.AttributeMapper; +import jdk.internal.classfile.Classfile; /** * Helper to create and manipulate type descriptors, where type descriptors are @@ -54,6 +54,15 @@ public class Util { private Util() { } + private static final int ATTRIBUTE_STABILITY_COUNT = AttributeMapper.AttributeStability.values().length; + + public static boolean isAttributeAllowed(final Attribute attr, + final Classfile.AttributesProcessingOption processingOption) { + return attr instanceof BoundAttribute + ? ATTRIBUTE_STABILITY_COUNT - attr.attributeMapper().stability().ordinal() > processingOption.ordinal() + : true; + } + public static int parameterSlots(MethodTypeDesc mDesc) { int count = 0; for (int i = 0; i < mDesc.parameterCount(); i++) { @@ -121,7 +130,7 @@ public class Util { for (int i = 0; i < result.length; i++) { result[i] = TemporaryConstantPool.INSTANCE.classEntry(list.get(i)); } - return SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArrayNullsAllowed(result); + return SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArray(result); } public static List moduleEntryList(List list) { @@ -129,7 +138,7 @@ public class Util { for (int i = 0; i < result.length; i++) { result[i] = TemporaryConstantPool.INSTANCE.moduleEntry(TemporaryConstantPool.INSTANCE.utf8Entry(list.get(i).name())); } - return SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArrayNullsAllowed(result); + return SharedSecrets.getJavaUtilCollectionAccess().listFromTrustedArray(result); } public static void checkKind(Opcode op, Opcode.Kind k) { diff --git a/test/jdk/jdk/classfile/OptionsTest.java b/test/jdk/jdk/classfile/OptionsTest.java new file mode 100644 index 00000000000..12d3ccf7570 --- /dev/null +++ b/test/jdk/jdk/classfile/OptionsTest.java @@ -0,0 +1,133 @@ +/* + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @summary Testing Classfile options on small Corpus. + * @run junit/othervm -Djunit.jupiter.execution.parallel.enabled=true OptionsTest + */ +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.parallel.Execution; +import org.junit.jupiter.api.parallel.ExecutionMode; +import static org.junit.jupiter.api.Assertions.*; + + +import java.io.IOException; +import java.lang.constant.ClassDesc; +import java.net.URI; +import java.net.URISyntaxException; +import java.nio.file.FileSystem; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.Path; +import jdk.internal.classfile.AttributeMapper; +import jdk.internal.classfile.AttributedElement; +import jdk.internal.classfile.BufWriter; +import jdk.internal.classfile.ClassReader; +import jdk.internal.classfile.ClassTransform; +import jdk.internal.classfile.Classfile; +import jdk.internal.classfile.ClassfileElement; +import jdk.internal.classfile.CodeTransform; +import jdk.internal.classfile.CompoundElement; +import jdk.internal.classfile.CustomAttribute; + +/** + * OptionsTest + */ +@Execution(ExecutionMode.CONCURRENT) +class OptionsTest { + + protected static final FileSystem JRT = FileSystems.getFileSystem(URI.create("jrt:/")); + + static Path[] corpus() throws IOException, URISyntaxException { + return Files.walk(JRT.getPath("modules/java.base/java/util")) + .filter(p -> Files.isRegularFile(p) && p.toString().endsWith(".class")) + .toArray(Path[]::new); + } + + @ParameterizedTest + @MethodSource("corpus") + void testAttributesProcessingOptionOnTransform(Path path) throws Exception { + testNoUnstable(path, Classfile.of().parse( + Classfile.of(Classfile.AttributesProcessingOption.DROP_UNSTABLE_ATRIBUTES).transform( + Classfile.of().parse(path), + ClassTransform.transformingMethodBodies(CodeTransform.ACCEPT_ALL)))); + } + + static class StrangeAttribute extends CustomAttribute { + public StrangeAttribute() { + super(STRANGE_ATTRIBUTE_MAPPER); + } + } + + static final AttributeMapper STRANGE_ATTRIBUTE_MAPPER = new AttributeMapper<>() { + + @Override + public String name() { + return "StrangeAttribute"; + } + + @Override + public StrangeAttribute readAttribute(AttributedElement enclosing, ClassReader cf, int pos) { + return new StrangeAttribute(); + } + + @Override + public void writeAttribute(BufWriter buf, StrangeAttribute attr) { + buf.writeIndex(buf.constantPool().utf8Entry(name())); + buf.writeInt(0); + } + + @Override + public AttributeMapper.AttributeStability stability() { + return AttributeMapper.AttributeStability.STATELESS; + } + }; + + @Test + void testUnknownAttribute() throws Exception { + var classBytes = Classfile.of(Classfile.AttributeMapperOption.of(e -> { + return e.equalsString(STRANGE_ATTRIBUTE_MAPPER.name()) ? STRANGE_ATTRIBUTE_MAPPER : null; + })).build(ClassDesc.of("StrangeClass"), clb -> clb.with(new StrangeAttribute())); + + //test default + assertFalse(Classfile.of().parse(classBytes).attributes().isEmpty()); + + //test drop unknown at transform + assertTrue(Classfile.of().parse( + Classfile.of(Classfile.AttributesProcessingOption.DROP_UNKNOWN_ATTRIBUTES).transform( + Classfile.of().parse(classBytes), + ClassTransform.ACCEPT_ALL)).attributes().isEmpty()); + } + + void testNoUnstable(Path path, ClassfileElement e) { + if (e instanceof AttributedElement ae) ae.attributes().forEach(a -> + assertTrue(AttributeMapper.AttributeStability.UNSTABLE.ordinal() >= a.attributeMapper().stability().ordinal(), + () -> "class " + path + " contains unexpected " + a)); + if (e instanceof CompoundElement ce) ce.forEachElement(ee -> testNoUnstable(path, (ClassfileElement)ee)); + } +}