Skip to content

Commit 2e50dff

Browse files
KroppebKroppeb
authored
Add flag to detect wrong variable merges (#477)
* Add flag to detect wrong variable merges * Copy dec file from ci
1 parent 1d17821 commit 2e50dff

File tree

9 files changed

+702
-554
lines changed

9 files changed

+702
-554
lines changed

‎src/org/jetbrains/java/decompiler/main/extern/IFernflowerPreferences.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,12 @@ public interface IFernflowerPreferences {
225225
@Type(DecompilerOption.Type.BOOLEAN)
226226
String VERIFY_VARIABLE_MERGES = "verify-merges";
227227

228+
@Name("[Experimental] Verify Pre Post Variable Merges")
229+
@Description("Will try to validate that code before and after variable merges is equivalent")
230+
@ShortName("pvm")
231+
@Type(DecompilerOption.Type.BOOLEAN)
232+
String VERIFY_PRE_POST_VARIABLE_MERGES = "verify-pre-post-merges";
233+
228234
@Name("[Experimental] Use old try deduplication")
229235
@Description("Use the old try deduplication algorithm for methods with obfuscated exceptions, which inserts dummy exception handlers instead of duplicating blocks")
230236
@Type(DecompilerOption.Type.BOOLEAN)
@@ -437,6 +443,7 @@ static Map<String, Object> getDefaults() {
437443
defaults.put(SHOW_HIDDEN_STATEMENTS, "0"); // Extra debugging that isn't useful in most cases
438444
defaults.put(SIMPLIFY_STACK_SECOND_PASS, "1"); // Generally produces better bytecode, useful to debug if it does something strange
439445
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.
446+
defaults.put(VERIFY_PRE_POST_VARIABLE_MERGES, "0");
440447
defaults.put(OLD_TRY_DEDUP, "0");
441448
defaults.put(DECOMPILE_PREVIEW, "1"); // Preview features are useful to decompile in almost all cases
442449

‎src/org/jetbrains/java/decompiler/modules/decompiler/vars/VarDefinitionHelper.java‎

Lines changed: 126 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,12 @@
99
import org.jetbrains.java.decompiler.main.extern.IFernflowerPreferences;
1010
import org.jetbrains.java.decompiler.main.rels.MethodWrapper;
1111
import org.jetbrains.java.decompiler.modules.decompiler.ExprProcessor;
12+
import org.jetbrains.java.decompiler.modules.decompiler.StackVarsProcessor;
1213
import org.jetbrains.java.decompiler.modules.decompiler.ValidationHelper;
1314
import org.jetbrains.java.decompiler.modules.decompiler.exps.*;
1415
import org.jetbrains.java.decompiler.modules.decompiler.flow.DirectGraph;
1516
import org.jetbrains.java.decompiler.modules.decompiler.flow.FlattenStatementsHelper;
17+
import org.jetbrains.java.decompiler.modules.decompiler.sforms.SSAUConstructorSparseEx;
1618
import org.jetbrains.java.decompiler.modules.decompiler.stats.*;
1719
import org.jetbrains.java.decompiler.modules.decompiler.vars.VarTypeProcessor.FinalType;
1820
import org.jetbrains.java.decompiler.struct.StructClass;
@@ -44,7 +46,7 @@ public class VarDefinitionHelper {
4446

4547
private final VarProcessor varproc;
4648

47-
private final Statement root;
49+
private final RootStatement root;
4850
private final StructMethod mt;
4951
private final Map<VarVersionPair, String> clashingNames = new HashMap<>();
5052

@@ -519,7 +521,118 @@ private void populateTypeBounds(VarProcessor proc, Statement stat) {
519521
}
520522
}
521523

522-
private VPPEntry mergeVars(Statement stat) {
524+
static class VarID {
525+
final VarExprent var;
526+
527+
VarID(VarExprent var) {
528+
this.var = var;
529+
}
530+
531+
@Override
532+
public int hashCode() {
533+
return System.identityHashCode(var);
534+
}
535+
536+
@Override
537+
public boolean equals(Object obj) {
538+
return obj instanceof VarID varID && var == varID.var;
539+
}
540+
}
541+
542+
private Map<VarID, Set<VarID>> getVarExprentSources() {
543+
// Do an ssau analysis to find the sources of variables
544+
SSAUConstructorSparseEx ssau = new SSAUConstructorSparseEx();
545+
try {
546+
ssau.splitVariables(root, mt);
547+
} catch (NullPointerException t) {
548+
// Can happen when something is wrong with variables ...
549+
550+
StackVarsProcessor.setVersionsToNull(root);
551+
return null;
552+
}
553+
554+
Map<VarVersionPair, VarID> lookup = new HashMap<>();
555+
findAllVarExprents(root, lookup);
556+
557+
Map<VarID, Set<VarID>> sources = new HashMap<>();
558+
for (VarVersionNode node : ssau.getSsuVersions().nodes) {
559+
VarID target = lookup.get(node.asPair());
560+
if (target == null) {
561+
continue;
562+
}
563+
564+
Set<VarID> sourceVars = new HashSet<>();
565+
566+
for (VarVersionNode predecessor : node.getPredecessors()) {
567+
VarID source = lookup.get(predecessor.asPair());
568+
if (source != null) {
569+
sourceVars.add(source);
570+
}
571+
}
572+
573+
if (node.phantomNode != null) {
574+
VarID source = lookup.get(node.phantomNode.asPair());
575+
if (source != null) {
576+
sourceVars.add(source);
577+
}
578+
}
579+
580+
if (!sourceVars.isEmpty()) {
581+
sources.put(target, sourceVars);
582+
}
583+
}
584+
585+
StackVarsProcessor.setVersionsToNull(root);
586+
587+
return sources;
588+
}
589+
590+
private static void findAllVarExprents(Statement stat, Map<VarVersionPair, VarID> lookup) {
591+
for (Exprent exprent : stat.getVarDefinitions()) {
592+
if (exprent instanceof VarExprent varExprent) {
593+
lookup.put(new VarVersionPair(varExprent), new VarID(varExprent));
594+
}
595+
}
596+
List<Exprent> lst = stat.getExprents();
597+
if (lst != null) {
598+
for (Exprent exprent : lst) {
599+
for (Exprent exp : exprent.getAllExprents(true, true)) {
600+
if (exp instanceof VarExprent varExprent) {
601+
lookup.put(new VarVersionPair(varExprent), new VarID(varExprent));
602+
}
603+
}
604+
}
605+
}
606+
607+
for (Statement subStat : stat.getStats()) {
608+
findAllVarExprents(subStat, lookup);
609+
}
610+
}
611+
612+
private void compareVarExprentSources(
613+
Map<VarID, Set<VarID>> oldSources,
614+
Map<VarID, Set<VarID>> newSources
615+
) {
616+
if (newSources == null) return;
617+
618+
for (var oldEntry : oldSources.entrySet()) {
619+
Set<VarID> oldSet = oldEntry.getValue();
620+
Set<VarID> newSet = newSources.get(oldEntry.getKey());
621+
622+
// Check if sets match
623+
if (!Objects.equals(oldSet, newSet)) {
624+
root.addComment("$VF: Variable merging failed for merge " + oldEntry.getKey().var + ". Code has semantic differences!");
625+
}
626+
}
627+
628+
for (var newVar : newSources.keySet()) {
629+
if (!oldSources.containsKey(newVar)) {
630+
root.addComment("$VF: Variable merging added a var? " + newVar.var);
631+
}
632+
}
633+
}
634+
635+
private VPPEntry mergeVars(RootStatement stat) {
523636
Map<Integer, VarVersionPair> parent = new HashMap<>(); // Always empty dua!
524637
MethodDescriptor md = MethodDescriptor.parseDescriptor(mt.getDescriptor());
525638

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

537650
populateTypeBounds(varproc, stat);
538651

652+
653+
Map<VarID, Set<VarID>> sources = null;
654+
if (DecompilerContext.getOption(IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES)) {
655+
sources = getVarExprentSources();
656+
}
657+
539658
Map<VarVersionPair, VarVersionPair> denylist = new HashMap<>();
540659
VPPEntry remap = mergeVars(stat, parent, new HashMap<>(), denylist);
541660
while (remap != null) {
@@ -546,6 +665,11 @@ private VPPEntry mergeVars(Statement stat) {
546665

547666
remap = mergeVars(stat, parent, new HashMap<>(), denylist);
548667
}
668+
669+
if (sources != null) {
670+
Map<VarID, Set<VarID>> newSources = getVarExprentSources();
671+
compareVarExprentSources(sources, newSources);
672+
}
549673
return null;
550674
}
551675

‎test/org/jetbrains/java/decompiler/SingleClassesTest.java‎

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,31 +20,35 @@ protected void registerAll() {
2020
IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES, "1",
2121
IFernflowerPreferences.INCLUDE_ENTIRE_CLASSPATH, "0",
2222
IFernflowerPreferences.TERNARY_CONDITIONS, "1",
23-
IFernflowerPreferences.FORCE_JSR_INLINE, "1"
23+
IFernflowerPreferences.FORCE_JSR_INLINE, "1",
24+
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
2425
);
2526
registerSet("Entire Classpath", this::registerEntireClassPath,
2627
IFernflowerPreferences.BYTECODE_SOURCE_MAPPING, "1",
2728
IFernflowerPreferences.DUMP_ORIGINAL_LINES, "1",
2829
IFernflowerPreferences.DUMP_EXCEPTION_ON_ERROR, "0",
2930
IFernflowerPreferences.IGNORE_INVALID_BYTECODE, "1",
3031
IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES, "1",
31-
IFernflowerPreferences.INCLUDE_ENTIRE_CLASSPATH, "1"
32+
IFernflowerPreferences.INCLUDE_ENTIRE_CLASSPATH, "1",
33+
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
3234
);
3335
registerSet("Java Runtime", this::registerJavaRuntime,
3436
IFernflowerPreferences.BYTECODE_SOURCE_MAPPING, "1",
3537
IFernflowerPreferences.DUMP_ORIGINAL_LINES, "1",
3638
IFernflowerPreferences.DUMP_EXCEPTION_ON_ERROR, "0",
3739
IFernflowerPreferences.IGNORE_INVALID_BYTECODE, "1",
3840
IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES, "1",
39-
IFernflowerPreferences.INCLUDE_JAVA_RUNTIME, "1"
41+
IFernflowerPreferences.INCLUDE_JAVA_RUNTIME, "1",
42+
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
4043
);
4144
registerSet("Literals", this::registerLiterals,
4245
IFernflowerPreferences.BYTECODE_SOURCE_MAPPING, "1",
4346
IFernflowerPreferences.DUMP_ORIGINAL_LINES, "1",
4447
IFernflowerPreferences.DUMP_EXCEPTION_ON_ERROR, "0",
4548
IFernflowerPreferences.IGNORE_INVALID_BYTECODE, "1",
4649
IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES, "1",
47-
IFernflowerPreferences.LITERALS_AS_IS, "0"
50+
IFernflowerPreferences.LITERALS_AS_IS, "0",
51+
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
4852
);
4953
registerSet("Pattern Matching", this::registerPatternMatching,
5054
IFernflowerPreferences.BYTECODE_SOURCE_MAPPING, "1",
@@ -53,7 +57,8 @@ protected void registerAll() {
5357
IFernflowerPreferences.IGNORE_INVALID_BYTECODE, "1",
5458
IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES, "1",
5559
IFernflowerPreferences.INCLUDE_ENTIRE_CLASSPATH, "0",
56-
IFernflowerPreferences.PATTERN_MATCHING, "1"
60+
IFernflowerPreferences.PATTERN_MATCHING, "1",
61+
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
5762
);
5863
registerSet("Ternary Constant Simplification", this::registerTernaryConstantSimplification,
5964
IFernflowerPreferences.BYTECODE_SOURCE_MAPPING, "1",
@@ -62,7 +67,8 @@ protected void registerAll() {
6267
IFernflowerPreferences.IGNORE_INVALID_BYTECODE, "1",
6368
IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES, "1",
6469
IFernflowerPreferences.LITERALS_AS_IS, "0",
65-
IFernflowerPreferences.TERNARY_CONSTANT_SIMPLIFICATION, "1"
70+
IFernflowerPreferences.TERNARY_CONSTANT_SIMPLIFICATION, "1",
71+
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
6672
);
6773
registerSet("LVT", this::registerLVT,
6874
IFernflowerPreferences.DECOMPILE_INNER, "1",
@@ -71,7 +77,8 @@ protected void registerAll() {
7177
IFernflowerPreferences.ASCII_STRING_CHARACTERS, "1",
7278
IFernflowerPreferences.REMOVE_SYNTHETIC, "1",
7379
IFernflowerPreferences.REMOVE_BRIDGE, "1",
74-
IFernflowerPreferences.USE_DEBUG_VAR_NAMES, "1"
80+
IFernflowerPreferences.USE_DEBUG_VAR_NAMES, "1",
81+
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
7582
);
7683
registerSet("Try Loop", this::registerTryLoop,
7784
IFernflowerPreferences.BYTECODE_SOURCE_MAPPING, "1",
@@ -80,7 +87,8 @@ protected void registerAll() {
8087
IFernflowerPreferences.IGNORE_INVALID_BYTECODE, "1",
8188
IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES, "1",
8289
IFernflowerPreferences.INCLUDE_ENTIRE_CLASSPATH, "0",
83-
IFernflowerPreferences.TRY_LOOP_FIX, "1"
90+
IFernflowerPreferences.TRY_LOOP_FIX, "1",
91+
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
8492
);
8593
registerSet("Javadoc", () -> {
8694
register(JAVA_8, "TestJavadoc");
@@ -108,7 +116,8 @@ public String getMethodDoc(StructClass structClass, StructMethod structMethod) {
108116
IFernflowerPreferences.IGNORE_INVALID_BYTECODE, "1",
109117
IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES, "1",
110118
IFernflowerPreferences.INCLUDE_ENTIRE_CLASSPATH, "0",
111-
IFernflowerPreferences.RENAME_ENTITIES, "1"
119+
IFernflowerPreferences.RENAME_ENTITIES, "1",
120+
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
112121
);
113122
registerSet("Complex Condys", () -> register(JASM, "TestComplexCondy"),
114123
IFernflowerPreferences.BYTECODE_SOURCE_MAPPING, "1",
@@ -118,7 +127,8 @@ public String getMethodDoc(StructClass structClass, StructMethod structMethod) {
118127
IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES, "1",
119128
IFernflowerPreferences.INCLUDE_ENTIRE_CLASSPATH, "0",
120129
IFernflowerPreferences.DECOMPILE_COMPLEX_CONDYS, "1",
121-
IFernflowerPreferences.PREFERRED_LINE_LENGTH, "250"
130+
IFernflowerPreferences.PREFERRED_LINE_LENGTH, "250",
131+
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
122132
);
123133
registerSet("Text Tokens", this::registerTextTokens,
124134
IFernflowerPreferences.DUMP_TEXT_TOKENS, "1",
@@ -130,15 +140,17 @@ public String getMethodDoc(StructClass structClass, StructMethod structMethod) {
130140
IFernflowerPreferences.INCLUDE_ENTIRE_CLASSPATH, "0",
131141
IFernflowerPreferences.TERNARY_CONDITIONS, "1",
132142
IFernflowerPreferences.FORCE_JSR_INLINE, "1",
133-
IFernflowerPreferences.PREFERRED_LINE_LENGTH, "120"
143+
IFernflowerPreferences.PREFERRED_LINE_LENGTH, "120",
144+
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
134145
);
135146
registerSet("Synthetics Marking", this::registerSyntheticsMarking,
136147
IFernflowerPreferences.BYTECODE_SOURCE_MAPPING, "1",
137148
IFernflowerPreferences.DUMP_ORIGINAL_LINES, "1",
138149
IFernflowerPreferences.DUMP_EXCEPTION_ON_ERROR, "0",
139150
IFernflowerPreferences.IGNORE_INVALID_BYTECODE, "1",
140151
IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES, "1",
141-
IFernflowerPreferences.MARK_CORRESPONDING_SYNTHETICS, "1"
152+
IFernflowerPreferences.MARK_CORRESPONDING_SYNTHETICS, "1",
153+
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
142154
);
143155
registerSet("Lambda to Anonymous Class", this::registerLambdaToAnonymousClass,
144156
IFernflowerPreferences.BYTECODE_SOURCE_MAPPING, "1",
@@ -149,7 +161,8 @@ public String getMethodDoc(StructClass structClass, StructMethod structMethod) {
149161
IFernflowerPreferences.INCLUDE_ENTIRE_CLASSPATH, "0",
150162
IFernflowerPreferences.TERNARY_CONDITIONS, "1",
151163
IFernflowerPreferences.FORCE_JSR_INLINE, "1",
152-
IFernflowerPreferences.LAMBDA_TO_ANONYMOUS_CLASS, "1"
164+
IFernflowerPreferences.LAMBDA_TO_ANONYMOUS_CLASS, "1",
165+
IFernflowerPreferences.VERIFY_PRE_POST_VARIABLE_MERGES, "1"
153166
);
154167
// TODO: user renamer class test
155168
}

0 commit comments

Comments
 (0)