Skip to content

Commit

Permalink
Fix MemberDescriptorSpecializer checking wrong processing flags
Browse files Browse the repository at this point in the history
This diff also ensures the `IS_CLASS_AVAILABLE` processing flag is set on classes instead of its members.
This requires us to change usages of this flag in some other places as well.
  • Loading branch information
robinlefever committed Dec 1, 2023
1 parent a02100c commit 12c9c3f
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 61 deletions.
2 changes: 1 addition & 1 deletion base/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ dependencies {
testImplementation 'io.kotest:kotest-property-jvm:5.5.4' // for kotest property test
testImplementation 'io.mockk:mockk:1.13.2' // for mocking

testImplementation(testFixtures("com.guardsquare:proguard-core:9.0.8")) {
testImplementation(testFixtures("com.guardsquare:proguard-core:${proguardCoreVersion}")) {
exclude group: 'com.guardsquare', module: 'proguard-core'
}
}
Expand Down
3 changes: 1 addition & 2 deletions base/src/main/java/proguard/mark/Marker.java
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,7 @@ private void markSafeGeneralizationMembers(ClassPool programClassPool,
{
// Program classes are always available and safe to generalize/specialize from/to.
ClassVisitor isClassAvailableMarker =
new AllMemberVisitor(
new ProcessingFlagSetter(ProcessingFlags.IS_CLASS_AVAILABLE));
new ProcessingFlagSetter(ProcessingFlags.IS_CLASS_AVAILABLE);

programClassPool.classesAccept(isClassAvailableMarker);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public void visitProgramField(ProgramClass programClass, ProgramField programFie

if (valueClass != null &&
valueClass.extendsOrImplements(ClassUtil.internalClassNameFromClassType(fieldType)) &&
(programField.getProcessingFlags() & ProcessingFlags.IS_CLASS_AVAILABLE) != 0)
(valueClass.getProcessingFlags() & ProcessingFlags.IS_CLASS_AVAILABLE) != 0)
{
logger.debug("MemberDescriptorSpecializer [{}.{} {}] -> {}",
programClass.getName(),
Expand Down Expand Up @@ -210,7 +210,7 @@ public void visitProgramMethod(ProgramClass programClass, ProgramMethod programM

if (valueClass != null &&
valueClass.extendsOrImplements(ClassUtil.internalClassNameFromClassType(parameterType)) &&
(programMethod.getProcessingFlags() & ProcessingFlags.IS_CLASS_AVAILABLE) != 0)
(valueClass.getProcessingFlags() & ProcessingFlags.IS_CLASS_AVAILABLE) != 0)
{
logger.debug("MemberDescriptorSpecializer [{}.{}{}]: parameter #{}: {} -> {}",
programClass.getName(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,22 @@

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import proguard.classfile.*;
import proguard.classfile.AccessConstants;
import proguard.classfile.Clazz;
import proguard.classfile.Field;
import proguard.classfile.Member;
import proguard.classfile.Method;
import proguard.classfile.ProgramClass;
import proguard.classfile.attribute.CodeAttribute;
import proguard.classfile.constant.*;
import proguard.classfile.constant.AnyMethodrefConstant;
import proguard.classfile.constant.ClassConstant;
import proguard.classfile.constant.FieldrefConstant;
import proguard.classfile.constant.RefConstant;
import proguard.classfile.constant.visitor.ConstantVisitor;
import proguard.classfile.editor.*;
import proguard.classfile.instruction.*;
import proguard.classfile.editor.CodeAttributeEditor;
import proguard.classfile.editor.ConstantPoolEditor;
import proguard.classfile.instruction.ConstantInstruction;
import proguard.classfile.instruction.Instruction;
import proguard.classfile.instruction.visitor.InstructionVisitor;
import proguard.classfile.visitor.ClassVisitor;
import proguard.util.ProcessingFlags;
Expand Down Expand Up @@ -200,7 +210,7 @@ public void visitAnyRefConstant(Clazz clazz, RefConstant refConstant)
// DGD-486: Only generalize members which are always available. Partial replacement of a class that is not
// available on all platforms may result in a VerifyError at runtime.
if (referencedMember != null &&
(referencedMember.getProcessingFlags() & ProcessingFlags.IS_CLASS_AVAILABLE) != 0)
(clazz.getProcessingFlags() & ProcessingFlags.IS_CLASS_AVAILABLE) != 0)
{
clazz.constantPoolEntryAccept(refConstant.u2classIndex, this);

Expand Down Expand Up @@ -287,13 +297,14 @@ public void visitAnyClass(Clazz clazz)
// Otherwise, look in the super class itself.
// Only consider public classes and methods, to avoid any
// access problems.
// Only consider classes that are marked as available.
if (generalizedClass == null &&
(superClass.getAccessFlags() & AccessConstants.PUBLIC) != 0)
(superClass.getAccessFlags() & AccessConstants.PUBLIC) != 0 &&
(superClass.getProcessingFlags() & ProcessingFlags.IS_CLASS_AVAILABLE) != 0)
{
Method method = superClass.findMethod(memberName, memberType);
if (method != null &&
(method.getAccessFlags() & AccessConstants.PUBLIC) != 0 &&
(method.getProcessingFlags() & ProcessingFlags.IS_CLASS_AVAILABLE) != 0)
(method.getAccessFlags() & AccessConstants.PUBLIC) != 0)
{
// Remember the generalized class and class member.
generalizedClass = superClass;
Expand All @@ -308,7 +319,7 @@ public void visitAnyClass(Clazz clazz)
Field field = clazz.findField(memberName, memberType);

if (field != null &&
(field.getProcessingFlags() & ProcessingFlags.IS_CLASS_AVAILABLE) != 0)
(clazz.getProcessingFlags() & ProcessingFlags.IS_CLASS_AVAILABLE) != 0)
{
// Remember the generalized class and class member.
generalizedClass = clazz;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package proguard.optimize
import io.kotest.core.spec.style.FreeSpec
import io.kotest.matchers.shouldBe
import proguard.classfile.AccessConstants
import proguard.classfile.ClassPool
import proguard.classfile.Clazz
import proguard.classfile.Member
import proguard.classfile.attribute.visitor.AllAttributeVisitor
Expand All @@ -26,7 +27,67 @@ import proguard.util.ProcessingFlagSetter
import proguard.util.ProcessingFlags.IS_CLASS_AVAILABLE

class MemberDescriptorSpecializerTest : FreeSpec({
"Given a method with a more general parameter type than its use" - {

fun specializeMemberDescriptors(
programClassPool: ClassPool,
libraryClassPool: ClassPool,
) {
// Mark all program classes as available.
programClassPool.classesAccept(ProcessingFlagSetter(IS_CLASS_AVAILABLE))

// Setup the OptimizationInfo on the classes
val keepMarker = KeepMarker()
libraryClassPool.classesAccept(keepMarker)
libraryClassPool.classesAccept(AllMemberVisitor(keepMarker))

programClassPool.classesAccept(ProgramClassOptimizationInfoSetter())
programClassPool.classesAccept(AllMemberVisitor(ProgramMemberOptimizationInfoSetter()))

// Create the optimization as in Optimizer
val fillingOutValuesClassVisitor = ClassVisitorFactory {
val valueFactory: ValueFactory = ParticularValueFactory()
val storingInvocationUnit: InvocationUnit = StoringInvocationUnit(
valueFactory,
true,
true,
true
)
ClassAccessFilter(
0, AccessConstants.SYNTHETIC,
AllMethodVisitor(
AllAttributeVisitor(
DebugAttributeVisitor(
"Filling out fields, method parameters, and return values",
PartialEvaluator(
valueFactory, storingInvocationUnit,
true
)
)
)
)
)
}

programClassPool.classesAccept(fillingOutValuesClassVisitor.createClassVisitor())

// Specialize class member descriptors, based on partial evaluation.
programClassPool.classesAccept(
AllMemberVisitor(
OptimizationInfoMemberFilter(
MemberDescriptorSpecializer(
true,
true,
true,
null,
null,
null
)
)
)
)
}

"Given a method with a more general program class pool parameter type than its use" - {
val (programClassPool, libraryClassPool) = ClassPoolBuilder.fromSource(
JavaSource(
"Test.java",
Expand All @@ -48,68 +109,97 @@ class MemberDescriptorSpecializerTest : FreeSpec({
)

"When specializing the member descriptors" - {
specializeMemberDescriptors(programClassPool, libraryClassPool)

// Mark all members as available.
programClassPool.classesAccept(AllMemberVisitor(ProcessingFlagSetter(IS_CLASS_AVAILABLE)))

// Setup the OptimizationInfo on the classes
val keepMarker = KeepMarker()
libraryClassPool.classesAccept(keepMarker)
libraryClassPool.classesAccept(AllMemberVisitor(keepMarker))

programClassPool.classesAccept(ProgramClassOptimizationInfoSetter())
programClassPool.classesAccept(AllMemberVisitor(ProgramMemberOptimizationInfoSetter()))

// Create the optimization as in Optimizer
val fillingOutValuesClassVisitor = ClassVisitorFactory {
val valueFactory: ValueFactory = ParticularValueFactory()
val storingInvocationUnit: InvocationUnit = StoringInvocationUnit(
valueFactory,
true,
true,
true
)
ClassAccessFilter(
0, AccessConstants.SYNTHETIC,
AllMethodVisitor(
AllAttributeVisitor(
DebugAttributeVisitor(
"Filling out fields, method parameters, and return values",
PartialEvaluator(
valueFactory, storingInvocationUnit,
true
)
)
"Then the member descriptor should be correctly specialised" {
lateinit var memberDescriptor: String
programClassPool.classAccept(
"Test",
AllMemberVisitor(
MemberNameFilter(
"foo*",
object : MemberVisitor {
override fun visitAnyMember(clazz: Clazz, member: Member) {
memberDescriptor = member.getDescriptor(clazz)
}
}
)
)
)
memberDescriptor shouldBe "(LFoo;)V"
}
}
}

programClassPool.classesAccept(fillingOutValuesClassVisitor.createClassVisitor())

// Specialize class member descriptors, based on partial evaluation.
programClassPool.classesAccept(
AllMemberVisitor(
OptimizationInfoMemberFilter(
MemberDescriptorSpecializer(
true,
true,
true,
null,
null,
null
"Given a field with a more general program class pool parameter type than its use" - {
val (programClassPool, libraryClassPool) = ClassPoolBuilder.fromSource(
JavaSource(
"Test.java",
"""
public class Test {
static Bar myField = null;

public static void main(String[] args) {
myField = new Foo();
}
}

class Bar { }

class Foo extends Bar { }
""".trimIndent()
)
)

"When specializing the member descriptors" - {
specializeMemberDescriptors(programClassPool, libraryClassPool)

"Then the member descriptor should be correctly specialised" {
lateinit var memberDescriptor: String
programClassPool.classAccept(
"Test",
AllMemberVisitor(
MemberNameFilter(
"myField*",
object : MemberVisitor {
override fun visitAnyMember(clazz: Clazz, member: Member) {
memberDescriptor = member.getDescriptor(clazz)
}
}
)
)
)
memberDescriptor shouldBe "LFoo;"
}
}
}

"Given a field with a more general library class pool parameter type than its use" - {
val (programClassPool, libraryClassPool) = ClassPoolBuilder.fromSource(
JavaSource(
"Test.java",
"""
public class Test {
static java.lang.Object myField = null;

public static void main(String[] args) {
myField = new java.lang.StringBuffer();
}
}
""".trimIndent()
)
)

"When specializing the member descriptors" - {
specializeMemberDescriptors(programClassPool, libraryClassPool)

"Then the member descriptor should be correctly specialised" {
lateinit var memberDescriptor: String
programClassPool.classAccept(
"Test",
AllMemberVisitor(
MemberNameFilter(
"foo*",
"myField*",
object : MemberVisitor {
override fun visitAnyMember(clazz: Clazz, member: Member) {
memberDescriptor = member.getDescriptor(clazz)
Expand All @@ -118,7 +208,8 @@ class MemberDescriptorSpecializerTest : FreeSpec({
)
)
)
memberDescriptor shouldBe "(LFoo;)V"
// Library classes are not marked as available by default. Therefore, they are not specialized.
memberDescriptor shouldBe "Ljava/lang/Object;"
}
}
}
Expand Down
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 potential access issues when backporting.
- Fix potential NoClassDefFoundError when using type specialization optimization. (#373)

## Version 7.4.1

Expand Down

0 comments on commit 12c9c3f

Please sign in to comment.