Skip to content

Commit

Permalink
fix(mappings): try to prevent mapping file reset on export exception,…
Browse files Browse the repository at this point in the history
… refactor and fix code to avoid NPE (#2220)(#2226)
  • Loading branch information
skylot committed Jul 25, 2024
1 parent a8d889d commit ec645d8
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 67 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,12 @@
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.AbstractMap.SimpleEntry;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;

import org.jetbrains.annotations.Nullable;
import org.slf4j.Logger;
Expand All @@ -27,17 +23,12 @@
import net.fabricmc.mappingio.tree.VisitOrder;
import net.fabricmc.mappingio.tree.VisitableMappingTree;

import jadx.api.ICodeInfo;
import jadx.api.data.ICodeComment;
import jadx.api.data.ICodeRename;
import jadx.api.data.IJavaNodeRef.RefType;
import jadx.api.data.impl.JadxCodeData;
import jadx.api.data.impl.JadxCodeRef;
import jadx.api.metadata.ICodeNodeRef;
import jadx.api.metadata.annotations.InsnCodeOffset;
import jadx.api.metadata.annotations.NodeDeclareRef;
import jadx.api.metadata.annotations.VarNode;
import jadx.api.utils.CodeUtils;
import jadx.core.Consts;
import jadx.core.codegen.TypeGen;
import jadx.core.dex.info.ClassInfo;
Expand All @@ -50,6 +41,7 @@
import jadx.core.utils.files.FileUtils;
import jadx.plugins.mappings.RenameMappingsData;
import jadx.plugins.mappings.utils.DalvikToJavaBytecodeUtils;
import jadx.plugins.mappings.utils.VariablesUtils;

public class MappingExporter {
private static final Logger LOG = LoggerFactory.getLogger(MappingExporter.class);
Expand All @@ -62,44 +54,6 @@ public MappingExporter(RootNode root) {
this.loadedMappingTree = RenameMappingsData.getTree(this.root);
}

private List<Entry<VarNode, Entry<Integer, Integer>>> collectMethodVars(MethodNode methodNode) {
ICodeInfo codeInfo = methodNode.getTopParentClass().getCode();
int mthDefPos = methodNode.getDefPosition();
int mthLineEndPos = CodeUtils.getLineEndForPos(codeInfo.getCodeStr(), mthDefPos);

List<Entry<VarNode, Entry<Integer, Integer>>> vars = new ArrayList<>();
AtomicInteger lastOffset = new AtomicInteger(-1);
codeInfo.getCodeMetadata().searchDown(mthLineEndPos, (pos, ann) -> {
if (ann instanceof InsnCodeOffset) {
lastOffset.set(((InsnCodeOffset) ann).getOffset());
}
if (ann instanceof NodeDeclareRef) {
ICodeNodeRef declRef = ((NodeDeclareRef) ann).getNode();
if (declRef instanceof VarNode) {
VarNode varNode = (VarNode) declRef;
if (!varNode.getMth().equals(methodNode)) { // Stop if we've gone too far and have entered a different method
if (!vars.isEmpty()) {
vars.get(vars.size() - 1).getValue().setValue(declRef.getDefPosition() - 1);
}
return Boolean.TRUE;
}
if (lastOffset.get() != -1) {
if (!vars.isEmpty()) {
vars.get(vars.size() - 1).getValue().setValue(lastOffset.get() - 1);
}
vars.add(new SimpleEntry<VarNode, Entry<Integer, Integer>>(varNode, new SimpleEntry<>(lastOffset.get(), null)));
} else {
LOG.warn("Local variable not present in bytecode, skipping: "
+ methodNode.getMethodInfo().getRawFullId() + "#" + varNode.getName());
}
lastOffset.set(-1);
}
}
return null;
});
return vars;
}

public void exportMappings(Path path, JadxCodeData codeData, MappingFormat mappingFormat) {
VisitableMappingTree mappingTree = new MemoryMappingTree();
// Map < SrcName >
Expand Down Expand Up @@ -142,14 +96,6 @@ public void exportMappings(Path path, JadxCodeData codeData, MappingFormat mappi
}

try {
if (mappingFormat.hasSingleFile()) {
FileUtils.deleteFileIfExists(path);
FileUtils.makeDirsForFile(path);
Files.createFile(path);
} else {
FileUtils.makeDirs(path);
}

String srcNamespace = MappingUtil.NS_SOURCE_FALLBACK;
String dstNamespace = MappingUtil.NS_TARGET_FALLBACK;

Expand Down Expand Up @@ -230,16 +176,11 @@ public void exportMappings(Path path, JadxCodeData codeData, MappingFormat mappi
// Not checking for comments since method args can't have any
}
// Method vars
var vars = collectMethodVars(mth);
for (int i = 0; i < vars.size(); i++) {
var entry = vars.get(i);
VarNode var = entry.getKey();
int startOpIdx = entry.getValue().getKey();
int endOpIdx = entry.getValue().getValue();
Integer lvIndex = DalvikToJavaBytecodeUtils.getMethodVarLvIndex(var);
if (lvIndex == null) {
lvIndex = -1;
}
for (VariablesUtils.VarInfo info : VariablesUtils.collect(mth)) {
VarNode var = info.getVar();
int startOpIdx = info.getStartOpIdx();
int endOpIdx = info.getEndOpIdx();
int lvIndex = DalvikToJavaBytecodeUtils.getMethodVarLvIndex(var);
String key = rawClassName + methodInfo.getShortId()
+ JadxCodeRef.forVar(var.getReg(), var.getSsa());
if (mappedMethodArgsAndVars.containsKey(key)) {
Expand All @@ -255,10 +196,18 @@ public void exportMappings(Path path, JadxCodeData codeData, MappingFormat mappi
}
}
}
// write file as late as possible because a mapping collection can fail with exception
if (mappingFormat.hasSingleFile()) {
FileUtils.deleteFileIfExists(path);
FileUtils.makeDirsForFile(path);
Files.createFile(path);
} else {
FileUtils.makeDirs(path);
}
// Write file
mappingTree.accept(MappingWriter.create(path, mappingFormat), VisitOrder.createByName());
mappingTree.visitEnd();
} catch (IOException e) {
} catch (Exception e) {
LOG.error("Failed to save deobfuscation map file '{}'", path.toAbsolutePath(), e);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ private static Integer getMethodArgLvIndexViaSsaVars(int regNum, MethodNode mth)

// Method vars

public static Integer getMethodVarLvIndex(VarNode methodVar) {
public static int getMethodVarLvIndex(VarNode methodVar) {
MethodNode mth = methodVar.getMth();
Integer lvIndex = getMethodVarLvIndexViaSsaVars(methodVar.getReg(), mth);
if (lvIndex != null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
package jadx.plugins.mappings.utils;

import java.util.ArrayList;
import java.util.List;

import org.jetbrains.annotations.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import jadx.api.ICodeInfo;
import jadx.api.metadata.ICodeAnnotation;
import jadx.api.metadata.ICodeNodeRef;
import jadx.api.metadata.annotations.InsnCodeOffset;
import jadx.api.metadata.annotations.NodeDeclareRef;
import jadx.api.metadata.annotations.VarNode;
import jadx.api.utils.CodeUtils;
import jadx.core.dex.nodes.MethodNode;

public class VariablesUtils {
private static final Logger LOG = LoggerFactory.getLogger(VariablesUtils.class);

public static class VarInfo {
private final VarNode var;
private final int startOpIdx;
private int endOpIdx;

public VarInfo(VarNode var, int startOpIdx) {
this.var = var;
this.startOpIdx = startOpIdx;
this.endOpIdx = startOpIdx;
}

public VarNode getVar() {
return var;
}

public int getStartOpIdx() {
return startOpIdx;
}

public int getEndOpIdx() {
return endOpIdx;
}

public void setEndOpIdx(int endOpIdx) {
this.endOpIdx = endOpIdx;
}
}

public static List<VarInfo> collect(MethodNode mth) {
ICodeInfo codeInfo = mth.getTopParentClass().getCode();
int mthDefPos = mth.getDefPosition();
int mthLineEndPos = CodeUtils.getLineEndForPos(codeInfo.getCodeStr(), mthDefPos);
CodeVisitor codeVisitor = new CodeVisitor(mth);
codeInfo.getCodeMetadata().searchDown(mthLineEndPos, codeVisitor::process);
return codeVisitor.getVars();
}

private static class CodeVisitor {
private final MethodNode mth;
private final List<VarInfo> vars = new ArrayList<>();
private int lastOffset = -1;

public CodeVisitor(MethodNode mth) {
this.mth = mth;
}

public @Nullable Boolean process(Integer pos, ICodeAnnotation ann) {
if (ann instanceof InsnCodeOffset) {
lastOffset = ((InsnCodeOffset) ann).getOffset();
}
if (ann instanceof NodeDeclareRef) {
ICodeNodeRef declRef = ((NodeDeclareRef) ann).getNode();
if (declRef instanceof VarNode) {
VarNode varNode = (VarNode) declRef;
if (!varNode.getMth().equals(mth)) { // Stop if we've gone too far and have entered a different method
if (!vars.isEmpty()) {
vars.get(vars.size() - 1).setEndOpIdx(declRef.getDefPosition() - 1);
}
return Boolean.TRUE;
}
if (lastOffset != -1) {
if (!vars.isEmpty()) {
vars.get(vars.size() - 1).setEndOpIdx(lastOffset - 1);
}
vars.add(new VarInfo(varNode, lastOffset));
} else {
LOG.warn("Local variable not present in bytecode, skipping: {}#{}",
mth.getMethodInfo().getRawFullId(), varNode.getName());
}
lastOffset = -1;
}
}
return null;
}

public List<VarInfo> getVars() {
return vars;
}
}
}

1 comment on commit ec645d8

@NebelNidas
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should remove the local variable mapping exporting entirely, as it's neither spec-compliant (calculating the lvIndex via decompiled code, not bytecode; and opcode-indices also being tied to the decomp rather than bytecode), nor can JADX actually import them afterwards anyway

Please sign in to comment.