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

Fix mapping stats not skipping anonymous classes #469

Open
wants to merge 5 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 @@ -54,7 +54,6 @@ public StatsResult generate(ProgressListener progress, Set<StatsMember> included
progress.init(totalWork, I18n.translate("progress.stats"));

Map<String, Integer> counts = new HashMap<>();

int numDone = 0;

if (includedMembers.contains(StatsMember.METHODS) || includedMembers.contains(StatsMember.PARAMETERS)) {
Expand Down Expand Up @@ -115,7 +114,7 @@ public StatsResult generate(ProgressListener progress, Set<StatsMember> included
}

private void update(Map<String, Integer> counts, Entry<?> entry) {
if (project.isObfuscated(entry)) {
if (project.isObfuscated(entry) && project.isRenamable(entry) && !project.isSynthetic(entry)) {
String parent = mapper.deobfuscate(entry.getAncestry().get(0)).getName().replace('/', '.');
counts.put(parent, counts.getOrDefault(parent, 0) + 1);
}
Expand Down
17 changes: 17 additions & 0 deletions enigma/src/main/java/cuchaz/enigma/EnigmaProject.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
import org.objectweb.asm.tree.ClassNode;

import cuchaz.enigma.analysis.EntryReference;
import cuchaz.enigma.analysis.index.InnerClassIndex;
import cuchaz.enigma.analysis.index.JarIndex;
import cuchaz.enigma.analysis.index.JarIndexer;
import cuchaz.enigma.api.service.NameProposalService;
import cuchaz.enigma.api.service.ObfuscationTestService;
import cuchaz.enigma.bytecode.translators.TranslationClassVisitor;
Expand Down Expand Up @@ -153,6 +155,17 @@ public boolean isRenamable(Entry<?> obfEntry) {
}
} else if (obfEntry instanceof LocalVariableEntry && !((LocalVariableEntry) obfEntry).isArgument()) {
return false;
} else if (obfEntry instanceof ClassEntry classEntry) {
InnerClassIndex innerClassIndex = jarIndex.getInnerClassIndex();

if (innerClassIndex.isInnerClass(classEntry) && innerClassIndex.hasOuterClassData(classEntry)) {
JarIndexer.InnerClassData innerClassData = innerClassIndex.getInnerClassData(classEntry);

if (!innerClassData.hasInnerName() && !innerClassData.hasOuterName()) {
// Anonymous classes don't have inner or outer names
return false;
}
}
}

return this.jarIndex.getEntryIndex().hasEntry(obfEntry);
Expand Down Expand Up @@ -194,6 +207,10 @@ public boolean isObfuscated(Entry<?> entry) {
return true;
}

public boolean isSynthetic(Entry<?> entry) {
return jarIndex.getEntryIndex().hasEntry(entry) && jarIndex.getEntryIndex().getEntryAccess(entry).isSynthetic();
}

public JarExport exportRemappedJar(ProgressListener progress) {
Collection<ClassEntry> classEntries = jarIndex.getEntryIndex().getClasses();
ClassProvider fixingClassProvider = new ObfuscationFixClassProvider(classProvider, jarIndex);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ public AccessFlags getEntryAccess(Entry<?> entry) {
return getFieldAccess(fieldEntry);
} else if (entry instanceof LocalVariableEntry localVariableEntry) {
return getMethodAccess(localVariableEntry.getParent());
} else if (entry instanceof ClassEntry classEntry) {
return getClassAccess(classEntry);
}

return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,20 @@ public void visit(int version, int access, String name, String signature, String
super.visit(version, access, name, signature, superName, interfaces);
}

@Override
public void visitInnerClass(String name, String outerName, String innerName, int access) {
indexer.indexInnerClass(classEntry, new JarIndexer.InnerClassData(name, outerName, innerName, access));

super.visitInnerClass(name, outerName, innerName, access);
}

@Override
public void visitOuterClass(String owner, String name, String descriptor) {
indexer.indexOuterClass(classEntry, new JarIndexer.OuterClassData(owner, name, descriptor));

super.visitOuterClass(owner, name, descriptor);
}

@Override
public FieldVisitor visitField(int access, String name, String desc, String signature, Object value) {
indexer.indexField(FieldDefEntry.parse(classEntry, access, name, desc, signature));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package cuchaz.enigma.analysis.index;

import java.util.HashMap;
import java.util.Map;
import java.util.Optional;

import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.Multimap;

import cuchaz.enigma.translation.representation.entry.ClassDefEntry;
import cuchaz.enigma.translation.representation.entry.ClassEntry;

public class InnerClassIndex implements JarIndexer {
private Multimap<ClassDefEntry, InnerClassData> innerClasses = ArrayListMultimap.create();
Copy link
Contributor

Choose a reason for hiding this comment

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

This thing is wrong; we don't want to record one map entry for every MethodHandles.Lookup reference. To check if a class is an anonymous class, simply check its EnclosingMethods https://docs.oracle.com/javase/specs/jvms/se19/html/jvms-4.html#jvms-4.7.7 which asm somehow calls "OuterClass" and describes it incorrectly

private Map<ClassDefEntry, OuterClassData> outerClassesData = new HashMap<>();

@Override
public void indexInnerClass(ClassDefEntry classEntry, InnerClassData innerClassData) {
innerClasses.put(classEntry, innerClassData);
}

@Override
public void indexOuterClass(ClassDefEntry classEntry, OuterClassData outerClassData) {
outerClassesData.put(classEntry, outerClassData);
}

private Optional<Map.Entry<ClassDefEntry, InnerClassData>> findInnerClassEntry(ClassEntry classEntry) {
return innerClasses.entries().stream().filter(entry -> entry.getValue().name().equals(classEntry.getFullName())).findFirst();
}

public boolean isInnerClass(ClassEntry classEntry) {
return findInnerClassEntry(classEntry).isPresent();
}

public InnerClassData getInnerClassData(ClassEntry classEntry) {
return findInnerClassEntry(classEntry).map(Map.Entry::getValue).orElse(null);
}

public ClassDefEntry getOuterClass(ClassEntry classEntry) {
return findInnerClassEntry(classEntry).map(Map.Entry::getKey).orElse(null);
}

private Optional<Map.Entry<ClassDefEntry, OuterClassData>> findOuterClassDataEntry(ClassEntry classEntry) {
return outerClassesData.entrySet().stream().filter(entry -> entry.getKey().equals(classEntry)).findFirst();
}

public boolean hasOuterClassData(ClassEntry classEntry) {
return findOuterClassDataEntry(classEntry).isPresent();
}

public OuterClassData getOuterClassData(ClassEntry classEntry) {
return findOuterClassDataEntry(classEntry).map(Map.Entry::getValue).orElse(null);
}
}
31 changes: 28 additions & 3 deletions enigma/src/main/java/cuchaz/enigma/analysis/index/JarIndex.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,20 +44,22 @@ public class JarIndex implements JarIndexer {
private final ReferenceIndex referenceIndex;
private final BridgeMethodIndex bridgeMethodIndex;
private final PackageVisibilityIndex packageVisibilityIndex;
private final InnerClassIndex innerClassIndex;
private final EntryResolver entryResolver;

private final Collection<JarIndexer> indexers;

private final Multimap<String, MethodDefEntry> methodImplementations = HashMultimap.create();
private final ListMultimap<ClassEntry, ParentedEntry> childrenByClass;

public JarIndex(EntryIndex entryIndex, InheritanceIndex inheritanceIndex, ReferenceIndex referenceIndex, BridgeMethodIndex bridgeMethodIndex, PackageVisibilityIndex packageVisibilityIndex) {
public JarIndex(EntryIndex entryIndex, InheritanceIndex inheritanceIndex, ReferenceIndex referenceIndex, BridgeMethodIndex bridgeMethodIndex, PackageVisibilityIndex packageVisibilityIndex, InnerClassIndex innerClassIndex) {
this.entryIndex = entryIndex;
this.inheritanceIndex = inheritanceIndex;
this.referenceIndex = referenceIndex;
this.bridgeMethodIndex = bridgeMethodIndex;
this.packageVisibilityIndex = packageVisibilityIndex;
this.indexers = List.of(entryIndex, inheritanceIndex, referenceIndex, bridgeMethodIndex, packageVisibilityIndex);
this.innerClassIndex = innerClassIndex;
this.indexers = List.of(entryIndex, inheritanceIndex, referenceIndex, bridgeMethodIndex, packageVisibilityIndex, innerClassIndex);
this.entryResolver = new IndexEntryResolver(this);
this.childrenByClass = ArrayListMultimap.create();
}
Expand All @@ -68,7 +70,8 @@ public static JarIndex empty() {
ReferenceIndex referenceIndex = new ReferenceIndex();
BridgeMethodIndex bridgeMethodIndex = new BridgeMethodIndex(entryIndex, inheritanceIndex, referenceIndex);
PackageVisibilityIndex packageVisibilityIndex = new PackageVisibilityIndex();
return new JarIndex(entryIndex, inheritanceIndex, referenceIndex, bridgeMethodIndex, packageVisibilityIndex);
InnerClassIndex innerClassIndex = new InnerClassIndex();
return new JarIndex(entryIndex, inheritanceIndex, referenceIndex, bridgeMethodIndex, packageVisibilityIndex, innerClassIndex);
}

public void indexJar(Set<String> classNames, ClassProvider classProvider, ProgressListener progress) {
Expand Down Expand Up @@ -175,6 +178,24 @@ public void indexLambda(MethodDefEntry callerEntry, Lambda lambda, ReferenceTarg
indexers.forEach(indexer -> indexer.indexLambda(callerEntry, lambda, targetType));
}

@Override
public void indexInnerClass(ClassDefEntry classEntry, InnerClassData innerClassData) {
if (classEntry.isJre()) {
return;
}

indexers.forEach(indexer -> indexer.indexInnerClass(classEntry, innerClassData));
}

@Override
public void indexOuterClass(ClassDefEntry classEntry, OuterClassData outerClassData) {
if (classEntry.isJre()) {
return;
}

indexers.forEach(indexer -> indexer.indexOuterClass(classEntry, outerClassData));
}

public EntryIndex getEntryIndex() {
return entryIndex;
}
Expand All @@ -195,6 +216,10 @@ public PackageVisibilityIndex getPackageVisibilityIndex() {
return packageVisibilityIndex;
}

public InnerClassIndex getInnerClassIndex() {
return innerClassIndex;
}

public EntryResolver getEntryResolver() {
return entryResolver;
}
Expand Down
22 changes: 22 additions & 0 deletions enigma/src/main/java/cuchaz/enigma/analysis/index/JarIndexer.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,28 @@ default void indexFieldReference(MethodDefEntry callerEntry, FieldEntry referenc
default void indexLambda(MethodDefEntry callerEntry, Lambda lambda, ReferenceTargetType targetType) {
}

default void indexInnerClass(ClassDefEntry classEntry, InnerClassData innerClassData) {
}

default void indexOuterClass(ClassDefEntry classEntry, OuterClassData outerClassData) {
}

default void processIndex(JarIndex index) {
}

record InnerClassData(String name, String innerName, String outerName, int access) {
public boolean hasInnerName() {
return innerName != null;
}

public boolean hasOuterName() {
return outerName != null;
}
}

record OuterClassData(String owner, String name, String descriptor) {
Copy link
Contributor

Choose a reason for hiding this comment

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

These names are wrong names from ASM, should've been like EnclosingMethodData. Also hasEnclosingMethod is wrong too; it returns that whether the given class is enclosed in a regular method instead of <init> or <clinit> (read the italics in https://docs.oracle.com/javase/specs/jvms/se19/html/jvms-4.html#jvms-4.7.7)

public boolean hasEnclosingMethod() {
return name != null && descriptor != null;
}
}
}