From 127989fd8e6396e06535c5ae92f15ba133843a14 Mon Sep 17 00:00:00 2001 From: Filip Wedzicha Date: Thu, 18 Jan 2024 14:05:47 +0000 Subject: [PATCH 1/5] perf: 2x speed up reading contents from zip This specific optimistaions is targeted towards JVM Class resources, in is isAsmCompliantClass we would read the class and check for custom attributes, then we would read the class again to build the class info in this pr I merged the functionality of isASMCompliantClass into ClassBuilderAppender --- .../info/builder/JvmClassInfoBuilder.java | 74 +++++++++++- .../CustomAttributeCollectingVisitor.java | 111 ------------------ .../recaf/workspace/io/BasicInfoImporter.java | 59 +++------- 3 files changed, 88 insertions(+), 156 deletions(-) delete mode 100644 recaf-core/src/main/java/software/coley/recaf/util/visitors/CustomAttributeCollectingVisitor.java diff --git a/recaf-core/src/main/java/software/coley/recaf/info/builder/JvmClassInfoBuilder.java b/recaf-core/src/main/java/software/coley/recaf/info/builder/JvmClassInfoBuilder.java index 0d5e97f2e..fd6938a09 100644 --- a/recaf-core/src/main/java/software/coley/recaf/info/builder/JvmClassInfoBuilder.java +++ b/recaf-core/src/main/java/software/coley/recaf/info/builder/JvmClassInfoBuilder.java @@ -8,6 +8,7 @@ import software.coley.recaf.info.JvmClassInfo; import software.coley.recaf.info.annotation.*; import software.coley.recaf.info.member.*; +import software.coley.recaf.util.MultiMap; import java.util.*; import java.util.function.Consumer; @@ -22,6 +23,8 @@ public class JvmClassInfoBuilder extends AbstractClassInfoBuilder { private byte[] bytecode; private int version = JvmClassInfo.BASE_VERSION + 8; // Java 8 + private boolean skipASMValidation; + private ClassBuilderAppender classVisitor; /** * Create empty builder. @@ -85,11 +88,13 @@ public JvmClassInfoBuilder adaptFrom(@Nonnull ClassReader reader) { * Reader flags to use when populating information. * * @return Builder. + * @throws IllegalStateException if using asm validation and the class has custom attributes */ @Nonnull @SuppressWarnings(value = "deprecation") public JvmClassInfoBuilder adaptFrom(@Nonnull ClassReader reader, int flags) { - reader.accept(new ClassBuilderAppender(), flags); + this.classVisitor = new ClassBuilderAppender(); + reader.accept(classVisitor, flags); return withBytecode(reader.b); } @@ -105,6 +110,12 @@ public JvmClassInfoBuilder withVersion(int version) { return this; } + @Nonnull + public JvmClassInfoBuilder skipASMValidation(boolean skipASMValidation) { + this.skipASMValidation = skipASMValidation; + return this; + } + public byte[] getBytecode() { return bytecode; } @@ -126,6 +137,10 @@ protected void verify() { throw new IllegalStateException("Bytecode required"); if (version < JvmClassInfo.BASE_VERSION) throw new IllegalStateException("Version cannot be lower than 44 (v1)"); + if (!this.skipASMValidation && classVisitor.hasCustomAttributes()) { + throw new IllegalStateException("Unknown attributes found in class: " + this.getName() + "[" + + String.join(", ", classVisitor.getCustomAttributeNames()) + "]"); + } } private class ClassBuilderAppender extends ClassVisitor { @@ -134,9 +149,20 @@ private class ClassBuilderAppender extends ClassVisitor { private final List innerClasses = new ArrayList<>(); private final List fields = new ArrayList<>(); private final List methods = new ArrayList<>(); + /** + * Collects custom attributes in a class. + */ + private final List classCustomAttributes = new ArrayList<>(); + private final MultiMap> fieldCustomAttributes; + private final MultiMap> methodCustomAttributes; + protected ClassBuilderAppender() { super(getAsmVersion()); + fieldCustomAttributes + = MultiMap.from(new HashMap<>(), ArrayList::new); + methodCustomAttributes + = MultiMap.from(new HashMap<>(), ArrayList::new); } @Override @@ -200,9 +226,22 @@ public void visitInnerClass(String name, String outerName, String innerName, int } + @Override + public void visitAttribute(Attribute attribute) { + classCustomAttributes.add(attribute); + super.visitAttribute(attribute); + } + @Override public FieldVisitor visitField(int access, String name, String descriptor, String signature, Object value) { return new FieldBuilderAppender(access, name, descriptor, signature, value) { + + @Override + public void visitAttribute(Attribute attribute) { + fieldCustomAttributes.get(name).add(attribute); + super.visitAttribute(attribute); + } + @Override public void visitEnd() { fields.add(getFieldMember()); @@ -213,6 +252,13 @@ public void visitEnd() { @Override public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) { return new MethodBuilderAppender(access, name, descriptor, signature, exceptions) { + + @Override + public void visitAttribute(Attribute attribute) { + methodCustomAttributes.get(name).add(attribute); + super.visitAttribute(attribute); + } + @Override public void visitEnd() { methods.add(getMethodMember()); @@ -232,6 +278,32 @@ public void visitEnd() { withAnnotations(annotations); withTypeAnnotations(typeAnnotations); } + + /** + * @return {@code true} when any custom attributes were found. + */ + public boolean hasCustomAttributes() { + return !classCustomAttributes.isEmpty() || + !fieldCustomAttributes.isEmpty() || + !methodCustomAttributes.isEmpty(); + } + + /** + * @return Unique names of attributes found. + */ + public Collection getCustomAttributeNames() { + Set names = new TreeSet<>(); + classCustomAttributes.stream() + .map(a -> a.type) + .forEach(names::add); + fieldCustomAttributes.values() + .map(a -> a.type) + .forEach(names::add); + methodCustomAttributes.values() + .map(a -> a.type) + .forEach(names::add); + return names; + } } private static class FieldBuilderAppender extends FieldVisitor { diff --git a/recaf-core/src/main/java/software/coley/recaf/util/visitors/CustomAttributeCollectingVisitor.java b/recaf-core/src/main/java/software/coley/recaf/util/visitors/CustomAttributeCollectingVisitor.java deleted file mode 100644 index 08bae89c6..000000000 --- a/recaf-core/src/main/java/software/coley/recaf/util/visitors/CustomAttributeCollectingVisitor.java +++ /dev/null @@ -1,111 +0,0 @@ -package software.coley.recaf.util.visitors; - -import jakarta.annotation.Nullable; -import org.objectweb.asm.Attribute; -import org.objectweb.asm.ClassVisitor; -import org.objectweb.asm.FieldVisitor; -import org.objectweb.asm.MethodVisitor; -import software.coley.recaf.RecafConstants; -import software.coley.recaf.util.MultiMap; - -import java.util.*; - -/** - * Collects custom attributes in a class. - * - * @author Matt Coley - */ -public class CustomAttributeCollectingVisitor extends ClassVisitor { - private final List classCustomAttributes = new ArrayList<>(); - private final MultiMap> fieldCustomAttributes; - private final MultiMap> methodCustomAttributes; - - /** - * @param cv - * Parent visitor. - */ - public CustomAttributeCollectingVisitor(@Nullable ClassVisitor cv) { - super(RecafConstants.getAsmVersion(), cv); - fieldCustomAttributes - = MultiMap.from(new HashMap<>(), ArrayList::new); - methodCustomAttributes - = MultiMap.from(new HashMap<>(), ArrayList::new); - } - - /** - * @return {@code true} when any custom attributes were found. - */ - public boolean hasCustomAttributes() { - return classCustomAttributes.size() > 0 || - fieldCustomAttributes.size() > 0 || - methodCustomAttributes.size() > 0; - } - - /** - * @return Unique names of attributes found. - */ - public Collection getCustomAttributeNames() { - Set names = new TreeSet<>(); - classCustomAttributes.stream() - .map(a -> a.type) - .forEach(names::add); - fieldCustomAttributes.values() - .map(a -> a.type) - .forEach(names::add); - methodCustomAttributes.values() - .map(a -> a.type) - .forEach(names::add); - return names; - } - - /** - * @return Class level custom attributes. - */ - public List getClassCustomAttributes() { - return classCustomAttributes; - } - - /** - * @return Field level custom attributes. Keys are field names. - */ - public MultiMap> getFieldCustomAttributes() { - return fieldCustomAttributes; - } - - /** - * @return Method level custom attributes. Keys are method names. - */ - public MultiMap> getMethodCustomAttributes() { - return methodCustomAttributes; - } - - @Override - public void visitAttribute(Attribute attribute) { - classCustomAttributes.add(attribute); - super.visitAttribute(attribute); - } - - @Override - public FieldVisitor visitField(int access, String name, String descriptor, String signature, Object value) { - FieldVisitor fv = super.visitField(access, name, descriptor, signature, value); - return new FieldVisitor(RecafConstants.getAsmVersion(), fv) { - @Override - public void visitAttribute(Attribute attribute) { - fieldCustomAttributes.get(name).add(attribute); - super.visitAttribute(attribute); - } - }; - } - - @Override - public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) { - MethodVisitor mv = super.visitMethod(access, name, descriptor, signature, exceptions); - return new MethodVisitor(RecafConstants.getAsmVersion(), mv) { - @Override - public void visitAttribute(Attribute attribute) { - methodCustomAttributes.get(name).add(attribute); - super.visitAttribute(attribute); - } - }; - } -} diff --git a/recaf-core/src/main/java/software/coley/recaf/workspace/io/BasicInfoImporter.java b/recaf-core/src/main/java/software/coley/recaf/workspace/io/BasicInfoImporter.java index 90cdddcdf..de65e22b7 100644 --- a/recaf-core/src/main/java/software/coley/recaf/workspace/io/BasicInfoImporter.java +++ b/recaf-core/src/main/java/software/coley/recaf/workspace/io/BasicInfoImporter.java @@ -15,7 +15,6 @@ import software.coley.recaf.util.IOUtil; import software.coley.recaf.util.android.AndroidXmlUtil; import software.coley.recaf.util.io.ByteSource; -import software.coley.recaf.util.visitors.CustomAttributeCollectingVisitor; import java.io.IOException; @@ -44,26 +43,26 @@ public Info readInfo(@Nonnull String name, @Nonnull ByteSource source) throws IO // Check for Java classes if (matchesClass(data)) { try { - // Patch if not compatible with ASM - if (!isAsmCompliantClass(data)) { + try { + return new JvmClassInfoBuilder(data) + .skipASMValidation(config.doSkipAsmValidation()) + .build(); + } catch (Throwable t) { + // Patch if not compatible with ASM byte[] patched = classPatcher.patch(name, data); - - // Ensure the patch was successful - if (!isAsmCompliantClass(patched)) { + logger.debug("CafeDude patched class: {}", name); + try { + return new JvmClassInfoBuilder(patched) + .skipASMValidation(config.doSkipAsmValidation()) + .build(); + } catch (Throwable t1) { logger.error("CafeDude patching output is still non-compliant with ASM for file: {}", name); return new FileInfoBuilder<>() - .withRawContent(data) - .withName(name) - .build(); - } else { - logger.debug("CafeDude patched class: {}", name); - return new JvmClassInfoBuilder(patched) - .build(); + .withRawContent(data) + .withName(name) + .build(); } } - - // Yield class - return new JvmClassInfoBuilder(data).build(); } catch (Throwable t) { // Invalid class, either some new edge case we need to add to CafeDude, or the file // isn't a class, but the structure models it close enough to look like one at a glance. @@ -181,34 +180,6 @@ private static boolean matchesClass(byte[] content) { return cpSize >= 4; } - /** - * Check if the class can be parsed by ASM. - * - * @param content - * The class file content. - * - * @return {@code true} if ASM can parse the class. - */ - private boolean isAsmCompliantClass(byte[] content) { - // Skip validation if configured. - if (config.doSkipAsmValidation()) - return true; - - // ASM should be able to write back the read class. - try { - CustomAttributeCollectingVisitor visitor = new CustomAttributeCollectingVisitor(new ClassWriter(0)); - ClassReader reader = new ClassReader(content); - reader.accept(visitor, 0); - if (visitor.hasCustomAttributes()) { - throw new IllegalStateException("Unknown attributes found in class: " + reader.getClassName() + "[" + - String.join(", ", visitor.getCustomAttributeNames()) + "]"); - } - return true; - } catch (Throwable t) { - return false; - } - } - @Nonnull @Override public String getServiceId() { From ccc617dcc384ab8e5f72e107c01df4c970daefac Mon Sep 17 00:00:00 2001 From: Filip Wedzicha Date: Thu, 18 Jan 2024 14:29:14 +0000 Subject: [PATCH 2/5] fix: class visitor can be null woops --- .../coley/recaf/info/builder/JvmClassInfoBuilder.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/recaf-core/src/main/java/software/coley/recaf/info/builder/JvmClassInfoBuilder.java b/recaf-core/src/main/java/software/coley/recaf/info/builder/JvmClassInfoBuilder.java index fd6938a09..485901f94 100644 --- a/recaf-core/src/main/java/software/coley/recaf/info/builder/JvmClassInfoBuilder.java +++ b/recaf-core/src/main/java/software/coley/recaf/info/builder/JvmClassInfoBuilder.java @@ -1,6 +1,7 @@ package software.coley.recaf.info.builder; import jakarta.annotation.Nonnull; +import jakarta.annotation.Nullable; import org.objectweb.asm.*; import software.coley.recaf.info.BasicInnerClassInfo; import software.coley.recaf.info.BasicJvmClassInfo; @@ -24,6 +25,7 @@ public class JvmClassInfoBuilder extends AbstractClassInfoBuilder Date: Fri, 19 Jan 2024 01:13:25 +0000 Subject: [PATCH 3/5] fix: check custom attributes when reading info Changed flag to skipCustomAttributeChecks and made it true by default since we should only be checking this from InfoImporter, we should be ignoring for jvm bundles for example. Also added back the skipASMValidation check to make sure parity is the same --- .../recaf/info/builder/JvmClassInfoBuilder.java | 15 ++++++++++----- .../recaf/workspace/io/BasicInfoImporter.java | 10 ++++++---- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/recaf-core/src/main/java/software/coley/recaf/info/builder/JvmClassInfoBuilder.java b/recaf-core/src/main/java/software/coley/recaf/info/builder/JvmClassInfoBuilder.java index 485901f94..35e992362 100644 --- a/recaf-core/src/main/java/software/coley/recaf/info/builder/JvmClassInfoBuilder.java +++ b/recaf-core/src/main/java/software/coley/recaf/info/builder/JvmClassInfoBuilder.java @@ -24,7 +24,7 @@ public class JvmClassInfoBuilder extends AbstractClassInfoBuilder { private byte[] bytecode; private int version = JvmClassInfo.BASE_VERSION + 8; // Java 8 - private boolean skipASMValidation; + private boolean skipCustomAttributeChecks = true; @Nullable private ClassBuilderAppender classVisitor; @@ -90,7 +90,6 @@ public JvmClassInfoBuilder adaptFrom(@Nonnull ClassReader reader) { * Reader flags to use when populating information. * * @return Builder. - * @throws IllegalStateException if using asm validation and the class has custom attributes */ @Nonnull @SuppressWarnings(value = "deprecation") @@ -112,9 +111,15 @@ public JvmClassInfoBuilder withVersion(int version) { return this; } + /** + * The default value is true. When {@code verify} is run it will check if there are any custom attributes. + * + * @param skipCustomAttributeChecks {@code false} if we want to verify the classes custom attribute + * @return {@code JvmClassInfoBuilder} + */ @Nonnull - public JvmClassInfoBuilder skipASMValidation(boolean skipASMValidation) { - this.skipASMValidation = skipASMValidation; + public JvmClassInfoBuilder skipCustomAttributeChecks(boolean skipCustomAttributeChecks) { + this.skipCustomAttributeChecks = skipCustomAttributeChecks; return this; } @@ -139,7 +144,7 @@ protected void verify() { throw new IllegalStateException("Bytecode required"); if (version < JvmClassInfo.BASE_VERSION) throw new IllegalStateException("Version cannot be lower than 44 (v1)"); - if (!this.skipASMValidation && classVisitor != null && classVisitor.hasCustomAttributes()) { + if (!this.skipCustomAttributeChecks && classVisitor != null && classVisitor.hasCustomAttributes()) { throw new IllegalStateException("Unknown attributes found in class: " + this.getName() + "[" + String.join(", ", classVisitor.getCustomAttributeNames()) + "]"); } diff --git a/recaf-core/src/main/java/software/coley/recaf/workspace/io/BasicInfoImporter.java b/recaf-core/src/main/java/software/coley/recaf/workspace/io/BasicInfoImporter.java index de65e22b7..b33b0131d 100644 --- a/recaf-core/src/main/java/software/coley/recaf/workspace/io/BasicInfoImporter.java +++ b/recaf-core/src/main/java/software/coley/recaf/workspace/io/BasicInfoImporter.java @@ -3,8 +3,6 @@ import jakarta.annotation.Nonnull; import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; -import org.objectweb.asm.ClassReader; -import org.objectweb.asm.ClassWriter; import org.slf4j.Logger; import software.coley.cafedude.classfile.VersionConstants; import software.coley.recaf.analytics.logging.Logging; @@ -43,9 +41,13 @@ public Info readInfo(@Nonnull String name, @Nonnull ByteSource source) throws IO // Check for Java classes if (matchesClass(data)) { try { + if (config.doSkipAsmValidation()) { + return new JvmClassInfoBuilder(data).build(); + } + try { return new JvmClassInfoBuilder(data) - .skipASMValidation(config.doSkipAsmValidation()) + .skipCustomAttributeChecks(false) .build(); } catch (Throwable t) { // Patch if not compatible with ASM @@ -53,7 +55,7 @@ public Info readInfo(@Nonnull String name, @Nonnull ByteSource source) throws IO logger.debug("CafeDude patched class: {}", name); try { return new JvmClassInfoBuilder(patched) - .skipASMValidation(config.doSkipAsmValidation()) + .skipCustomAttributeChecks(false) .build(); } catch (Throwable t1) { logger.error("CafeDude patching output is still non-compliant with ASM for file: {}", name); From ea7c56d7ba81934024ea66e047d2aeddf41488b8 Mon Sep 17 00:00:00 2001 From: Matt Date: Fri, 19 Jan 2024 06:50:05 -0500 Subject: [PATCH 4/5] Add back ClassWriter to validation checks --- .../info/builder/JvmClassInfoBuilder.java | 132 +++++++++++------- .../recaf/workspace/io/BasicInfoImporter.java | 16 ++- 2 files changed, 91 insertions(+), 57 deletions(-) diff --git a/recaf-core/src/main/java/software/coley/recaf/info/builder/JvmClassInfoBuilder.java b/recaf-core/src/main/java/software/coley/recaf/info/builder/JvmClassInfoBuilder.java index 35e992362..6bc8d6acf 100644 --- a/recaf-core/src/main/java/software/coley/recaf/info/builder/JvmClassInfoBuilder.java +++ b/recaf-core/src/main/java/software/coley/recaf/info/builder/JvmClassInfoBuilder.java @@ -24,9 +24,9 @@ public class JvmClassInfoBuilder extends AbstractClassInfoBuilder { private byte[] bytecode; private int version = JvmClassInfo.BASE_VERSION + 8; // Java 8 - private boolean skipCustomAttributeChecks = true; + private boolean skipValidationChecks = true; @Nullable - private ClassBuilderAppender classVisitor; + private ClassBuilderAdapter adapter; /** * Create empty builder. @@ -70,6 +70,18 @@ public JvmClassInfoBuilder(@Nonnull byte[] bytecode) { /** * Copies over values by reading the contents of the class file in the reader. * Calls {@link #adaptFrom(ClassReader, int)} with {@code flags=0}. + *

+ * IMPORTANT: If {@link #skipValidationChecks(boolean)} is {@code false} and validation checks are active + * extra steps are taken to ensure the class is fully ASM compliant. You will want to wrap this call in a try-catch + * block handling {@link Throwable} to cover any potential ASM failure. + *
+ * Validation is disabled by default. + *
+ * If you wish to validate the input, you must use the one of the given constructors: + *

    + *
  • {@link #JvmClassInfoBuilder()}
    • + *
  • {@link #JvmClassInfoBuilder(JvmClassInfo)}
    • + *
* * @param reader * ASM class reader to pull data from. @@ -83,6 +95,18 @@ public JvmClassInfoBuilder adaptFrom(@Nonnull ClassReader reader) { /** * Copies over values by reading the contents of the class file in the reader. + *

+ * IMPORTANT: If {@link #skipValidationChecks(boolean)} is {@code false} and validation checks are active + * extra steps are taken to ensure the class is fully ASM compliant. You will want to wrap this call in a try-catch + * block handling {@link Throwable} to cover any potential ASM failure. + *
+ * Validation is disabled by default. + *
+ * If you wish to validate the input, you must use the one of the given constructors: + *

    + *
  • {@link #JvmClassInfoBuilder()}
    • + *
  • {@link #JvmClassInfoBuilder(JvmClassInfo)}
    • + *
* * @param reader * ASM class reader to pull data from. @@ -94,8 +118,11 @@ public JvmClassInfoBuilder adaptFrom(@Nonnull ClassReader reader) { @Nonnull @SuppressWarnings(value = "deprecation") public JvmClassInfoBuilder adaptFrom(@Nonnull ClassReader reader, int flags) { - this.classVisitor = new ClassBuilderAppender(); - reader.accept(classVisitor, flags); + // If we are doing validation checks, delegating the reader to a writer should catch most issues + // that would normally crash ASM. It is the caller's responsibility to error handle ASM failing + // if such failures occur. + adapter = new ClassBuilderAdapter(skipValidationChecks ? null : new ClassWriter(reader, 0)); + reader.accept(adapter, flags); return withBytecode(reader.b); } @@ -112,14 +139,17 @@ public JvmClassInfoBuilder withVersion(int version) { } /** - * The default value is true. When {@code verify} is run it will check if there are any custom attributes. + * The default value is {@code true}. Setting to {@code false} enables class validation steps. + * When {@link #verify()} is run it will check if there are any custom attributes. + * + * @param skipValidationChecks + * {@code false} if we want to verify the classes custom attribute * - * @param skipCustomAttributeChecks {@code false} if we want to verify the classes custom attribute * @return {@code JvmClassInfoBuilder} */ @Nonnull - public JvmClassInfoBuilder skipCustomAttributeChecks(boolean skipCustomAttributeChecks) { - this.skipCustomAttributeChecks = skipCustomAttributeChecks; + public JvmClassInfoBuilder skipValidationChecks(boolean skipValidationChecks) { + this.skipValidationChecks = skipValidationChecks; return this; } @@ -144,32 +174,33 @@ protected void verify() { throw new IllegalStateException("Bytecode required"); if (version < JvmClassInfo.BASE_VERSION) throw new IllegalStateException("Version cannot be lower than 44 (v1)"); - if (!this.skipCustomAttributeChecks && classVisitor != null && classVisitor.hasCustomAttributes()) { + if (!skipValidationChecks && adapter != null && adapter.hasCustomAttributes()) { throw new IllegalStateException("Unknown attributes found in class: " + this.getName() + "[" + - String.join(", ", classVisitor.getCustomAttributeNames()) + "]"); + String.join(", ", adapter.getCustomAttributeNames()) + "]"); } } - private class ClassBuilderAppender extends ClassVisitor { + /** + * Converts ASM visitor actions to 'with' actions in the class builder. + * Results in a fully reconstructed class model. + * + * @see FieldBuilderAdapter + * @see MethodBuilderAdapter + */ + private class ClassBuilderAdapter extends ClassVisitor { private final List annotations = new ArrayList<>(); private final List typeAnnotations = new ArrayList<>(); private final List innerClasses = new ArrayList<>(); private final List fields = new ArrayList<>(); private final List methods = new ArrayList<>(); - /** - * Collects custom attributes in a class. - */ private final List classCustomAttributes = new ArrayList<>(); private final MultiMap> fieldCustomAttributes; private final MultiMap> methodCustomAttributes; - - protected ClassBuilderAppender() { - super(getAsmVersion()); - fieldCustomAttributes - = MultiMap.from(new HashMap<>(), ArrayList::new); - methodCustomAttributes - = MultiMap.from(new HashMap<>(), ArrayList::new); + protected ClassBuilderAdapter(@Nullable ClassVisitor cv) { + super(getAsmVersion(), cv); + fieldCustomAttributes = MultiMap.from(new HashMap<>(), ArrayList::new); + methodCustomAttributes = MultiMap.from(new HashMap<>(), ArrayList::new); } @Override @@ -199,12 +230,12 @@ public void visitOuterClass(String owner, String name, String descriptor) { @Override public AnnotationVisitor visitAnnotation(String descriptor, boolean visible) { - return new AnnotationBuilderAppender(visible, descriptor, annotations::add); + return new AnnotationBuilderAdapter(visible, descriptor, annotations::add); } @Override public AnnotationVisitor visitTypeAnnotation(int typeRef, TypePath typePath, String descriptor, boolean visible) { - return new AnnotationBuilderAppender(visible, descriptor, + return new AnnotationBuilderAdapter(visible, descriptor, anno -> typeAnnotations.add(anno.withTypeInfo(typeRef, typePath))); } @@ -230,7 +261,6 @@ public void visitInnerClass(String name, String outerName, String innerName, int withOuterClassName(outerName); } } - } @Override @@ -241,7 +271,7 @@ public void visitAttribute(Attribute attribute) { @Override public FieldVisitor visitField(int access, String name, String descriptor, String signature, Object value) { - return new FieldBuilderAppender(access, name, descriptor, signature, value) { + return new FieldBuilderAdapter(access, name, descriptor, signature, value) { @Override public void visitAttribute(Attribute attribute) { @@ -258,7 +288,7 @@ public void visitEnd() { @Override public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) { - return new MethodBuilderAppender(access, name, descriptor, signature, exceptions) { + return new MethodBuilderAdapter(access, name, descriptor, signature, exceptions) { @Override public void visitAttribute(Attribute attribute) { @@ -291,8 +321,8 @@ public void visitEnd() { */ public boolean hasCustomAttributes() { return !classCustomAttributes.isEmpty() || - !fieldCustomAttributes.isEmpty() || - !methodCustomAttributes.isEmpty(); + !fieldCustomAttributes.isEmpty() || + !methodCustomAttributes.isEmpty(); } /** @@ -301,35 +331,35 @@ public boolean hasCustomAttributes() { public Collection getCustomAttributeNames() { Set names = new TreeSet<>(); classCustomAttributes.stream() - .map(a -> a.type) - .forEach(names::add); + .map(a -> a.type) + .forEach(names::add); fieldCustomAttributes.values() - .map(a -> a.type) - .forEach(names::add); + .map(a -> a.type) + .forEach(names::add); methodCustomAttributes.values() - .map(a -> a.type) - .forEach(names::add); + .map(a -> a.type) + .forEach(names::add); return names; } } - private static class FieldBuilderAppender extends FieldVisitor { + private static class FieldBuilderAdapter extends FieldVisitor { private final BasicFieldMember fieldMember; - public FieldBuilderAppender(int access, String name, String descriptor, - String signature, Object value) { + public FieldBuilderAdapter(int access, String name, String descriptor, + String signature, Object value) { super(getAsmVersion()); fieldMember = new BasicFieldMember(name, descriptor, signature, access, value); } @Override public AnnotationVisitor visitAnnotation(String descriptor, boolean visible) { - return new AnnotationBuilderAppender(visible, descriptor, fieldMember::addAnnotation); + return new AnnotationBuilderAdapter(visible, descriptor, fieldMember::addAnnotation); } @Override public AnnotationVisitor visitTypeAnnotation(int typeRef, TypePath typePath, String descriptor, boolean visible) { - return new AnnotationBuilderAppender(visible, descriptor, + return new AnnotationBuilderAdapter(visible, descriptor, anno -> fieldMember.addTypeAnnotation(anno.withTypeInfo(typeRef, typePath))); } @@ -339,11 +369,11 @@ public BasicFieldMember getFieldMember() { } } - private static class MethodBuilderAppender extends MethodVisitor { + private static class MethodBuilderAdapter extends MethodVisitor { private final BasicMethodMember methodMember; - public MethodBuilderAppender(int access, String name, String descriptor, - String signature, String[] exceptions) { + public MethodBuilderAdapter(int access, String name, String descriptor, + String signature, String[] exceptions) { super(getAsmVersion()); List exceptionList = exceptions == null ? Collections.emptyList() : Arrays.asList(exceptions); methodMember = new BasicMethodMember(name, descriptor, signature, access, exceptionList, new ArrayList<>()); @@ -351,12 +381,12 @@ public MethodBuilderAppender(int access, String name, String descriptor, @Override public AnnotationVisitor visitAnnotation(String descriptor, boolean visible) { - return new AnnotationBuilderAppender(visible, descriptor, methodMember::addAnnotation); + return new AnnotationBuilderAdapter(visible, descriptor, methodMember::addAnnotation); } @Override public AnnotationVisitor visitTypeAnnotation(int typeRef, TypePath typePath, String descriptor, boolean visible) { - return new AnnotationBuilderAppender(visible, descriptor, + return new AnnotationBuilderAdapter(visible, descriptor, anno -> methodMember.addTypeAnnotation(anno.withTypeInfo(typeRef, typePath))); } @@ -372,15 +402,15 @@ public BasicMethodMember getMethodMember() { } } - private static class AnnotationBuilderAppender extends AnnotationVisitor { + private static class AnnotationBuilderAdapter extends AnnotationVisitor { private final Consumer annotationConsumer; private final Map elements = new HashMap<>(); private final List arrayValues = new ArrayList<>(); private final boolean visible; private final String descriptor; - protected AnnotationBuilderAppender(boolean visible, String descriptor, - Consumer annotationConsumer) { + protected AnnotationBuilderAdapter(boolean visible, String descriptor, + Consumer annotationConsumer) { super(getAsmVersion()); this.visible = visible; this.descriptor = descriptor; @@ -412,7 +442,7 @@ public void visitEnum(String name, String descriptor, String value) { @Override public AnnotationVisitor visitAnnotation(String name, String descriptor) { - return new AnnotationBuilderAppender(true, descriptor, anno -> { + return new AnnotationBuilderAdapter(true, descriptor, anno -> { if (name == null) { arrayValues.add(anno); } else { @@ -423,11 +453,11 @@ public AnnotationVisitor visitAnnotation(String name, String descriptor) { @Override public AnnotationVisitor visitArray(String name) { - AnnotationBuilderAppender outer = this; - return new AnnotationBuilderAppender(true, "", null) { + AnnotationBuilderAdapter outer = this; + return new AnnotationBuilderAdapter(true, "", null) { @Override public void visitEnd() { - AnnotationBuilderAppender inner = this; + AnnotationBuilderAdapter inner = this; outer.elements.put(name, new BasicAnnotationElement(name, inner.arrayValues)); } }; diff --git a/recaf-core/src/main/java/software/coley/recaf/workspace/io/BasicInfoImporter.java b/recaf-core/src/main/java/software/coley/recaf/workspace/io/BasicInfoImporter.java index b33b0131d..1ce946524 100644 --- a/recaf-core/src/main/java/software/coley/recaf/workspace/io/BasicInfoImporter.java +++ b/recaf-core/src/main/java/software/coley/recaf/workspace/io/BasicInfoImporter.java @@ -3,6 +3,7 @@ import jakarta.annotation.Nonnull; import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; +import org.objectweb.asm.ClassReader; import org.slf4j.Logger; import software.coley.cafedude.classfile.VersionConstants; import software.coley.recaf.analytics.logging.Logging; @@ -41,21 +42,24 @@ public Info readInfo(@Nonnull String name, @Nonnull ByteSource source) throws IO // Check for Java classes if (matchesClass(data)) { try { - if (config.doSkipAsmValidation()) { + // If we're skipping validation, any ASM parse failures will result in the class + // being treated as a file instead (see catch block) + if (config.doSkipAsmValidation()) return new JvmClassInfoBuilder(data).build(); - } + // If we are doing validation, disable skipping ASM checks. try { - return new JvmClassInfoBuilder(data) - .skipCustomAttributeChecks(false) - .build(); + return new JvmClassInfoBuilder() + .skipValidationChecks(false) + .adaptFrom(new ClassReader(data)) + .build(); } catch (Throwable t) { // Patch if not compatible with ASM byte[] patched = classPatcher.patch(name, data); logger.debug("CafeDude patched class: {}", name); try { return new JvmClassInfoBuilder(patched) - .skipCustomAttributeChecks(false) + .skipValidationChecks(false) .build(); } catch (Throwable t1) { logger.error("CafeDude patching output is still non-compliant with ASM for file: {}", name); From 9a8a032a820cb076d8a8963ff493ae76c27a8202 Mon Sep 17 00:00:00 2001 From: Matt Date: Fri, 19 Jan 2024 07:01:06 -0500 Subject: [PATCH 5/5] Create code-cov config which shouldn't fail PR's for negligible coverage changes --- codecov.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 codecov.yml diff --git a/codecov.yml b/codecov.yml new file mode 100644 index 000000000..1b215550e --- /dev/null +++ b/codecov.yml @@ -0,0 +1,10 @@ +coverage: + precision: 2 + round: down + status: + project: + default: + informational: true + patch: + default: + informational: true \ No newline at end of file