Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Kotlin simple enums in optimization #350

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
import proguard.evaluation.value.*;
import proguard.optimize.info.SimpleEnumMarker;

import static proguard.classfile.ClassConstants.NAME_JAVA_LANG_OBJECT;

/**
* This AttributeVisitor simplifies the use of enums in the code attributes that
* it visits.
Expand Down Expand Up @@ -277,6 +279,15 @@ public void visitConstantInstruction(Clazz clazz, Method method, CodeAttribute c
invokedMethodName,
invokedMethodType);
}
else if (isObjectCloneCallPoppingSimpleEnumArray(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this also trigger when we have a reguler Object.clone call that has nothing to do with Kotlin? We shouldn't apply this replacement in that case then? Could be useful to also add a test for that.

offset,
stackEntryIndex,
invokedMethodName,
invokedMethodType,
clazz.getRefClassName(constantInstruction.constantIndex)))
{
replaceObjectCloneCall(clazz, offset, constantInstruction, invokedMethodName, invokedMethodType);
}

// Fall through to check the parameters.
}
Expand Down Expand Up @@ -567,6 +578,26 @@ private boolean isPoppingSimpleEnumArray(int offset, int stackEntryIndex)
ClassUtil.internalArrayTypeDimensionCount(referenceValue.getType()) == 1;
}

/**
* Returns whether the instruction at the given offset is popping a simple enum array
* and the class reference is java.lang.Object.
* <p>
* This accounts for the Kotlin compiler generating the following call in the `values()` method:
* <code>
* invokevirtual [Ljava/lang/Object;->clone();
* </code>
* where the Java compiler would generate:
* <code>
* invokevirtual [LMyEnum;->clone();
* </code>
*/
private boolean isObjectCloneCallPoppingSimpleEnumArray(int offset, int stackEntryIndex, String invokedMethodName, String invokedMethodType, String classRef)
{
return invokedMethodName.equals("clone") &&
invokedMethodType.equals("()Ljava/lang/Object;") &&
ClassUtil.internalClassNameFromClassType(classRef).equals(NAME_JAVA_LANG_OBJECT) &&
isPoppingSimpleEnumArray(offset, stackEntryIndex);
}

/**
* Returns whether the given class is not null and a simple enum class.
Expand Down Expand Up @@ -603,6 +634,29 @@ private void replaceSupportedMethod(Clazz clazz,
}
}

/**
* Replaces a Kotlin generated clone() invocation with the correct integer array invocation.
*/
private void replaceObjectCloneCall(Clazz clazz,
int offset,
Instruction instruction,
String name,
String type)
{
if (name.equals("clone") && type.equals("()Ljava/lang/Object;"))
{
ConstantPoolEditor cpe = new ConstantPoolEditor((ProgramClass) clazz);
Instruction[] replacementInstructions = new Instruction[] {
new ConstantInstruction(
Instruction.OP_INVOKEVIRTUAL,
cpe.addMethodrefConstant("[I", "clone", "()Ljava/lang/Object;", null, null)
)
};

replaceInstructions(clazz, offset, instruction, replacementInstructions);
}
}


/**
* Replaces the instruction at the given offset by the given instructions.
Expand Down
109 changes: 109 additions & 0 deletions base/src/test/kotlin/proguard/optimize/EnumSimplifierTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
package proguard.optimize

import io.kotest.core.spec.style.FreeSpec
import io.kotest.matchers.shouldBe
import proguard.classfile.ClassPool
import proguard.classfile.ProgramClass
import proguard.classfile.ProgramMember
import proguard.classfile.attribute.visitor.AllAttributeVisitor
import proguard.classfile.editor.MemberReferenceFixer
import proguard.classfile.util.ClassPoolClassLoader
import proguard.classfile.visitor.AllMemberVisitor
import proguard.classfile.visitor.AllMethodVisitor
import proguard.classfile.visitor.MemberNameFilter
import proguard.classfile.visitor.MemberVisitor
import proguard.optimize.evaluation.SimpleEnumClassSimplifier
import proguard.optimize.evaluation.SimpleEnumDescriptorSimplifier
import proguard.optimize.evaluation.SimpleEnumUseSimplifier
import proguard.optimize.info.ProgramClassOptimizationInfoSetter
import proguard.optimize.info.ProgramMemberOptimizationInfoSetter
import proguard.optimize.info.SimpleEnumFilter
import proguard.optimize.info.SimpleEnumMarker
import proguard.testutils.ClassPoolBuilder
import proguard.testutils.JavaSource
import proguard.testutils.KotlinSource
import proguard.testutils.and
import proguard.testutils.match

class EnumSimplifierTest : FreeSpec({
"Test simplification of simple Kotlin enum (#349)" {
val (programClassPool, libraryClassPool) = ClassPoolBuilder.fromSource(
KotlinSource(
"TestEnum.kt", "enum class TestEnum { A, B, C }"
)
)

simplifyEnum(libraryClassPool, programClassPool)
}

"Test simplification of simple Java enum" {
val (programClassPool, libraryClassPool) = ClassPoolBuilder.fromSource(
JavaSource(
"TestEnum.java", "public enum TestEnum { A, B, C, }"
)
)

simplifyEnum(libraryClassPool, programClassPool)
}
})

private fun simplifyEnum(libraryClassPool: ClassPool, programClassPool: ClassPool) {
// Setup the OptimizationInfo on the classes
val keepMarker = KeepMarker()
libraryClassPool.classesAccept(keepMarker)
libraryClassPool.classesAccept(AllMemberVisitor(keepMarker))
programClassPool.classesAccept(ProgramClassOptimizationInfoSetter())
programClassPool.classesAccept(AllMemberVisitor(ProgramMemberOptimizationInfoSetter()))

programClassPool.classAccept("TestEnum", SimpleEnumMarker(true))

val enumClazz = programClassPool.getClass("TestEnum")

SimpleEnumMarker.isSimpleEnum(enumClazz) shouldBe true

// Application of enum simplification similar to the Optimizer.

// Simplify the use of the enum classes in code.
programClassPool.classesAccept(
AllMethodVisitor(
AllAttributeVisitor(
SimpleEnumUseSimplifier()
)
)
)

// Simplify the static initializers of simple enum classes.
programClassPool.classesAccept(
SimpleEnumFilter(
SimpleEnumClassSimplifier()
)
)

// Simplify the use of the enum classes in descriptors.
programClassPool.classesAccept(
SimpleEnumDescriptorSimplifier()
)

// Update references to class members with simple enum classes.
programClassPool.classesAccept(MemberReferenceFixer(false))

// Trigger class loading, that would previously result in a verify error.
val classPoolClassLoader = ClassPoolClassLoader(programClassPool)
val clazz = classPoolClassLoader.findClass("TestEnum")
clazz.fields.single { it.name.startsWith("A") }.get(null) shouldBe 1

enumClazz.methodsAccept(
MemberNameFilter(
"values*",
object : MemberVisitor {
override fun visitProgramMember(programClass: ProgramClass, programMember: ProgramMember) {
with(programClass and programMember) {
match {
invokevirtual("[I", "clone", "()Ljava/lang/Object;")
} shouldBe true
}
}
}
)
)
}
1 change: 1 addition & 0 deletions docs/md/manual/releasenotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
### Bugfixes

- Fix "NoClassDefFoundError: Failed resolution of: Lorg/apache/logging/log4j/LogManager" when using GSON optimization or `-addconfigurationdebugging`. (#326)
- Fix "VerifyError" when optimizing Kotlin simple enums. (#349)

## Version 7.3.2

Expand Down
Loading