Skip to content

Add flag to detect wrong variable merges #477

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

Merged
merged 2 commits into from
Aug 14, 2025
Merged

Add flag to detect wrong variable merges #477

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c4d22d9
Add flag to detect wrong variable merges
Kroppeb May 13, 2025
9fa0435
Copy dec file from ci
Kroppeb May 13, 2025
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
7 changes: 7 additions & 0 deletions src/org/jetbrains/java/decompiler/main/extern/IFernflowerPreferences.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,12 @@ public interface IFernflowerPreferences {
@Type(DecompilerOption.Type.BOOLEAN)
String VERIFY_VARIABLE_MERGES = "verify-merges";

@Name("[Experimental] Verify Pre Post Variable Merges")
@Description("Will try to validate that code before and after variable merges is equivalent")
@ShortName("pvm")
@Type(DecompilerOption.Type.BOOLEAN)
String VERIFY_PRE_POST_VARIABLE_MERGES = "verify-pre-post-merges";

@Name("[Experimental] Use old try deduplication")
@Description("Use the old try deduplication algorithm for methods with obfuscated exceptions, which inserts dummy exception handlers instead of duplicating blocks")
@Type(DecompilerOption.Type.BOOLEAN)
Expand Down Expand Up @@ -437,6 +443,7 @@ static Map<String, Object> getDefaults() {
defaults.put(SHOW_HIDDEN_STATEMENTS, "0"); // Extra debugging that isn't useful in most cases
defaults.put(SIMPLIFY_STACK_SECOND_PASS, "1"); // Generally produces better bytecode, useful to debug if it does something strange
defaults.put(VERIFY_VARIABLE_MERGES, "0"); // Produces more correct code in rare cases, but hurts code cleanliness in the majority of cases. Default off until a better fix is created.
defaults.put(VERIFY_PRE_POST_VARIABLE_MERGES, "0");
defaults.put(OLD_TRY_DEDUP, "0");
defaults.put(DECOMPILE_PREVIEW, "1"); // Preview features are useful to decompile in almost all cases

Expand Down
128 changes: 126 additions & 2 deletions src/org/jetbrains/java/decompiler/modules/decompiler/vars/VarDefinitionHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@
import org.jetbrains.java.decompiler.main.extern.IFernflowerPreferences;
import org.jetbrains.java.decompiler.main.rels.MethodWrapper;
import org.jetbrains.java.decompiler.modules.decompiler.ExprProcessor;
import org.jetbrains.java.decompiler.modules.decompiler.StackVarsProcessor;
import org.jetbrains.java.decompiler.modules.decompiler.ValidationHelper;
import org.jetbrains.java.decompiler.modules.decompiler.exps.*;
import org.jetbrains.java.decompiler.modules.decompiler.flow.DirectGraph;
import org.jetbrains.java.decompiler.modules.decompiler.flow.FlattenStatementsHelper;
import org.jetbrains.java.decompiler.modules.decompiler.sforms.SSAUConstructorSparseEx;
import org.jetbrains.java.decompiler.modules.decompiler.stats.*;
import org.jetbrains.java.decompiler.modules.decompiler.vars.VarTypeProcessor.FinalType;
import org.jetbrains.java.decompiler.struct.StructClass;
Expand Down Expand Up @@ -44,7 +46,7 @@ public class VarDefinitionHelper {

private final VarProcessor varproc;

private final Statement root;
private final RootStatement root;
private final StructMethod mt;
private final Map<VarVersionPair, String> clashingNames = new HashMap<>();

Expand Down Expand Up @@ -519,7 +521,118 @@ private void populateTypeBounds(VarProcessor proc, Statement stat) {
}
}

private VPPEntry mergeVars(Statement stat) {
static class VarID {
final VarExprent var;

VarID(VarExprent var) {
this.var = var;
}

@Override
public int hashCode() {
return System.identityHashCode(var);
}

@Override
public boolean equals(Object obj) {
return obj instanceof VarID varID && var == varID.var;
}
}

private Map<VarID, Set<VarID>> getVarExprentSources() {
// Do an ssau analysis to find the sources of variables
SSAUConstructorSparseEx ssau = new SSAUConstructorSparseEx();
try {
ssau.splitVariables(root, mt);
} catch (NullPointerException t) {
// Can happen when something is wrong with variables ...

StackVarsProcessor.setVersionsToNull(root);
return null;
}

Map<VarVersionPair, VarID> lookup = new HashMap<>();
findAllVarExprents(root, lookup);

Map<VarID, Set<VarID>> sources = new HashMap<>();
for (VarVersionNode node : ssau.getSsuVersions().nodes) {
VarID target = lookup.get(node.asPair());
if (target == null) {
continue;
}

Set<VarID> sourceVars = new HashSet<>();

for (VarVersionNode predecessor : node.getPredecessors()) {
VarID source = lookup.get(predecessor.asPair());
if (source != null) {
sourceVars.add(source);
}
}

if (node.phantomNode != null) {
VarID source = lookup.get(node.phantomNode.asPair());
if (source != null) {
sourceVars.add(source);
}
}

if (!sourceVars.isEmpty()) {
sources.put(target, sourceVars);
}
}

StackVarsProcessor.setVersionsToNull(root);

return sources;
}

private static void findAllVarExprents(Statement stat, Map<VarVersionPair, VarID> lookup) {
for (Exprent exprent : stat.getVarDefinitions()) {
if (exprent instanceof VarExprent varExprent) {
lookup.put(new VarVersionPair(varExprent), new VarID(varExprent));
}
}
List<Exprent> lst = stat.getExprents();
if (lst != null) {
for (Exprent exprent : lst) {
for (Exprent exp : exprent.getAllExprents(true, true)) {
if (exp instanceof VarExprent varExprent) {
lookup.put(new VarVersionPair(varExprent), new VarID(varExprent));
}
}
}
}

for (Statement subStat : stat.getStats()) {
findAllVarExprents(subStat, lookup);
}
}

private void compareVarExprentSources(
Map<VarID, Set<VarID>> oldSources,
Map<VarID, Set<VarID>> newSources
) {
if (newSources == null) return;

for (var oldEntry : oldSources.entrySet()) {
Set<VarID> oldSet = oldEntry.getValue();
Set<VarID> newSet = newSources.get(oldEntry.getKey());

// Check if sets match
if (!Objects.equals(oldSet, newSet)) {
root.addComment("$VF: Variable merging failed for merge " + oldEntry.getKey().var + ". Code has semantic differences!");
}
}

for (var newVar : newSources.keySet()) {
if (!oldSources.containsKey(newVar)) {
root.addComment("$VF: Variable merging added a var? " + newVar.var);
}
}
}

private VPPEntry mergeVars(RootStatement stat) {
Map<Integer, VarVersionPair> parent = new HashMap<>(); // Always empty dua!
MethodDescriptor md = MethodDescriptor.parseDescriptor(mt.getDescriptor());

Expand All @@ -536,6 +649,12 @@ private VPPEntry mergeVars(Statement stat) {

populateTypeBounds(varproc, stat);


Map<VarID, Set<VarID>> sources = null;
if (DecompilerContext.getOption(IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES)) {
sources = getVarExprentSources();
}

Map<VarVersionPair, VarVersionPair> denylist = new HashMap<>();
VPPEntry remap = mergeVars(stat, parent, new HashMap<>(), denylist);
while (remap != null) {
Expand All @@ -546,6 +665,11 @@ private VPPEntry mergeVars(Statement stat) {

remap = mergeVars(stat, parent, new HashMap<>(), denylist);
}

if (sources != null) {
Map<VarID, Set<VarID>> newSources = getVarExprentSources();
compareVarExprentSources(sources, newSources);
}
return null;
}

Expand Down
39 changes: 26 additions & 13 deletions test/org/jetbrains/java/decompiler/SingleClassesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,31 +20,35 @@ protected void registerAll() {
IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES, "1",
IFernflowerPreferences.INCLUDE_ENTIRE_CLASSPATH, "0",
IFernflowerPreferences.TERNARY_CONDITIONS, "1",
IFernflowerPreferences.FORCE_JSR_INLINE, "1"
IFernflowerPreferences.FORCE_JSR_INLINE, "1",
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
);
registerSet("Entire Classpath", this::registerEntireClassPath,
IFernflowerPreferences.BYTECODE_SOURCE_MAPPING, "1",
IFernflowerPreferences.DUMP_ORIGINAL_LINES, "1",
IFernflowerPreferences.DUMP_EXCEPTION_ON_ERROR, "0",
IFernflowerPreferences.IGNORE_INVALID_BYTECODE, "1",
IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES, "1",
IFernflowerPreferences.INCLUDE_ENTIRE_CLASSPATH, "1"
IFernflowerPreferences.INCLUDE_ENTIRE_CLASSPATH, "1",
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
);
registerSet("Java Runtime", this::registerJavaRuntime,
IFernflowerPreferences.BYTECODE_SOURCE_MAPPING, "1",
IFernflowerPreferences.DUMP_ORIGINAL_LINES, "1",
IFernflowerPreferences.DUMP_EXCEPTION_ON_ERROR, "0",
IFernflowerPreferences.IGNORE_INVALID_BYTECODE, "1",
IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES, "1",
IFernflowerPreferences.INCLUDE_JAVA_RUNTIME, "1"
IFernflowerPreferences.INCLUDE_JAVA_RUNTIME, "1",
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
);
registerSet("Literals", this::registerLiterals,
IFernflowerPreferences.BYTECODE_SOURCE_MAPPING, "1",
IFernflowerPreferences.DUMP_ORIGINAL_LINES, "1",
IFernflowerPreferences.DUMP_EXCEPTION_ON_ERROR, "0",
IFernflowerPreferences.IGNORE_INVALID_BYTECODE, "1",
IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES, "1",
IFernflowerPreferences.LITERALS_AS_IS, "0"
IFernflowerPreferences.LITERALS_AS_IS, "0",
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
);
registerSet("Pattern Matching", this::registerPatternMatching,
IFernflowerPreferences.BYTECODE_SOURCE_MAPPING, "1",
Expand All @@ -53,7 +57,8 @@ protected void registerAll() {
IFernflowerPreferences.IGNORE_INVALID_BYTECODE, "1",
IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES, "1",
IFernflowerPreferences.INCLUDE_ENTIRE_CLASSPATH, "0",
IFernflowerPreferences.PATTERN_MATCHING, "1"
IFernflowerPreferences.PATTERN_MATCHING, "1",
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
);
registerSet("Ternary Constant Simplification", this::registerTernaryConstantSimplification,
IFernflowerPreferences.BYTECODE_SOURCE_MAPPING, "1",
Expand All @@ -62,7 +67,8 @@ protected void registerAll() {
IFernflowerPreferences.IGNORE_INVALID_BYTECODE, "1",
IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES, "1",
IFernflowerPreferences.LITERALS_AS_IS, "0",
IFernflowerPreferences.TERNARY_CONSTANT_SIMPLIFICATION, "1"
IFernflowerPreferences.TERNARY_CONSTANT_SIMPLIFICATION, "1",
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
);
registerSet("LVT", this::registerLVT,
IFernflowerPreferences.DECOMPILE_INNER, "1",
Expand All @@ -71,7 +77,8 @@ protected void registerAll() {
IFernflowerPreferences.ASCII_STRING_CHARACTERS, "1",
IFernflowerPreferences.REMOVE_SYNTHETIC, "1",
IFernflowerPreferences.REMOVE_BRIDGE, "1",
IFernflowerPreferences.USE_DEBUG_VAR_NAMES, "1"
IFernflowerPreferences.USE_DEBUG_VAR_NAMES, "1",
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
);
registerSet("Try Loop", this::registerTryLoop,
IFernflowerPreferences.BYTECODE_SOURCE_MAPPING, "1",
Expand All @@ -80,7 +87,8 @@ protected void registerAll() {
IFernflowerPreferences.IGNORE_INVALID_BYTECODE, "1",
IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES, "1",
IFernflowerPreferences.INCLUDE_ENTIRE_CLASSPATH, "0",
IFernflowerPreferences.TRY_LOOP_FIX, "1"
IFernflowerPreferences.TRY_LOOP_FIX, "1",
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
);
registerSet("Javadoc", () -> {
register(JAVA_8, "TestJavadoc");
Expand Down Expand Up @@ -108,7 +116,8 @@ public String getMethodDoc(StructClass structClass, StructMethod structMethod) {
IFernflowerPreferences.IGNORE_INVALID_BYTECODE, "1",
IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES, "1",
IFernflowerPreferences.INCLUDE_ENTIRE_CLASSPATH, "0",
IFernflowerPreferences.RENAME_ENTITIES, "1"
IFernflowerPreferences.RENAME_ENTITIES, "1",
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
);
registerSet("Complex Condys", () -> register(JASM, "TestComplexCondy"),
IFernflowerPreferences.BYTECODE_SOURCE_MAPPING, "1",
Expand All @@ -118,7 +127,8 @@ public String getMethodDoc(StructClass structClass, StructMethod structMethod) {
IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES, "1",
IFernflowerPreferences.INCLUDE_ENTIRE_CLASSPATH, "0",
IFernflowerPreferences.DECOMPILE_COMPLEX_CONDYS, "1",
IFernflowerPreferences.PREFERRED_LINE_LENGTH, "250"
IFernflowerPreferences.PREFERRED_LINE_LENGTH, "250",
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
);
registerSet("Text Tokens", this::registerTextTokens,
IFernflowerPreferences.DUMP_TEXT_TOKENS, "1",
Expand All @@ -130,15 +140,17 @@ public String getMethodDoc(StructClass structClass, StructMethod structMethod) {
IFernflowerPreferences.INCLUDE_ENTIRE_CLASSPATH, "0",
IFernflowerPreferences.TERNARY_CONDITIONS, "1",
IFernflowerPreferences.FORCE_JSR_INLINE, "1",
IFernflowerPreferences.PREFERRED_LINE_LENGTH, "120"
IFernflowerPreferences.PREFERRED_LINE_LENGTH, "120",
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
);
registerSet("Synthetics Marking", this::registerSyntheticsMarking,
IFernflowerPreferences.BYTECODE_SOURCE_MAPPING, "1",
IFernflowerPreferences.DUMP_ORIGINAL_LINES, "1",
IFernflowerPreferences.DUMP_EXCEPTION_ON_ERROR, "0",
IFernflowerPreferences.IGNORE_INVALID_BYTECODE, "1",
IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES, "1",
IFernflowerPreferences.MARK_CORRESPONDING_SYNTHETICS, "1"
IFernflowerPreferences.MARK_CORRESPONDING_SYNTHETICS, "1",
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
);
registerSet("Lambda to Anonymous Class", this::registerLambdaToAnonymousClass,
IFernflowerPreferences.BYTECODE_SOURCE_MAPPING, "1",
Expand All @@ -149,7 +161,8 @@ public String getMethodDoc(StructClass structClass, StructMethod structMethod) {
IFernflowerPreferences.INCLUDE_ENTIRE_CLASSPATH, "0",
IFernflowerPreferences.TERNARY_CONDITIONS, "1",
IFernflowerPreferences.FORCE_JSR_INLINE, "1",
IFernflowerPreferences.LAMBDA_TO_ANONYMOUS_CLASS, "1"
IFernflowerPreferences.LAMBDA_TO_ANONYMOUS_CLASS, "1",
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
);
// TODO: user renamer class test
}
Expand Down
Loading