Synchronization on class reference inside if block causes "Tried to end blocks but..." error when "recpass" is set

[Col-E]

CFR Version

CFR 0.151 (fecd6421a8b9de98ade2b9f9b89caecf4a8c93d8)

Compiler

OpenJDK javac 1.8.0_252

Description

Initially reported in https://github.com/Col-E/Recaf/issues/387 - Having a synchronized block operating on a class reference in a simple if block causes a crash. When the if statement is defined inside the synchronized block it decompiles correctly (albeit with some warnings).

Example

public class Example {
    private static final Object lock = new Object();
    private static Example e = null;

    // Fails to decompile
    public static Example getFail1() {
        if (e == null)
            synchronized (Example.class) {
                e = new Example();
            }
        return e;
    }
}

The output:

    /*
     * Exception decompiling
     */
    public static Example getFail1() {
        /*
         * This method has failed to decompile.  When submitting a bug report, please provide this stack trace, and (if you hold appropriate legal rights) the relevant class file.
         * 
         * org.benf.cfr.reader.util.ConfusedCFRException: Tried to end blocks [3[SIMPLE_IF_TAKEN]], but top level block is 1[MONITOR]
         *     at org.benf.cfr.reader.bytecode.analysis.opgraph.Op04StructuredStatement.processEndingBlocks(Op04StructuredStatement.java:435)
         *     at org.benf.cfr.reader.bytecode.analysis.opgraph.Op04StructuredStatement.buildNestedBlocks(Op04StructuredStatement.java:484)
         *     at org.benf.cfr.reader.bytecode.analysis.opgraph.Op03SimpleStatement.createInitialStructuredBlock(Op03SimpleStatement.java:736)
         *     at org.benf.cfr.reader.bytecode.CodeAnalyser.getAnalysisInner(CodeAnalyser.java:845)
         *     at org.benf.cfr.reader.bytecode.CodeAnalyser.getAnalysisOrWrapFail(CodeAnalyser.java:278)
         *     at org.benf.cfr.reader.bytecode.CodeAnalyser.getAnalysis(CodeAnalyser.java:198)
         *     at org.benf.cfr.reader.entities.attributes.AttributeCode.analyse(AttributeCode.java:94)
         *     at org.benf.cfr.reader.entities.Method.analyse(Method.java:531)
         *     at org.benf.cfr.reader.entities.ClassFile.analyseMid(ClassFile.java:1042)
         *     at org.benf.cfr.reader.entities.ClassFile.analyseTop(ClassFile.java:929)
         *     at org.benf.cfr.reader.Driver.doClass(Driver.java:84)
         *     at org.benf.cfr.reader.CfrDriverImpl.analyse(CfrDriverImpl.java:75)
         *     at me.coley.recaf.decompile.cfr.CfrDecompiler.decompile(CfrDecompiler.java:65)
         *     at me.coley.recaf.ui.controls.view.ClassViewport.lambda$updateView$1(ClassViewport.java:112)
         *     at java.util.concurrent.ForkJoinTask$AdaptedCallable.exec(ForkJoinTask.java:1424)
         *     at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289)
         *     at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056)
         *     at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692)
         *     at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:157)
         */
        throw new IllegalStateException("Decompilation failed");
    }

If we move the if statement inside the block:

    /*
     * WARNING - Removed try catching itself - possible behaviour change.
     */
    public static Example get2() {
        Class<Example> clazz = Example.class;
        synchronized (Example.class) {
            if (e == null) {
                e = new Example();
            }
            // ** MonitorExit[var0] (shouldn't be in output)
            return e;
        }
    }

Example.java - Includes some methods with slight modifications to show behavior difference

[leibnitz27]

Interesting, I don't see this (fail, just the ugly behaviour) - can you attach a class file? Attached what I get from ojava18_252. (rename txt -> class as usual)

C:\code\cfr\target\classes>"c:\Program Files\Java\openjdk8\bin"\java -version
openjdk version "1.8.0_252"
OpenJDK Runtime Environment (build 1.8.0_252-b09)
OpenJDK 64-Bit Server VM (build 25.252-b09, mixed mode)

java -jar cfr-0.151.jar c:\code\cfr_tests\output\ojava_8\org\benf\cfr\tests\SyncTest253.class
/*
 * Decompiled with CFR 0.151.
 */
package org.benf.cfr.tests;

public class SyncTest253 {
    private static final Object lock = new Object();
    private static SyncTest253 e = null;

    /*
     * WARNING - Removed try catching itself - possible behaviour change.
     * Enabled force condition propagation
     * Lifted jumps to return sites
     */
    public static SyncTest253 getFail1() {
        if (e != null) return e;
        Class<SyncTest253> clazz = SyncTest253.class;
        synchronized (SyncTest253.class) {
            e = new SyncTest253();
            // ** MonitorExit[var0] (shouldn't be in output)
            return e;
        }
    }
}

SyncTest253.txt

[Col-E]

Huh, it looks like its something with the config I'm passing then. I assumed the OptionDecoderParam#getDefaultValue() would yield simple booleans for conditional values it yields a string. Similar issues occur for other values, so the config is all messed up. The default settings work fine as you've found.

I'll sort this on my end, but for reference this is what the faulty assumption generated as a config:

 // the rest of the values decoded as expected
"constobf" -> "Value of option 'antiobf'"
"hidebridgemethods" -> "Value of option 'obfattr'"
"obfattr" -> "Value of option 'antiobf'"
"obfcontrol" -> "Value of option 'antiobf'"
"renamedupmembers" -> "Value of option 'rename'"
"renameenumidents" -> "Value of option 'rename'"
"renameillegalidents" -> "Value of option 'rename'"

If I remove my default map population logic with, these bad values aren't passed and it decompiles correctly.

[Col-E]

I've cleaned up my map population issue and it still occurred, so I narrowed down what option was causing the behavior change and it looks like just specifying recpass causes this. I pass it using whatever the ArgumentParam says is the default, so I didn't anticipate a behavior change like this.