Fix more weird edge cases in SrgMerger

This commit is contained in:
Juuz
2022-01-30 18:15:22 +02:00
parent a8a91164c4
commit 3776db3a5a
7 changed files with 170 additions and 7 deletions

View File

@@ -1,7 +1,7 @@
/*
* This file is part of fabric-loom, licensed under the MIT License (MIT).
*
* Copyright (c) 2020-2021 FabricMC
* Copyright (c) 2020-2022 FabricMC
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
@@ -25,6 +25,7 @@
package net.fabricmc.loom.util.function;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.function.Function;
@@ -72,4 +73,18 @@ public final class CollectionUtil {
return result;
}
/**
* Filters a list based on a predicate.
*
* @param collection the source collection
* @param filter the filtering predicate
* @param <A> the element type
* @return a mutable list with the filtered entries
*/
public static <A> List<A> filter(Collection<? extends A> collection, Predicate<? super A> filter) {
ArrayList<A> result = new ArrayList<>(collection);
result.removeIf(filter.negate());
return result;
}
}

View File

@@ -28,10 +28,17 @@ import java.io.BufferedReader;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Consumer;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ListMultimap;
import org.jetbrains.annotations.Nullable;
import net.fabricmc.loom.api.mappings.layered.MappingsNamespace;
@@ -50,17 +57,19 @@ import net.fabricmc.mappingio.tree.MappingTreeView;
import net.fabricmc.mappingio.tree.MemoryMappingTree;
/**
* Utilities for merging SRG mappings.
* Merges a Tiny file with an SRG file.
*
* @author Juuz
*/
public final class SrgMerger {
private static final List<String> INPUT_NAMESPACES = List.of("official", "intermediary", "named");
private final MemoryMappingTree srg;
private final MemoryMappingTree src;
private final MemoryMappingTree output;
private final FlatMappingVisitor flatOutput;
private final boolean lenient;
private final @Nullable MemoryMappingTree extra;
private final ListMultimap<SrgMethodKey, MethodData> methodsBySrgName;
private SrgMerger(Path srg, Path tiny, @Nullable ExtraMappings extraMappings, boolean lenient) throws IOException {
this.srg = readSrg(srg);
@@ -68,6 +77,7 @@ public final class SrgMerger {
this.output = new MemoryMappingTree();
this.flatOutput = new RegularAsFlatMappingVisitor(output);
this.lenient = lenient;
this.methodsBySrgName = ArrayListMultimap.create();
if (extraMappings != null) {
this.extra = new MemoryMappingTree();
@@ -88,14 +98,20 @@ public final class SrgMerger {
}
MappingReader.read(tiny, this.src);
if (!"official".equals(this.src.getSrcNamespace())) {
throw new MappingException("Mapping file " + tiny + " does not have the 'official' namespace as the default!");
}
checkInputNamespaces(tiny);
this.output.visitNamespaces(this.src.getSrcNamespace(), Stream.concat(Stream.of("srg"), this.src.getDstNamespaces().stream()).collect(Collectors.toList()));
}
private void checkInputNamespaces(Path tiny) {
List<String> inputNamespaces = new ArrayList<>(this.src.getDstNamespaces());
inputNamespaces.add(0, this.src.getSrcNamespace());
if (!inputNamespaces.equals(INPUT_NAMESPACES)) {
throw new MappingException("Mapping file " + tiny + " does not have 'official, intermediary, named' as its namespaces! Found: " + inputNamespaces);
}
}
/**
* Creates a destination name array where the first element
* will be the srg name of the mapping element.
@@ -149,6 +165,8 @@ public final class SrgMerger {
}
}
resolveConflicts();
return output;
}
@@ -184,6 +202,7 @@ public final class SrgMerger {
private void mergeMethod(MappingTree.ClassMapping srgClass, MappingTree.MethodMapping srgMethod, @Nullable MappingTree.ClassMapping tinyClass) throws IOException {
String[] dstNames = createDstNameArray(srgMethod);
MappingTree.MethodMapping tinyMethod = null;
String intermediaryName, namedName;
if (tinyClass != null) {
tinyMethod = tinyClass.getMethod(srgMethod.getSrcName(), srgMethod.getSrcDesc());
@@ -193,6 +212,8 @@ public final class SrgMerger {
if (tinyMethod != null) {
copyDstNames(dstNames, tinyMethod);
intermediaryName = tinyMethod.getName("intermediary");
namedName = tinyMethod.getName("named");
} else {
if (srgMethod.getSrcName().equals(srgMethod.getDstName(0))) {
// These are only methods like <init> or toString which have the same name in every NS.
@@ -212,6 +233,7 @@ public final class SrgMerger {
if (fillMethod != null) {
fillMappings(dstNames, fillMethod);
intermediaryName = namedName = fillMethod.getSrcName();
} else {
// Do not allow missing methods as these are typically subclass methods and cause issues where
// class B extends A, and overrides a method from the superclass. Then the subclass method
@@ -220,12 +242,97 @@ public final class SrgMerger {
}
}
if (!srgMethod.getSrcName().equals(dstNames[0])) { // ignore <init> and the likes
methodsBySrgName.put(
new SrgMethodKey(dstNames[0], srgMethod.getSrcDesc()),
new MethodData(srgClass.getSrcName(), srgMethod.getSrcName(), srgMethod.getSrcDesc(), tinyMethod != null, intermediaryName, namedName)
);
}
flatOutput.visitMethod(srgClass.getSrcName(), srgMethod.getSrcName(), srgMethod.getSrcDesc(), dstNames);
}
/**
* Resolves conflicts where multiple methods map to an SRG method.
* We will prefer the ones with the Tiny mappings.
*/
private void resolveConflicts() {
List<String> conflicts = new ArrayList<>();
for (SrgMethodKey srg : methodsBySrgName.keySet()) {
List<MethodData> methods = methodsBySrgName.get(srg);
if (methods.size() == 1) continue;
// Determine whether the names conflict
Set<String> foundNamedNames = new HashSet<>();
for (MethodData method : methods) {
foundNamedNames.add(method.namedName());
}
if (foundNamedNames.size() == 1) {
// No conflict, go on
continue;
}
// Find preferred method
@Nullable MethodData preferred = findPreferredMethod(methods, conflicts::add);
if (preferred == null) continue;
// Remove non-preferred methods
for (MethodData method : methods) {
if (method != preferred) {
MappingTree.ClassMapping clazz = output.getClass(method.obfOwner());
clazz.getMethods().removeIf(m -> m.getSrcName().equals(method.obfName()) && m.getSrcDesc().equals(method.obfDesc()));
}
}
}
if (!conflicts.isEmpty()) {
throw new MappingException("Unfixable conflicts:\n" + String.join("\n", conflicts));
}
}
private @Nullable MethodData findPreferredMethod(List<MethodData> methods, Consumer<String> conflictReporter) {
List<MethodData> hasTiny = CollectionUtil.filter(methods, MethodData::hasTiny);
// Record conflicts if needed
if (hasTiny.size() > 1) { // Multiple methods map to this SRG name
// Sometimes unrelated methods share an SRG name. Probably overrides from a past version?
Set<String> intermediaryNames = new HashSet<>();
for (MethodData method : methods) {
intermediaryNames.add(method.intermediaryName());
}
// Only record a conflict if we map one intermediary name with multiple named names
if (intermediaryNames.size() == 1) {
StringBuilder message = new StringBuilder();
message.append("- multiple preferred methods for ").append(srg).append(':');
for (MethodData preferred : hasTiny) {
message.append("\n\t> ").append(preferred);
}
conflictReporter.accept(message.toString());
}
return null;
} else if (hasTiny.isEmpty()) { // No methods map to this SRG name
conflictReporter.accept("- no preferred methods found for " + srg + ", available: " + methods);
return null;
}
return hasTiny.get(0);
}
/**
* Merges SRG mappings with a tiny mappings tree through the obf names.
*
* <p>The namespaces in the tiny file should be {@code official, intermediary, named}.
* The SRG names will add a new namespace called {@code srg} so that the final namespaces
* are {@code official, srg, intermediary, named}.
*
* <p>This method does not care about method parameters, local variables or javadoc comments.
* As such, it shouldn't be used for remapping the game jar.
*
@@ -239,7 +346,7 @@ public final class SrgMerger {
* whether an unobfuscated name is needed in the result file
* @param lenient whether lenient mode is enabled
* @throws IOException if an IO error occurs while reading or writing the mappings
* @throws MappingException if the input tiny tree's default namespace is not 'official'
* @throws MappingException if the input tiny mappings' namespaces are incorrect
* or if an element mentioned in the SRG file does not have tiny mappings in non-lenient mode
*/
public static void mergeSrg(Path srg, Path tiny, Path out, @Nullable ExtraMappings extraMappings, boolean lenient)
@@ -276,4 +383,18 @@ public final class SrgMerger {
return new ExtraMappings(path, MappingFormat.TSRG2, MappingsNamespace.OFFICIAL.toString(), MappingsNamespace.NAMED.toString());
}
}
private record SrgMethodKey(String name, String desc) {
@Override
public String toString() {
return name + desc;
}
}
private record MethodData(String obfOwner, String obfName, String obfDesc, boolean hasTiny, String intermediaryName, String namedName) {
@Override
public String toString() {
return "%s.%s%s => %s/%s (%s)".formatted(obfOwner, obfName, obfDesc, intermediaryName, namedName, hasTiny ? "tiny" : "filled");
}
}
}

View File

@@ -2,6 +2,11 @@ tiny 2 0 official srg intermediary named
c a test/FakeClass class_1 test/SomeClass
f I a f_1_ field_1 myInt
m ()I a m_1_ method_1 getMyInt
m ()V evilUnobfMethod m_3_ method_2 differentNamedName
c b test/FakeSubclass class_2 test/SomeSubclass
c test/DeObfClass test/DeObfClass test/DeObfClass test/DeObfClass
m ()V iAmNotObfuscated m_2_ iAmNotObfuscated iAmNotObfuscated
c c test/FormerSiblingA class_3 test/RelatedA
m ()V a m_4_ method_3 fromClassA
c d test/FormerSiblingB class_4 test/RelatedB
m ()V a m_4_ method_4 fromClassB

View File

@@ -3,8 +3,13 @@ a test/FakeClass
<init> ()V <init>
a I fakeField
a ()I getFakeField
evilUnobfMethod ()V evilUnobfMethod
b test/FakeSubclass
<init> ()V <init>
test/DeObfClass test/DeObfClass
<init> ()V <init>
iAmNotObfuscated ()V iAmNotObfuscated
c test/FormerSiblingA
a ()V something
d test/FormerSiblingB
a ()V something

View File

@@ -2,8 +2,14 @@ test.FakeClass -> a:
void <init>() -> <init>
int fakeField -> a
int getFakeField() -> a
void evilUnobfMethod() -> evilUnobfMethod
test.FakeSubclass -> b:
void <init>() -> <init>
void evilUnobfMethod() -> evilUnobfMethod
test.DeObfClass -> test.DeObfClass:
void <init>() -> <init>
void iAmNotObfuscated() -> iAmNotObfuscated
test.FormerSiblingA -> c:
void something() -> a
test.FormerSiblingB -> d:
void something() -> a

View File

@@ -3,9 +3,15 @@ a test/FakeClass
<init> ()V <init>
a I f_1_
a ()I m_1_
evilUnobfMethod ()V m_3_
b test/FakeSubclass
<init> ()V <init>
a ()I m_1_
evilUnobfMethod ()V m_3_
test/DeObfClass test/DeObfClass
<init> ()V <init>
iAmNotObfuscated ()V m_2_
c test/FormerSiblingA
a ()V m_4_
d test/FormerSiblingB
a ()V m_4_

View File

@@ -2,4 +2,9 @@ tiny 2 0 official intermediary named
c a class_1 test/SomeClass
f I a field_1 myInt
m ()I a method_1 getMyInt
m ()V evilUnobfMethod method_2 differentNamedName
c b class_2 test/SomeSubclass
c c class_3 test/RelatedA
m ()V a method_3 fromClassA
c d class_4 test/RelatedB
m ()V a method_4 fromClassB