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.

Sign up for GitHub

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

Fix mapping stats not skipping anonymous classes #469

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f992016
Allow filtering of packages in Mapping Stats window
OroArmor Dec 9, 2021
2265309
Fix mapping stats not skipping anonymous classes
IotaBread Mar 15, 2022
fd22417
Fix checkstyle
NebelNidas Sep 24, 2022
9cd4b78
Improve variable name
NebelNidas Sep 24, 2022
68a8078
Revert "Allow filtering of packages in Mapping Stats window"
NebelNidas Sep 26, 2022
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
30 changes: 25 additions & 5 deletions enigma-swing/src/main/java/cuchaz/enigma/gui/dialog/StatsDialog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ public static void show(Gui gui) {
results.put(member, statsGenerator.generate(listener, Collections.singleton(member), "", false));
}

SwingUtilities.invokeLater(() -> show(gui, results));
SwingUtilities.invokeLater(() -> show(gui, results, ""));
});
}

public static void show(Gui gui, Map<StatsMember, StatsResult> results) {
public static void show(Gui gui, Map<StatsMember, StatsResult> results, String packageName) {
// init frame
JDialog dialog = new JDialog(gui.getFrame(), I18n.translate("menu.file.stats.title"), true);
JDialog dialog = new JDialog(gui.getFrame(), packageName.isEmpty() ? I18n.translate("menu.file.stats.title") : I18n.translateFormatted("menu.file.stats.title_filtered", packageName), true);
Container contentPane = dialog.getContentPane();
contentPane.setLayout(new GridBagLayout());

Expand Down Expand Up @@ -80,10 +80,30 @@ public static void show(Gui gui, Map<StatsMember, StatsResult> results) {
topLevelPackage.setText(UiConfig.getLastTopLevelPackage());
contentPane.add(topLevelPackage, cb1.pos(0, results.size() + 2).fill(GridBagConstraints.HORIZONTAL).build());

// Show filter button
JButton filterButton = new JButton(I18n.translate("menu.file.stats.filter"));
filterButton.addActionListener(action -> {
dialog.dispose();
ProgressDialog.runOffThread(gui.getFrame(), listener -> {
UiConfig.setLastTopLevelPackage(topLevelPackage.getText());
UiConfig.save();

final StatsGenerator statsGenerator = new StatsGenerator(gui.getController().project);
final Map<StatsMember, StatsResult> _results = new HashMap<>();
NebelNidas marked this conversation as resolved.
Show resolved Hide resolved

for (StatsMember member : StatsMember.values()) {
_results.put(member, statsGenerator.generate(listener, Collections.singleton(member), UiConfig.getLastTopLevelPackage(), false));
}

SwingUtilities.invokeLater(() -> show(gui, _results, UiConfig.getLastTopLevelPackage()));
});
});
contentPane.add(filterButton, cb1.pos(0, results.size() + 3).anchor(GridBagConstraints.EAST).build());

// show synthetic members option
JCheckBox syntheticParametersOption = new JCheckBox(I18n.translate("menu.file.stats.synthetic_parameters"));
syntheticParametersOption.setSelected(UiConfig.shouldIncludeSyntheticParameters());
contentPane.add(syntheticParametersOption, cb1.pos(0, results.size() + 3).build());
contentPane.add(syntheticParametersOption, cb1.pos(0, results.size() + 4).build());

// show generate button
JButton button = new JButton(I18n.translate("menu.file.stats.generate"));
Expand All @@ -98,7 +118,7 @@ public static void show(Gui gui, Map<StatsMember, StatsResult> results) {
generateStats(gui, checkboxes, topLevelPackage.getText(), syntheticParametersOption.isSelected());
});

contentPane.add(button, cb1.pos(0, results.size() + 4).weightY(1.0).anchor(GridBagConstraints.SOUTHEAST).build());
contentPane.add(button, cb1.pos(0, results.size() + 5).weightY(1.0).anchor(GridBagConstraints.SOUTHWEST).build());

// add action listener to each checkbox
checkboxes.forEach((key, value) -> value.addActionListener(action -> {
Expand Down
38 changes: 33 additions & 5 deletions enigma-swing/src/main/java/cuchaz/enigma/gui/stats/StatsGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,20 @@ public StatsResult generate(ProgressListener progress, Set<StatsMember> included
progress.init(totalWork, I18n.translate("progress.stats"));

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

String topLevelPackageSlash = topLevelPackage.replace(".", "/");
int numDone = 0;

if (includedMembers.contains(StatsMember.METHODS) || includedMembers.contains(StatsMember.PARAMETERS)) {
for (MethodEntry method : entryIndex.getMethods()) {
progress.step(numDone++, I18n.translate("type.methods"));
MethodEntry root = entryResolver.resolveEntry(method, ResolutionStrategy.RESOLVE_ROOT).stream().findFirst().orElseThrow(AssertionError::new);

if (root == method) {
MethodEntry root = entryResolver
.resolveEntry(method, ResolutionStrategy.RESOLVE_ROOT)
.stream()
.findFirst()
.orElseThrow(AssertionError::new);
ClassEntry clazz = root.getParent();

if (root == method && this.mapper.deobfuscate(clazz).getPackageName().startsWith(topLevelPackageSlash)) {
if (includedMembers.contains(StatsMember.METHODS) && !((MethodDefEntry) method).getAccess().isSynthetic()) {
update(counts, method);
totalMappable++;
Expand All @@ -81,6 +86,29 @@ public StatsResult generate(ProgressListener progress, Set<StatsMember> included
}
}

if (includedMembers.contains(StatsMember.FIELDS)) {
for (FieldEntry field : entryIndex.getFields()) {
progress.step(numDone++, I18n.translate("type.fields"));
ClassEntry clazz = field.getParent();

if (!((FieldDefEntry) field).getAccess().isSynthetic() && this.mapper.deobfuscate(clazz).getPackageName().startsWith(topLevelPackageSlash)) {
update(counts, field);
totalMappable++;
}
}
}

if (includedMembers.contains(StatsMember.CLASSES)) {
for (ClassEntry clazz : entryIndex.getClasses()) {
progress.step(numDone++, I18n.translate("type.classes"));

if (this.mapper.deobfuscate(clazz).getPackageName().startsWith(topLevelPackageSlash)) {
update(counts, clazz);
totalMappable++;
}
}
}

if (includedMembers.contains(StatsMember.FIELDS)) {
for (FieldEntry field : entryIndex.getFields()) {
progress.step(numDone++, I18n.translate("type.fields"));
Expand Down Expand Up @@ -115,7 +143,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
View file
Open in desktop
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
2 changes: 2 additions & 0 deletions enigma/src/main/java/cuchaz/enigma/analysis/index/EntryIndex.java
View file
Open in desktop
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
14 changes: 14 additions & 0 deletions enigma/src/main/java/cuchaz/enigma/analysis/index/IndexClassVisitor.java
View file
Open in desktop
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
54 changes: 54 additions & 0 deletions enigma/src/main/java/cuchaz/enigma/analysis/index/InnerClassIndex.java
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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;
}
}
}
2 changes: 2 additions & 0 deletions enigma/src/main/resources/lang/en_us.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
"menu.file.export.jar": "Export Jar...",
"menu.file.stats": "Mapping Stats...",
"menu.file.stats.title": "Mapping Stats",
"menu.file.stats.title_filtered": "Mapping Stats for %s",
"menu.file.stats.filter": "Filter",
"menu.file.stats.top_level_package": "Top-Level Package:",
"menu.file.stats.synthetic_parameters": "Include Synthetic Parameters",
"menu.file.stats.generate": "Generate Diagram",
Expand Down