Skip to content

Commit

Permalink
fix: Unhandled reused variable indices not being flattened.
Browse files Browse the repository at this point in the history
While not a proper fix, it does allow editing of methods previously unsupported due to reused variables. This is unfortunately a lossy process until proper scope support can be added
  • Loading branch information
Col-E committed Dec 11, 2020
1 parent afeff3a commit a9c2bac
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 5 deletions.
51 changes: 50 additions & 1 deletion src/main/java/me/coley/recaf/parse/bytecode/Disassembler.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ private void setup(MethodNode value) {
// Ensure there is a label before the first variable instruction and after the last usage.
enforceLabelRanges(value);
// Validate each named variable has the same type.
splitSameIndexedVariablesOfDiffNames(value);
// Validate each named variable has the same type.
splitSameNamedVariablesOfDiffTypes(value);
// Input validation
if (value.instructions == null)
Expand Down Expand Up @@ -607,7 +609,43 @@ public static void splitSameNamedVariablesOfDiffTypes(MethodNode node) {
}
// Logging
if (changed) {
Log.warn("Replacing confusing variable names in disassembly: " + node.name + node.desc);
Log.warn("Separating variables of same name pointing to different indices: " + node.name + node.desc);
}
}

/**
* Reallocates variable indices of variables that share the same index but names.
* This is done to prevent type conflicts of variables by the same index in different scopes.
* Recaf does not understand variable scope at the moment,
* so this is a hack to be removed in the future when it does.
*
* @param node
* Method to update.
*
* @see VariableGenerator Place to add scoped variable support later.
*/
public static void splitSameIndexedVariablesOfDiffNames(MethodNode node) {
if (node.localVariables == null)
return;
Map<Integer, String> indexToName = new HashMap<>();
boolean changed = false;
int nextFreeVar = computeMavVar(AccessFlag.isStatic(node.access), node.localVariables);
for(LocalVariableNode lvn : node.localVariables) {
int index = lvn.index;
String name = lvn.name;
if(!name.equals(indexToName.getOrDefault(index, name))) {
// Update the variables index and bump the next free index
index = lvn.index = nextFreeVar;
nextFreeVar += Type.getType(lvn.desc).getSize();
indexToName.put(index, lvn.name);
changed = true;
} else {
indexToName.put(index, name);
}
}
// Logging
if (changed) {
Log.warn("Separating variables of same index reusing the same name: " + node.name + node.desc);
}
}

Expand All @@ -629,6 +667,17 @@ private static String nextIndexName(String name, MethodNode node) {
return tmp[0];
}

private static int computeMavVar(boolean isStatic, List<LocalVariableNode> vars) {
int max = isStatic ? 0 : 1;
for (LocalVariableNode lvn : vars) {
if (lvn.index >= max) {
max = lvn.index + Type.getType(lvn.desc).getSize();
}
}
return max;
}


private String name(LabelNode label) {
return labelToName.getOrDefault(label, "?");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,17 @@ void computeVariables(MethodVerifier verifier) throws AssemblerException {
int sort = reference.getVariableSort();
String name = reference.getVariableName().getName();
indexToSort.put(index, sort);
indexToName.put(index, name);
// Only add variable refs that are not numeric
if (!name.matches("\\d+"))
indexToName.put(index, name);
}
// Compute the method argument variables first
for (DefinitionArgAST reference : compilation.getAst().getRoot().search(DefinitionArgAST.class)) {
int index = reference.getVariableIndex(nameCache);
String name = reference.getVariableName().getName();
computeArgument(index, name, reference.getDesc().getDesc());
// Only add variable refs that are not numeric
if (!name.matches("\\d+"))
computeArgument(index, name, reference.getDesc().getDesc());
}
// If a variable index only has one type, this is very easy.
// Otherwise, if it has multiple types we must consider scope...
Expand All @@ -84,7 +88,8 @@ void computeVariables(MethodVerifier verifier) throws AssemblerException {
if (sorts.size() == 1) {
// TODO: Don't pass in the name like this.
// See above note.
computeSimple(index, names.iterator().next());
if (!names.isEmpty())
computeSimple(index, names.iterator().next());
} else {
computeScoped(index, verifier);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,9 @@ public BytecodeEditorPane(GuiController controller, String className, String mem
MethodNode existingMethod = ClassUtil.getMethod(controller.getWorkspace()
.getClassReader(className), 0, memberName, memberDesc);
if (existingMethod != null && existingMethod.localVariables != null) {
// We call the disassembler's method here so that any changes the disassembler
// We call the disassembler's methods here so that any changes the disassembler
// makes to the local variables is what gets populated as default information
Disassembler.splitSameIndexedVariablesOfDiffNames(existingMethod);
Disassembler.splitSameNamedVariablesOfDiffTypes(existingMethod);
assembler.setDefaultVariables(existingMethod.localVariables);
}
Expand Down

0 comments on commit a9c2bac

Please sign in to comment.