From b1772a723a0dff47315ee4ccec62a2b18c553395 Mon Sep 17 00:00:00 2001 From: Blend Hamiti Date: Fri, 2 Aug 2024 15:50:16 +0200 Subject: [PATCH 1/4] Print exceptions as warning when incomplete class hierarchies are allowed --- .../IncompleteClassHierarchyException.java | 4 ++++ .../evaluation/value/TypedReferenceValue.java | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/base/src/main/java/proguard/evaluation/exception/IncompleteClassHierarchyException.java b/base/src/main/java/proguard/evaluation/exception/IncompleteClassHierarchyException.java index e887ae8be..e3ffb4eed 100644 --- a/base/src/main/java/proguard/evaluation/exception/IncompleteClassHierarchyException.java +++ b/base/src/main/java/proguard/evaluation/exception/IncompleteClassHierarchyException.java @@ -18,6 +18,8 @@ package proguard.evaluation.exception; +import proguard.evaluation.value.TypedReferenceValue; + /** * Represents an exception during partial evaluation when an incomplete class hierarchy was * encountered. @@ -26,5 +28,7 @@ public class IncompleteClassHierarchyException extends proguard.evaluation.IncompleteClassHierarchyException { public IncompleteClassHierarchyException(String message) { super(message); + + TypedReferenceValue.INCOMPLETE_CLASS_HIERARCHY = true; } } diff --git a/base/src/main/java/proguard/evaluation/value/TypedReferenceValue.java b/base/src/main/java/proguard/evaluation/value/TypedReferenceValue.java index b7a5213c4..846215a79 100644 --- a/base/src/main/java/proguard/evaluation/value/TypedReferenceValue.java +++ b/base/src/main/java/proguard/evaluation/value/TypedReferenceValue.java @@ -27,6 +27,8 @@ import java.util.Iterator; import java.util.Objects; import java.util.Set; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import proguard.classfile.AccessConstants; import proguard.classfile.ClassConstants; import proguard.classfile.Clazz; @@ -48,10 +50,23 @@ */ public class TypedReferenceValue extends ReferenceValue { + /** Indicates whether there were occurrences of {@link IncompleteClassHierarchyException}. */ + public static boolean INCOMPLETE_CLASS_HIERARCHY = false; + + /** If true, do not throw an {@link IncompleteClassHierarchyException} when one would occur. */ public static boolean ALLOW_INCOMPLETE_CLASS_HIERARCHY = System.getProperty("allow.incomplete.class.hierarchy") != null; + + /** + * If true, print warnings for occurrences of {@link IncompleteClassHierarchyException} when + * ALLOW_INCOMPLETE_CLASS_HIERARCHY is enabled. + */ + public static boolean WARN_INCOMPLETE_CLASS_HIERARCHY = false; + private static final boolean DEBUG = false; + private static final Logger logger = LogManager.getLogger(TypedReferenceValue.class); + protected final String type; protected final Clazz referencedClass; protected final boolean mayBeExtension; @@ -300,6 +315,9 @@ public ReferenceValue generalize(TypedReferenceValue other) { } catch (IncompleteClassHierarchyException e) { // The class hierarchy seems to be incomplete. if (ALLOW_INCOMPLETE_CLASS_HIERARCHY) { + if (WARN_INCOMPLETE_CLASS_HIERARCHY) { + logger.warn("Warning: Incomplete class hierarchy: {}", e.getMessage()); + } // We'll return an unknown reference value. return BasicValueFactory.REFERENCE_VALUE; } From 5174a4bdefc0a6b490a4c530dd807dabf2288c9f Mon Sep 17 00:00:00 2001 From: James Hamilton Date: Fri, 9 Aug 2024 17:14:54 +0200 Subject: [PATCH 2/4] Update kotest version --- android/build.gradle | 10 +++++----- base/build.gradle | 16 ++++++++-------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/android/build.gradle b/android/build.gradle index 7a4cb59f7..5e801837f 100644 --- a/android/build.gradle +++ b/android/build.gradle @@ -21,11 +21,11 @@ dependencies { testImplementation "org.jetbrains.kotlin:kotlin-stdlib:${kotlinVersion}" testImplementation "org.jetbrains.kotlin:kotlin-reflect:${kotlinVersion}" - testImplementation 'io.kotest:kotest-runner-junit5-jvm:5.6.2' // for kotest framework - testImplementation 'io.kotest:kotest-assertions-core-jvm:5.6.2' // for kotest core jvm assertions - testImplementation 'io.kotest:kotest-property-jvm:5.6.2' // for kotest property test - testImplementation 'io.kotest:kotest-framework-datatest:5.6.2' - testImplementation 'io.mockk:mockk:1.13.5' // for mocking + testImplementation 'io.kotest:kotest-runner-junit5-jvm:5.7.2' // for kotest framework + testImplementation 'io.kotest:kotest-assertions-core-jvm:5.7.2' // for kotest core jvm assertions + testImplementation 'io.kotest:kotest-property-jvm:5.7.2' // for kotest property test + testImplementation 'io.kotest:kotest-framework-datatest:5.7.2' + testImplementation 'io.mockk:mockk:1.13.12' // for mocking testImplementation 'org.junit.jupiter:junit-jupiter-api:5.9.3' // for junit framework testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.9.3' // for junit framework diff --git a/base/build.gradle b/base/build.gradle index 1ecd2b754..74e32ab06 100644 --- a/base/build.gradle +++ b/base/build.gradle @@ -25,14 +25,14 @@ dependencies { testImplementation "org.jetbrains.kotlin:kotlin-reflect:${kotlinVersion}" testFixturesImplementation "org.jetbrains.kotlin:kotlin-reflect:${kotlinVersion}" testFixturesImplementation group: "dev.zacsweers.kctfork", name: "core", version: "0.5.0" - testFixturesImplementation 'io.kotest:kotest-runner-junit5-jvm:5.5.4' // for kotest framework - testImplementation 'io.kotest:kotest-runner-junit5-jvm:5.5.4' // for kotest framework - testImplementation 'io.kotest:kotest-assertions-core-jvm:5.5.4' // for kotest core jvm assertions - testFixturesImplementation 'io.kotest:kotest-assertions-core-jvm:5.5.4' // for kotest core jvm assertions - - testImplementation 'io.kotest:kotest-property-jvm:5.5.4' // for kotest property test - testImplementation 'io.kotest:kotest-framework-datatest:5.5.4' - testImplementation 'io.mockk:mockk:1.13.3' // for mocking + testFixturesImplementation 'io.kotest:kotest-runner-junit5-jvm:5.7.2' // for kotest framework + testImplementation 'io.kotest:kotest-runner-junit5-jvm:5.7.2' // for kotest framework + testImplementation 'io.kotest:kotest-assertions-core-jvm:5.7.2' // for kotest core jvm assertions + testFixturesImplementation 'io.kotest:kotest-assertions-core-jvm:5.7.2' // for kotest core jvm assertions + + testImplementation 'io.kotest:kotest-property-jvm:5.7.2' // for kotest property test + testImplementation 'io.kotest:kotest-framework-datatest:5.7.2' + testImplementation 'io.mockk:mockk:1.13.12' // for mocking testImplementation 'org.junit.jupiter:junit-jupiter-api:5.9.0' // for junit framework testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.9.0' // for junit framework From 55e2c85b1bc73fc958978298d5c3a7c4f389855f Mon Sep 17 00:00:00 2001 From: "niccolo.piazzesi" Date: Tue, 13 Aug 2024 11:05:53 +0200 Subject: [PATCH 3/4] Make skip_broken_debug_info configurable with a flag and not only a system property. --- .../main/java/proguard/dexfile/reader/DexFileReader.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/android/src/main/java/proguard/dexfile/reader/DexFileReader.java b/android/src/main/java/proguard/dexfile/reader/DexFileReader.java index 20cd64469..3c7cf7e31 100755 --- a/android/src/main/java/proguard/dexfile/reader/DexFileReader.java +++ b/android/src/main/java/proguard/dexfile/reader/DexFileReader.java @@ -80,6 +80,9 @@ public class DexFileReader implements BaseDexFileReader { /** keep clinit method when {@link #SKIP_DEBUG} */ public static final int SKIP_EXCEPTION = 1 << 8; + /** Do not throw an exception if debug info is not valid */ + public static final int SKIP_BROKEN_DEBUG_INFO = 1 << 9; + // private static final int REVERSE_ENDIAN_CONSTANT = 0x78563412; static final int DBG_END_SEQUENCE = 0x00; @@ -119,7 +122,7 @@ public class DexFileReader implements BaseDexFileReader { private static final int TYPE_CALL_SITE_ID_ITEM = 0x0007; private static final int TYPE_METHOD_HANDLE_ITEM = 0x0008; - private static final boolean SKIP_BROKEN_DEBUG_INFO = + private static final boolean SKIP_BROKEN_DEBUG_INFO_SYSTEM_PROPERTY = System.getProperty("dex2jar.skip_broken_debug_information") != null; final ByteBuffer annotationSetRefListIn; @@ -1507,6 +1510,8 @@ private void findTryCatch( } } // 处理debug信息 + boolean skipBrokenInfo = + SKIP_BROKEN_DEBUG_INFO_SYSTEM_PROPERTY || 0 != (config & SKIP_BROKEN_DEBUG_INFO); if (debug_info_off != 0 && (0 == (config & SKIP_DEBUG))) { DexDebugVisitor ddv = dcv.visitDebug(); if (ddv != null) { @@ -1515,7 +1520,7 @@ private void findTryCatch( debug_info_off, registers_size, isStatic, method, labelsMap, ddv, insnsArray.length); ddv.visitEnd(); } catch (Exception e) { - if (!SKIP_BROKEN_DEBUG_INFO) { + if (!skipBrokenInfo) { throw e; } } From 5893999f5661afa8aee8d8f5ce0957719c270061 Mon Sep 17 00:00:00 2001 From: Carlo Alberto Pozzoli Date: Fri, 19 Jul 2024 15:00:57 +0200 Subject: [PATCH 4/4] Use DetailedArrayReferenceValue consistently for particular arrays in ExecutingInvocationUnit Summary: This solves the problem of particular array values being possibly `ParticularReferenceValue` or `DetailedArrayReferenceValue` based on how they are created. Now the latter is used consistently, also when a method tracked by `ExecutingInvocationUnit` returns a array of references. Other inconsistencies of how array are handled are still present and not addressed on purpose, since that behavior is present deeply in the PE code. For example, `ExecutingInvocationUnit#createNonParticularValue` still invokes `ValueFactory#createReferenceValue` even if the argument is an array, instead of `createArrayReferenceValue`, resulting in having an `IdentifiedReferenceValue` instead of an `IdentifiedArrayReferenceValue`. This is because values created outside the invocation unit use methods from `BasicValueFactory` and not overriden by the more advanced value factories, which also never end up invoking `ValueFactory#createArrayReferenceValue`. Trying to change this behavior to have consistency would probably be a longer process, so I'm keeping the code as it is. NB: the updated behavior will result in `ArrayReferenceValue` being created if `ArrayReferenceValueFactory` is used instead of `DetailedArrayValueFactory` and only in the case an array is returned by an executor (since `createValue` actually invokes `createArrayReferenceValue`), this inconsistency does not happen if `DetailedArrayValueFactory` is used with `ExecutingInvocationUnit`. In general the fix is quite hacky because it relies on `DetailedArrayValueFactory` calling the `referenceValueFactory` from `ParticularValueFactory` directly, since it needs to bypass the behavior of `IdentifiedValueFactory` because of some improper inheritance. --- .../evaluation/ExecutingInvocationUnit.java | 10 +- .../value/ArrayReferenceValueFactory.java | 7 + .../value/DetailedArrayReferenceValue.java | 2 - .../value/DetailedArrayValueFactory.java | 142 +++++++++++++----- .../value/IdentifiedValueFactory.java | 28 +++- .../value/ParticularReferenceValue.java | 8 + .../value/ParticularValueFactory.java | 8 +- .../value/object/AnalyzedObjectFactory.java | 13 +- .../test/kotlin/proguard/DetailedArrayTest.kt | 4 +- .../proguard/ParticularReferenceTest.kt | 5 +- .../analysis/ExecutingInvocationUnitTest.kt | 50 +++++- .../analysis/PartialEvaluatorErrorsTest.kt | 4 +- .../analysis/SelectiveCallResolverTest.kt | 3 +- .../cpa/ExecutingInvocationUnitTest.kt | 2 +- docs/md/releasenotes.md | 4 + .../java/proguard/examples/EvaluateCode.java | 3 +- 16 files changed, 224 insertions(+), 69 deletions(-) diff --git a/base/src/main/java/proguard/evaluation/ExecutingInvocationUnit.java b/base/src/main/java/proguard/evaluation/ExecutingInvocationUnit.java index 8060091da..8f636672e 100644 --- a/base/src/main/java/proguard/evaluation/ExecutingInvocationUnit.java +++ b/base/src/main/java/proguard/evaluation/ExecutingInvocationUnit.java @@ -514,17 +514,15 @@ private MethodResult createFallbackResult(MethodExecutionInfo methodInfo) { } } - // TODO: update to handle array of references the same way as primitive arrays - if (ClassUtil.internalArrayTypeDimensionCount(type) == 1 - && isInternalPrimitiveType(ClassUtil.internalTypeFromArrayType(type)) - && concreteValue != null) { + if (ClassUtil.internalArrayTypeDimensionCount(type) == 1 && concreteValue != null) { if (concreteValue instanceof Model) { throw new IllegalStateException( "Modeled arrays are not supported by ExecutingInvocationUnit"); } return valueFactory.createArrayReferenceValue( - type, - null, // NB the class is null just because we are filtering for primitive arrays + // This method expects the type of the content of the array + type.substring(1), + referencedClass, // Might be null (for primitive arrays) valueFactory.createIntegerValue(Array.getLength(concreteValue)), concreteValue); } diff --git a/base/src/main/java/proguard/evaluation/value/ArrayReferenceValueFactory.java b/base/src/main/java/proguard/evaluation/value/ArrayReferenceValueFactory.java index 50c6307a0..c978c47bc 100644 --- a/base/src/main/java/proguard/evaluation/value/ArrayReferenceValueFactory.java +++ b/base/src/main/java/proguard/evaluation/value/ArrayReferenceValueFactory.java @@ -28,10 +28,17 @@ public class ArrayReferenceValueFactory extends TypedReferenceValueFactory { // Implementations for ReferenceValue. + @Override public ReferenceValue createArrayReferenceValue( String type, Clazz referencedClass, IntegerValue arrayLength) { return type == null ? REFERENCE_VALUE_NULL : new ArrayReferenceValue(TypeConstants.ARRAY + type, referencedClass, false, arrayLength); } + + @Override + public ReferenceValue createArrayReferenceValue( + String type, Clazz referencedClass, IntegerValue arrayLength, Object elementValues) { + return createArrayReferenceValue(type, referencedClass, arrayLength); + } } diff --git a/base/src/main/java/proguard/evaluation/value/DetailedArrayReferenceValue.java b/base/src/main/java/proguard/evaluation/value/DetailedArrayReferenceValue.java index 656c5345b..6fa1edb18 100644 --- a/base/src/main/java/proguard/evaluation/value/DetailedArrayReferenceValue.java +++ b/base/src/main/java/proguard/evaluation/value/DetailedArrayReferenceValue.java @@ -28,8 +28,6 @@ /** * This {@link IdentifiedArrayReferenceValue} represents an identified array reference value with * its elements. - * - * @author Eric Lafortune */ public class DetailedArrayReferenceValue extends IdentifiedArrayReferenceValue { private static final int MAXIMUM_STORED_ARRAY_LENGTH = 64; diff --git a/base/src/main/java/proguard/evaluation/value/DetailedArrayValueFactory.java b/base/src/main/java/proguard/evaluation/value/DetailedArrayValueFactory.java index 21667ec73..0602f927e 100644 --- a/base/src/main/java/proguard/evaluation/value/DetailedArrayValueFactory.java +++ b/base/src/main/java/proguard/evaluation/value/DetailedArrayValueFactory.java @@ -17,17 +17,37 @@ */ package proguard.evaluation.value; -import proguard.classfile.*; +import proguard.classfile.Clazz; +import proguard.classfile.TypeConstants; +import proguard.classfile.util.ClassUtil; +import proguard.evaluation.value.object.AnalyzedObject; +import proguard.evaluation.value.object.AnalyzedObjectFactory; /** * This identified value factory creates array reference values that also represent their elements, * in as far as possible. - * - * @author Eric Lafortune */ public class DetailedArrayValueFactory extends IdentifiedValueFactory { - // Implementations for ReferenceValue. + /** Creates a new DetailedArrayValueFactory, which does not track reference values. */ + @Deprecated + public DetailedArrayValueFactory() { + this(new TypedReferenceValueFactory()); + } + + /** + * Creates a new DetailedArrayValueFactory, which uses the given value factory for non-array + * reference construction. + */ + public DetailedArrayValueFactory(ValueFactory referenceValueFactory) { + // This is quite ugly, but unavoidable without refactoring the value factories hierarchy. Since + // what's currently going on is that DetailedArrayReferenceValue is a ParticularValueFactory + // that overrides all the methods where arrayReferenceValueFactory is used, even if we pass an + // ArrayReferenceValueFactory to super this will never be used. + super(new UnusableArrayValueFactory(), referenceValueFactory); + } + + @Override public ReferenceValue createArrayReferenceValue( String type, Clazz referencedClass, IntegerValue arrayLength) { return type == null @@ -49,14 +69,17 @@ public ReferenceValue createArrayReferenceValue( generateReferenceId()); } + @Override public ReferenceValue createArrayReferenceValue( String type, Clazz referencedClass, IntegerValue arrayLength, Object elementValues) { if (type == null) return TypedReferenceValueFactory.REFERENCE_VALUE_NULL; + String arrayType = TypeConstants.ARRAY + type; + if (!arrayLength.isParticular()) { return new IdentifiedArrayReferenceValue( - type, referencedClass, false, arrayLength, this, generateReferenceId()); + arrayType, referencedClass, false, arrayLength, this, generateReferenceId()); } if (!elementValues.getClass().isArray() || elementValues.getClass().getComponentType().isArray()) { @@ -66,38 +89,38 @@ public ReferenceValue createArrayReferenceValue( DetailedArrayReferenceValue detailedArray = new DetailedArrayReferenceValue( - type, referencedClass, false, arrayLength, this, generateReferenceId()); - if (elementValues.getClass().isArray()) { - switch (type.charAt(1)) // 0 is the array char - { - case TypeConstants.BOOLEAN: - storeBooleanArray(detailedArray, (boolean[]) elementValues); - break; - case TypeConstants.BYTE: - storeByteArray(detailedArray, (byte[]) elementValues); - break; - case TypeConstants.CHAR: - storeCharArray(detailedArray, (char[]) elementValues); - break; - case TypeConstants.SHORT: - storeShortArray(detailedArray, (short[]) elementValues); - break; - case TypeConstants.INT: - storeIntArray(detailedArray, (int[]) elementValues); - break; - case TypeConstants.LONG: - storeLongArray(detailedArray, (long[]) elementValues); - break; - case TypeConstants.FLOAT: - storeFloatArray(detailedArray, (float[]) elementValues); - break; - case TypeConstants.DOUBLE: - storeDoubleArray(detailedArray, (double[]) elementValues); - break; - default: - storeObjectArray(detailedArray, (Object[]) elementValues); - } + arrayType, referencedClass, false, arrayLength, this, generateReferenceId()); + + switch (arrayType.charAt(1)) // 0 is the array char + { + case TypeConstants.BOOLEAN: + storeBooleanArray(detailedArray, (boolean[]) elementValues); + break; + case TypeConstants.BYTE: + storeByteArray(detailedArray, (byte[]) elementValues); + break; + case TypeConstants.CHAR: + storeCharArray(detailedArray, (char[]) elementValues); + break; + case TypeConstants.SHORT: + storeShortArray(detailedArray, (short[]) elementValues); + break; + case TypeConstants.INT: + storeIntArray(detailedArray, (int[]) elementValues); + break; + case TypeConstants.LONG: + storeLongArray(detailedArray, (long[]) elementValues); + break; + case TypeConstants.FLOAT: + storeFloatArray(detailedArray, (float[]) elementValues); + break; + case TypeConstants.DOUBLE: + storeDoubleArray(detailedArray, (double[]) elementValues); + break; + default: + storeObjectArray(detailedArray, (Object[]) elementValues); } + return detailedArray; } @@ -150,11 +173,48 @@ private void storeDoubleArray(DetailedArrayReferenceValue detailedArray, double[ } } - private void storeObjectArray(DetailedArrayReferenceValue detailedArray, Object[] elementValues) { - for (int i = 0; i < elementValues.length; i++) { - detailedArray.arrayStore( - createIntegerValue(i), - createReferenceValue(detailedArray.referencedClass, elementValues[i])); + private void storeObjectArray(DetailedArrayReferenceValue detailedArray, Object[] elements) { + for (int i = 0; i < elements.length; i++) { + Object element = elements[i]; + Value elementValue; + if (element == null) { + elementValue = createReferenceValueNull(); + } else { + Clazz referencedClass = detailedArray.referencedClass; + AnalyzedObject object = + AnalyzedObjectFactory.create( + element, + ClassUtil.internalTypeFromClassName(referencedClass.getName()), + referencedClass); + // Call on referenceValueFactory instead of "this" to avoid the behavior from + // IdentifiedValueFactory (from which DetailedArrayValueFactory should probably not + // inherit). + elementValue = + referenceValueFactory.createReferenceValue( + referencedClass, ClassUtil.isExtendable(referencedClass), false, object); + } + detailedArray.arrayStore(createIntegerValue(i), elementValue); + } + } + + /** + * A value factory that should never be used, used as a placeholder for the arrayValueFactory in + * {@link ParticularValueFactory}, since all the methods calling it should be overridden by {@link + * DetailedArrayValueFactory}. + */ + private static class UnusableArrayValueFactory extends ArrayReferenceValueFactory { + @Override + public ReferenceValue createArrayReferenceValue( + String type, Clazz referencedClass, IntegerValue arrayLength) { + throw new IllegalStateException( + "This value factory should never be used, DetailedArrayValueFactory should override all methods calling this"); + } + + @Override + public ReferenceValue createArrayReferenceValue( + String type, Clazz referencedClass, IntegerValue arrayLength, Object elementValues) { + throw new IllegalStateException( + "This value factory should never be used, DetailedArrayValueFactory should override all methods calling this"); } } } diff --git a/base/src/main/java/proguard/evaluation/value/IdentifiedValueFactory.java b/base/src/main/java/proguard/evaluation/value/IdentifiedValueFactory.java index e111d4085..49c639406 100644 --- a/base/src/main/java/proguard/evaluation/value/IdentifiedValueFactory.java +++ b/base/src/main/java/proguard/evaluation/value/IdentifiedValueFactory.java @@ -33,6 +33,30 @@ public class IdentifiedValueFactory extends ParticularValueFactory { private final AtomicInteger doubleID = new AtomicInteger(0); private static final AtomicInteger referenceIdProvider = new AtomicInteger(0); + /** Creates a new IdentifiedValueFactory which does not keep track of particular references. */ + public IdentifiedValueFactory() { + super(); + } + + /** + * Creates a new IdentifiedValueFactory, which uses the given valuefactory for both array and + * non-array reference construction. + */ + public IdentifiedValueFactory(ValueFactory referenceValueFactory) { + super(referenceValueFactory); + } + + /** + * Creates a new IdentifiedValueFactory. + * + * @param arrayReferenceValueFactory the valuefactory to delegate new array references to. + * @param referenceValueFactory the valuefactory to delegate new references to. + */ + public IdentifiedValueFactory( + ValueFactory arrayReferenceValueFactory, ValueFactory referenceValueFactory) { + super(arrayReferenceValueFactory, referenceValueFactory); + } + // Implementations for BasicValueFactory. public IntegerValue createIntegerValue() { @@ -121,9 +145,7 @@ public ReferenceValue createReferenceValueForId( boolean mayBeNull, Object id, Object value) { - return type == null - ? TypedReferenceValueFactory.REFERENCE_VALUE_NULL - : new IdentifiedReferenceValue(type, referencedClass, mayBeExtension, mayBeNull, this, id); + return this.createReferenceValueForId(type, referencedClass, mayBeExtension, mayBeNull, id); } public ReferenceValue createArrayReferenceValue( diff --git a/base/src/main/java/proguard/evaluation/value/ParticularReferenceValue.java b/base/src/main/java/proguard/evaluation/value/ParticularReferenceValue.java index 730d39ccc..fbbb94d24 100644 --- a/base/src/main/java/proguard/evaluation/value/ParticularReferenceValue.java +++ b/base/src/main/java/proguard/evaluation/value/ParticularReferenceValue.java @@ -28,6 +28,8 @@ * with an associated value. E.g., a String with the value "HelloWorld". */ public class ParticularReferenceValue extends IdentifiedReferenceValue { + public static boolean ARRAY_EXCEPTIONS = + System.getProperty("proguard.particularvalue.arrayexceptions") != null; // The actual value of the object. private final AnalyzedObject objectValue; @@ -51,6 +53,12 @@ public ParticularReferenceValue( value.getType(), "ParticularReferenceValue should not be created with a 'NullObject', a 'TypedReferenceValue' with null type is expected in that case"); + if (ARRAY_EXCEPTIONS + && proguard.classfile.util.ClassUtil.isInternalArrayType(value.getType())) { + throw new IllegalStateException( + "ParticularReferenceValue should not be used for arrays use DetailedArrayReferenceValue instead"); + } + this.objectValue = value; } diff --git a/base/src/main/java/proguard/evaluation/value/ParticularValueFactory.java b/base/src/main/java/proguard/evaluation/value/ParticularValueFactory.java index 07d701afa..6d8b332d6 100644 --- a/base/src/main/java/proguard/evaluation/value/ParticularValueFactory.java +++ b/base/src/main/java/proguard/evaluation/value/ParticularValueFactory.java @@ -48,9 +48,11 @@ public class ParticularValueFactory extends BasicValueFactory implements ValueFa private static final long POS_ZERO_DOUBLE_BITS = Double.doubleToLongBits(0.0); private final ValueFactory arrayReferenceValueFactory; - // if set to true, a ParticularReferenceValue will be created for each reference. Otherwise, this - // will be delegated to the referenceValueFactory. - private final ValueFactory referenceValueFactory; + // This is protected to be usable directly by children, which should ideally be not allowed, but + // this is a hacky fix for improper inheritance currently taking place (i.e., + // DetailedArrayReferenceValue inheriting from IdentifiedReferenceValue, which inherits from this + // class). + protected final ValueFactory referenceValueFactory; /** Creates a new ParticularValueFactory which does not keep track of particular references. */ public ParticularValueFactory() { diff --git a/base/src/main/java/proguard/evaluation/value/object/AnalyzedObjectFactory.java b/base/src/main/java/proguard/evaluation/value/object/AnalyzedObjectFactory.java index 3da76fb9a..169a0b016 100644 --- a/base/src/main/java/proguard/evaluation/value/object/AnalyzedObjectFactory.java +++ b/base/src/main/java/proguard/evaluation/value/object/AnalyzedObjectFactory.java @@ -4,8 +4,10 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import proguard.classfile.Clazz; +import proguard.classfile.TypeConstants; import proguard.classfile.util.ClassUtil; import proguard.classfile.util.InitializedClassUtil; +import proguard.evaluation.value.ParticularReferenceValue; import proguard.evaluation.value.Value; /** Factory methods to create {@link AnalyzedObject}. */ @@ -63,8 +65,9 @@ public static AnalyzedObject create( return createModeled(model); } - // TODO create ArrayObject if value is an array once we fix the code using - // ParticularReferenceValue for arrays + if (ParticularReferenceValue.ARRAY_EXCEPTIONS) { + checkNotArray(value, type); + } checkValidPrecise(value, type, referencedClass); return createPrecise(value); @@ -114,6 +117,12 @@ private static void checkValidModel(Model model, Clazz referencedClass) { } } + private static void checkNotArray(Object value, String type) { + if (value.getClass().isArray() || TypeConstants.ARRAY == type.charAt(0)) { + throw new IllegalStateException("Arrays are supported only via ArrayModel"); + } + } + private static void checkValidPrecise(Object value, String type, Clazz referencedClass) { String objectType = ClassUtil.internalType(value.getClass().getTypeName()); if (referencedClass == null && !type.equals(objectType)) { diff --git a/base/src/test/kotlin/proguard/DetailedArrayTest.kt b/base/src/test/kotlin/proguard/DetailedArrayTest.kt index 459394be2..8f160241b 100644 --- a/base/src/test/kotlin/proguard/DetailedArrayTest.kt +++ b/base/src/test/kotlin/proguard/DetailedArrayTest.kt @@ -23,7 +23,7 @@ import proguard.testutils.PartialEvaluatorUtil import proguard.util.PartialEvaluatorHelper class DetailedArrayTest : FreeSpec({ - val valueFactory: ValueFactory = ParticularValueFactory(DetailedArrayValueFactory(), ParticularReferenceValueFactory()) + val valueFactory: ValueFactory = ParticularValueFactory(DetailedArrayValueFactory(ParticularReferenceValueFactory()), ParticularReferenceValueFactory()) "Array values evaluated correctly" - { @@ -418,7 +418,7 @@ class DetailedArrayTest : FreeSpec({ javacArguments = listOf("-source", "8", "-target", "8"), ) - val invocationsWithStack = PartialEvaluatorHelper.evaluateMethod("A", "functions", "()V", programClassPool, libraryClassPool, DetailedArrayValueFactory()) + val invocationsWithStack = PartialEvaluatorHelper.evaluateMethod("A", "functions", "()V", programClassPool, libraryClassPool, DetailedArrayValueFactory(ParticularReferenceValueFactory())) "Primitive byte array evaluated correctly" { val value = invocationsWithStack[14]!!.stack[0] diff --git a/base/src/test/kotlin/proguard/ParticularReferenceTest.kt b/base/src/test/kotlin/proguard/ParticularReferenceTest.kt index 13bcfc7ed..94d9a994b 100644 --- a/base/src/test/kotlin/proguard/ParticularReferenceTest.kt +++ b/base/src/test/kotlin/proguard/ParticularReferenceTest.kt @@ -21,6 +21,7 @@ import proguard.classfile.editor.ClassBuilder import proguard.evaluation.BasicInvocationUnit import proguard.evaluation.PartialEvaluator import proguard.evaluation.ParticularReferenceValueFactory +import proguard.evaluation.value.ArrayReferenceValue import proguard.evaluation.value.ArrayReferenceValueFactory import proguard.evaluation.value.IdentifiedReferenceValue import proguard.evaluation.value.ParticularIntegerValue @@ -763,9 +764,9 @@ class ParticularReferenceTest : FreeSpec({ "String array evaluated correctly" { val value = invocationsWithStack[14]!!.stack[0] - value.shouldBeInstanceOf() + // NB: this is not an IdentifiedArrayReferenceValue because ArrayReferenceValueFactory does not support identified arrays + value.shouldBeInstanceOf() value.type shouldBe "[Ljava/lang/String;" - value.value() shouldBe arrayOf("42", "43") } } diff --git a/base/src/test/kotlin/proguard/analysis/ExecutingInvocationUnitTest.kt b/base/src/test/kotlin/proguard/analysis/ExecutingInvocationUnitTest.kt index faa481efd..7d1f531dc 100644 --- a/base/src/test/kotlin/proguard/analysis/ExecutingInvocationUnitTest.kt +++ b/base/src/test/kotlin/proguard/analysis/ExecutingInvocationUnitTest.kt @@ -16,12 +16,15 @@ import proguard.evaluation.ValueCalculator import proguard.evaluation.executor.Executor import proguard.evaluation.executor.MethodExecutionInfo import proguard.evaluation.value.ArrayReferenceValueFactory +import proguard.evaluation.value.DetailedArrayReferenceValue import proguard.evaluation.value.DetailedArrayValueFactory import proguard.evaluation.value.IdentifiedReferenceValue import proguard.evaluation.value.ParticularReferenceValue import proguard.evaluation.value.ParticularValueFactory +import proguard.evaluation.value.ReferenceValue import proguard.evaluation.value.TypedReferenceValue import proguard.evaluation.value.ValueFactory +import proguard.evaluation.value.`object`.ArrayModel import proguard.testutils.ClassPoolBuilder import proguard.testutils.JavaSource import proguard.testutils.PartialEvaluatorUtil @@ -323,7 +326,7 @@ class ExecutingInvocationUnitTest : FreeSpec({ ) val (programClassPool, libraryClassPool) = ClassPoolBuilder.fromSource(code, javacArguments = listOf("-g", "-source", "1.8", "-target", "1.8")) - val valueFactory: ValueFactory = ParticularValueFactory(DetailedArrayValueFactory(), ParticularReferenceValueFactory()) + val valueFactory: ValueFactory = ParticularValueFactory(DetailedArrayValueFactory(ParticularReferenceValueFactory()), ParticularReferenceValueFactory()) val invocationUnit = ExecutingInvocationUnit.Builder().setEnableSameInstanceIdApproximation(true).build(valueFactory, libraryClassPool) val partialEvaluator = PartialEvaluator( valueFactory, @@ -362,7 +365,7 @@ class ExecutingInvocationUnitTest : FreeSpec({ ) val (programClassPool, libraryClassPool) = ClassPoolBuilder.fromSource(code, javacArguments = listOf("-g", "-source", "1.8", "-target", "1.8")) - val valueFactory: ValueFactory = ParticularValueFactory(DetailedArrayValueFactory(), ParticularReferenceValueFactory()) + val valueFactory: ValueFactory = ParticularValueFactory(DetailedArrayValueFactory(ParticularReferenceValueFactory()), ParticularReferenceValueFactory()) val invocationUnit = ExecutingInvocationUnit.Builder().setEnableSameInstanceIdApproximation(true).build(valueFactory, libraryClassPool) val partialEvaluator = PartialEvaluator( valueFactory, @@ -402,7 +405,7 @@ class ExecutingInvocationUnitTest : FreeSpec({ ) val (programClassPool, libraryClassPool) = ClassPoolBuilder.fromSource(code, javacArguments = listOf("-g", "-source", "1.8", "-target", "1.8")) - val valueFactory: ValueFactory = ParticularValueFactory(DetailedArrayValueFactory(), ParticularReferenceValueFactory()) + val valueFactory: ValueFactory = ParticularValueFactory(DetailedArrayValueFactory(ParticularReferenceValueFactory()), ParticularReferenceValueFactory()) val fooExecutor = object : Executor { @@ -449,4 +452,45 @@ class ExecutingInvocationUnitTest : FreeSpec({ str.shouldBeInstanceOf() str.value.preciseValue shouldBe "42" } + + "Executed methods returning non-primitive array has correct type and value" - { + + val code = JavaSource( + "Test.java", + """ + class Test { + public void test(){ + String[] array = "Hello World".split(" "); + } + } + """, + ) + + val (programClassPool, libraryClassPool) = ClassPoolBuilder.fromSource(code, javacArguments = listOf("-g", "-source", "1.8", "-target", "1.8")) + val valueFactory: ValueFactory = ParticularValueFactory(DetailedArrayValueFactory(ParticularReferenceValueFactory()), ParticularReferenceValueFactory()) + val invocationUnit = ExecutingInvocationUnit.Builder().build(valueFactory, libraryClassPool) + val partialEvaluator = PartialEvaluator( + valueFactory, + invocationUnit, + false, + ) + + val (instructions, variableTable) = PartialEvaluatorUtil.evaluate( + "Test", + "test", + "()V", + programClassPool, + partialEvaluator, + ) + + val (instruction, _) = instructions.last() + + val array = partialEvaluator.getVariablesBefore(instruction).getValue(variableTable["array"]!!) + + "String array" { + array.shouldBeInstanceOf() + array.type shouldBe "[Ljava/lang/String;" + (array.value.modeledValue as ArrayModel).values.map { value -> (value as ReferenceValue).value.preciseValue } shouldBe arrayOf("Hello", "World") + } + } }) diff --git a/base/src/test/kotlin/proguard/analysis/PartialEvaluatorErrorsTest.kt b/base/src/test/kotlin/proguard/analysis/PartialEvaluatorErrorsTest.kt index 231263581..cef7a5a15 100644 --- a/base/src/test/kotlin/proguard/analysis/PartialEvaluatorErrorsTest.kt +++ b/base/src/test/kotlin/proguard/analysis/PartialEvaluatorErrorsTest.kt @@ -390,7 +390,7 @@ class PartialEvaluatorErrorsTest : FreeSpec({ programClass, PartialEvaluator( ParticularValueFactory( - DetailedArrayValueFactory(), + DetailedArrayValueFactory(ParticularReferenceValueFactory()), ParticularReferenceValueFactory(), ), ), @@ -419,7 +419,7 @@ class PartialEvaluatorErrorsTest : FreeSpec({ programClass, PartialEvaluator( ParticularValueFactory( - DetailedArrayValueFactory(), + DetailedArrayValueFactory(ParticularReferenceValueFactory()), ParticularReferenceValueFactory(), ), ), diff --git a/base/src/test/kotlin/proguard/analysis/SelectiveCallResolverTest.kt b/base/src/test/kotlin/proguard/analysis/SelectiveCallResolverTest.kt index b41e1559b..d6845f4cc 100644 --- a/base/src/test/kotlin/proguard/analysis/SelectiveCallResolverTest.kt +++ b/base/src/test/kotlin/proguard/analysis/SelectiveCallResolverTest.kt @@ -23,6 +23,7 @@ import io.kotest.matchers.shouldBe import proguard.analysis.datastructure.callgraph.Call import proguard.analysis.datastructure.callgraph.CallGraph import proguard.classfile.MethodSignature +import proguard.evaluation.ParticularReferenceValueFactory import proguard.evaluation.value.DetailedArrayValueFactory import proguard.evaluation.value.IdentifiedReferenceValue import proguard.evaluation.value.ParticularReferenceValue @@ -134,7 +135,7 @@ class C { .setIncludeSubClasses(false) .setMaxPartialEvaluations(50) .setSkipIncompleteCalls(false) - .setArrayValueFactory(DetailedArrayValueFactory()) + .setArrayValueFactory(DetailedArrayValueFactory(ParticularReferenceValueFactory())) .useSelectiveParameterReconstruction(interestingMethods, interestingCallPredicates) .build() diff --git a/base/src/test/kotlin/proguard/analysis/cpa/ExecutingInvocationUnitTest.kt b/base/src/test/kotlin/proguard/analysis/cpa/ExecutingInvocationUnitTest.kt index 3dae966ad..a7262f746 100644 --- a/base/src/test/kotlin/proguard/analysis/cpa/ExecutingInvocationUnitTest.kt +++ b/base/src/test/kotlin/proguard/analysis/cpa/ExecutingInvocationUnitTest.kt @@ -36,7 +36,7 @@ import java.lang.ClassCastException private val javaLangString = libraryClassPool.getClass("java/lang/String") private val javaLangStringBuilder = libraryClassPool.getClass("java/lang/StringBuilder") -private val valueFactory = ParticularValueFactory(DetailedArrayValueFactory(), ParticularReferenceValueFactory()) +private val valueFactory = ParticularValueFactory(DetailedArrayValueFactory(ParticularReferenceValueFactory()), ParticularReferenceValueFactory()) private val stringExecutor = StringReflectionExecutor(libraryClassPool) private val invocationUnit = ExecutingInvocationUnit.Builder().build(valueFactory, libraryClassPool) private fun Int.toValue(): Value = diff --git a/docs/md/releasenotes.md b/docs/md/releasenotes.md index 867100272..565ad17cc 100644 --- a/docs/md/releasenotes.md +++ b/docs/md/releasenotes.md @@ -8,12 +8,16 @@ - Prevent `unknown enum value for KmVersionRequirementVersionKind` exception when processing code compiled with an outdated Kotlin version. - Fix `UnknownReferenceValue` return wrong string format in `getType`. - Fix `ReflectionExecutor` not updating instance of `StringBuilder`s in fallback result. +- Fix `DetailedArrayValueFactory#createArrayReferenceValue` expecting an array type instead of the type of the values contained in the array. +- Fix `ExecutingInvocationUnit` using both `ParticularReferenceValue` and `DetailedArrayReferenceValue` for arrays of reference types. Now `DetailedArrayValueFactory` is used consistently. ### API changes - `Executor`s do not support `MethodSignature` wildcards anymore. The assumption from `ExecutorLookup` is now that all the signatures supported by the executor are declared explicitly in `getSupportedMethodSignatures`. - `StringExecutor`, `ExecutingInvocationUnit`, and `JvmValueBamCpaRun` now need the library class pool as parameter. - Calls to `InstructionSequenceBuilder.ldc` now optionally accept a `ConstantVisitor`. The visitor will visit the constant that is referenced by the added instruction. +- `DetailedArrayValueFactory` now takes a `referenceValueFactory` parameter, which determines if the value factory supports direct creation of reference arrays. +- `ParticularReferenceValue` can't be used with arrays anymore. ## Version 9.1.4 diff --git a/examples/src/main/java/proguard/examples/EvaluateCode.java b/examples/src/main/java/proguard/examples/EvaluateCode.java index 095c1e284..5f416bd72 100644 --- a/examples/src/main/java/proguard/examples/EvaluateCode.java +++ b/examples/src/main/java/proguard/examples/EvaluateCode.java @@ -138,7 +138,8 @@ private static PartialEvaluator createPartialEvaluator(String precision) { : precision.equals(ARRAY) ? new ArrayReferenceValueFactory() : precision.equals(DETAILEDARRAY) - ? new DetailedArrayValueFactory() + ? new DetailedArrayValueFactory( + new TypedReferenceValueFactory()) : unknownPrecision(precision); // In this example, we pick an invocation unit that doesn't try to