From ea5c57edc4eb858761ad30b7f933915200da7f41 Mon Sep 17 00:00:00 2001 From: Player Date: Mon, 14 Mar 2022 19:44:21 +0100 Subject: [PATCH] Add useResolvedOwners option to swap the owner in member accesses with the resolved class. Original fix by @KabanFriends and reported by @kennytv in #87 --- .../tinyremapper/AsmClassRemapper.java | 51 ++++++++++++++++--- .../fabricmc/tinyremapper/AsmRemapper.java | 17 ++++--- .../fabricmc/tinyremapper/TinyRemapper.java | 18 +++++-- 3 files changed, 69 insertions(+), 17 deletions(-) diff --git a/src/main/java/net/fabricmc/tinyremapper/AsmClassRemapper.java b/src/main/java/net/fabricmc/tinyremapper/AsmClassRemapper.java index cca49f5b..511256f9 100644 --- a/src/main/java/net/fabricmc/tinyremapper/AsmClassRemapper.java +++ b/src/main/java/net/fabricmc/tinyremapper/AsmClassRemapper.java @@ -198,6 +198,10 @@ private static class AsmMethodRemapper extends MethodRemapper { this.inferNameFromSameLvIndex = inferNameFromSameLvIndex; } + private AsmRemapper getRemapper() { + return (AsmRemapper) remapper; + } + @Override public AnnotationVisitor createAnnotationRemapper(String descriptor, AnnotationVisitor annotationVisitor) { return new AsmAnnotationRemapper(descriptor, annotationVisitor, (AsmRemapper) remapper); @@ -240,21 +244,54 @@ public void visitMultiANewArrayInsn(String descriptor, int numDimensions) { } @Override - public void visitFieldInsn(int opcode, String owner, String name, String descriptor) { + public void visitFieldInsn(int opcode, String owner, String name, String desc) { if (checkPackageAccess) { - PackageAccessChecker.checkMember(this.owner, owner, name, descriptor, TrMember.MemberType.FIELD, "field instruction", (AsmRemapper) remapper); + PackageAccessChecker.checkMember(this.owner, owner, name, desc, TrMember.MemberType.FIELD, "field instruction", (AsmRemapper) remapper); + } + + AsmRemapper remapper = getRemapper(); + ClassInstance cls = remapper.getClass(owner); + + if (cls != null) { + MemberInstance member = cls.resolveField(name, desc); + + name = remapper.mapFieldName(cls, member, name, desc); + + if (remapper.tr.useResolvedOwners && member != null && member.getOwner() != cls) { + owner = member.getOwner().getName(); + } } - super.visitFieldInsn(opcode, owner, name, descriptor); + mv.visitFieldInsn(opcode, + remapper.mapType(owner), + name, + remapper.mapDesc(desc)); } @Override - public void visitMethodInsn(int opcode, String owner, String name, String descriptor, boolean isInterface) { + public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean isInterface) { if (checkPackageAccess) { - PackageAccessChecker.checkMember(this.owner, owner, name, descriptor, TrMember.MemberType.METHOD, "method instruction", (AsmRemapper) remapper); + PackageAccessChecker.checkMember(this.owner, owner, name, desc, TrMember.MemberType.METHOD, "method instruction", (AsmRemapper) remapper); + } + + AsmRemapper remapper = getRemapper(); + ClassInstance cls = remapper.getClass(owner); + + if (cls != null) { + MemberInstance member = cls.resolveMethod(name, desc); + + name = remapper.mapMethodName(cls, member, name, desc); + + if (remapper.tr.useResolvedOwners && member != null && member.getOwner() != cls) { + owner = member.getOwner().getName(); + } } - super.visitMethodInsn(opcode, owner, name, descriptor, isInterface); + mv.visitMethodInsn(opcode, + remapper.mapType(owner), + name, + remapper.mapMethodDesc(desc), + isInterface); } @Override @@ -271,7 +308,7 @@ public void visitInvokeDynamicInsn(String name, String descriptor, Handle bootst bootstrapMethodArguments[i] = remapper.mapValue(bootstrapMethodArguments[i]); } - // bypass remapper + // bypass remapper, TODO: implement useResolvedOwners mv.visitInvokeDynamicInsn(name, remapper.mapMethodDesc(descriptor), (Handle) remapper.mapValue(bootstrapMethodHandle), bootstrapMethodArguments); diff --git a/src/main/java/net/fabricmc/tinyremapper/AsmRemapper.java b/src/main/java/net/fabricmc/tinyremapper/AsmRemapper.java index 35e8d6f6..2fbea5c8 100644 --- a/src/main/java/net/fabricmc/tinyremapper/AsmRemapper.java +++ b/src/main/java/net/fabricmc/tinyremapper/AsmRemapper.java @@ -48,11 +48,12 @@ public String mapFieldName(String owner, String name, String desc) { ClassInstance cls = getClass(owner); if (cls == null) return name; - return mapFieldName(cls, name, desc); + MemberInstance member = cls.resolveField(name, desc); + + return mapFieldName(cls, member, name, desc); } - final String mapFieldName(ClassInstance cls, String name, String desc) { - MemberInstance member = cls.resolve(TrMember.MemberType.FIELD, MemberInstance.getFieldId(name, desc, tr.ignoreFieldDesc)); + final String mapFieldName(ClassInstance cls, MemberInstance member, String name, String desc) { String newName; if (member != null && (newName = member.getNewName()) != null) { @@ -78,11 +79,12 @@ public String mapMethodName(String owner, String name, String desc) { ClassInstance cls = getClass(owner); if (cls == null) return name; // TODO: try to map these from just the mappings?, warn if actual class is missing - return mapMethodName(cls, name, desc); + MemberInstance member = cls.resolveMethod(name, desc); + + return mapMethodName(cls, member, name, desc); } - final String mapMethodName(ClassInstance cls, String name, String desc) { - MemberInstance member = cls.resolve(TrMember.MemberType.METHOD, MemberInstance.getMethodId(name, desc)); + final String mapMethodName(ClassInstance cls, MemberInstance member, String name, String desc) { String newName; if (member != null && (newName = member.getNewName()) != null) { @@ -134,6 +136,7 @@ public String mapMethodArg(String methodOwner, String methodName, String methodD return originatingNewName != null ? originatingNewName : name; } + @Override public String mapMethodVar(String methodOwner, String methodName, String methodDesc, int lvIndex, int startOpIdx, int asmIndex, String name) { return name; // TODO: implement } @@ -148,7 +151,7 @@ public String mapAnnotationAttributeName(final String annotationDesc, final Stri String annotationClass = Type.getType(annotationDesc).getInternalName(); if (attributeDesc == null) { - return this.mapMethodNamePrefixDesc(annotationClass, name, "()"); + return mapMethodNamePrefixDesc(annotationClass, name, "()"); } else { return this.mapMethodName(annotationClass, name, "()" + attributeDesc); } diff --git a/src/main/java/net/fabricmc/tinyremapper/TinyRemapper.java b/src/main/java/net/fabricmc/tinyremapper/TinyRemapper.java index 5ce12f32..81652102 100644 --- a/src/main/java/net/fabricmc/tinyremapper/TinyRemapper.java +++ b/src/main/java/net/fabricmc/tinyremapper/TinyRemapper.java @@ -132,6 +132,14 @@ public Builder resolveMissing(boolean value) { return this; } + /** + * Whether to use the resolved owner class instead of the current class for member accesses. + */ + public Builder useResolvedOwners(boolean value) { + useResolvedOwners = value; + return this; + } + public Builder checkPackageAccess(boolean value) { checkPackageAccess = value; return this; @@ -217,7 +225,7 @@ public TinyRemapper build() { keepInputData, forcePropagation, propagatePrivate, propagateBridges, propagateRecordComponents, - removeFrames, ignoreConflicts, resolveMissing, checkPackageAccess || fixPackageAccess, fixPackageAccess, + removeFrames, ignoreConflicts, resolveMissing, useResolvedOwners, checkPackageAccess || fixPackageAccess, fixPackageAccess, rebuildSourceFilenames, skipLocalMapping, renameInvalidLocals, invalidLvNamePattern, inferNameFromSameLvIndex, analyzeVisitors, stateProcessors, preApplyVisitors, postApplyVisitors, extraRemapper); @@ -236,6 +244,7 @@ public TinyRemapper build() { private boolean removeFrames = false; private boolean ignoreConflicts = false; private boolean resolveMissing = false; + private boolean useResolvedOwners = false; private boolean checkPackageAccess = false; private boolean fixPackageAccess = false; private boolean rebuildSourceFilenames = false; @@ -274,6 +283,7 @@ private TinyRemapper(Collection mappingProviders, boolean igno boolean removeFrames, boolean ignoreConflicts, boolean resolveMissing, + boolean useResolvedOwners, boolean checkPackageAccess, boolean fixPackageAccess, boolean rebuildSourceFilenames, @@ -294,6 +304,7 @@ private TinyRemapper(Collection mappingProviders, boolean igno this.removeFrames = removeFrames; this.ignoreConflicts = ignoreConflicts; this.resolveMissing = resolveMissing; + this.useResolvedOwners = useResolvedOwners; this.checkPackageAccess = checkPackageAccess; this.fixPackageAccess = fixPackageAccess; this.rebuildSourceFilenames = rebuildSourceFilenames; @@ -1089,10 +1100,10 @@ private byte[] fixClass(ClassInstance cls, byte[] data) { String mappedName, mappedDesc; if (member.type == TrMember.MemberType.FIELD) { - mappedName = remapper.mapFieldName(cls, member.name, member.desc); + mappedName = remapper.mapFieldName(cls, member, member.name, member.desc); mappedDesc = remapper.mapDesc(member.desc); } else { - mappedName = remapper.mapMethodName(cls, member.name, member.desc); + mappedName = remapper.mapMethodName(cls, member, member.name, member.desc); mappedDesc = remapper.mapMethodDesc(member.desc); } @@ -1311,6 +1322,7 @@ public void propagate(TrMember m, String newName) { private final boolean removeFrames; private final boolean ignoreConflicts; private final boolean resolveMissing; + final boolean useResolvedOwners; private final boolean checkPackageAccess; private final boolean fixPackageAccess; private final boolean rebuildSourceFilenames;