JoshRosen
commented
3 weeks ago My original bug report contained an alleged Java reproduction, but upon re-read I spotted that CFR's Java decompilation was perhaps over-complicated (unnecessary use of labels, I think) but technically correct, so I have updated the above to include only the Scala reproducer. Attached below is the Scala-generated class file.
Edit 2:
Here is a new pure-Java reproducer:
public class TryCatchReturn {
public static String test() {
String result = "abc";
Object obj = new Object();
try {
System.out.println("In try block");
} catch (Exception e) {
if (e == obj) {
result = e.toString();
} else {
throw e;
}
}
return result;
}
}CFR decompiled it to
public class TryCatchReturn {
public static String test() {
String string = "abc";
Object object = new Object();
try {
System.out.println("In try block");
}
catch (Exception exception) {
if (exception == object) {
string = exception.toString();
}
throw exception;
}
return string;
}
}where the else has been lost and the re-throw occur unconditionally, but after my proposed change it decompiles to
public class TryCatchReturn {
public static String test() {
String string = "abc";
Object object = new Object();
try {
System.out.println("In try block");
} catch (Exception exception) {
if (exception == object) {
string = exception.toString();
} else {
throw exception;
}
}
return string;
}
}The pure-Java repro class has somewhat similar javap output to the Scala example:
public static java.lang.String test();
descriptor: ()Ljava/lang/String;
flags: (0x0009) ACC_PUBLIC, ACC_STATIC
Code:
stack=2, locals=3, args_size=0
0: ldc #2 // String abc
2: astore_0
3: new #3 // class java/lang/Object
6: dup
7: invokespecial #1 // Method java/lang/Object."<init>":()V
10: astore_1
11: getstatic #4 // Field java/lang/System.out:Ljava/io/PrintStream;
14: ldc #5 // String In try block
16: invokevirtual #6 // Method java/io/PrintStream.println:(Ljava/lang/String;)V
19: goto 38
22: astore_2
23: aload_2
24: aload_1
25: if_acmpne 36
28: aload_2
29: invokevirtual #8 // Method java/lang/Exception.toString:()Ljava/lang/String;
32: astore_0
33: goto 38
36: aload_2
37: athrow
38: aload_0
39: areturn
Exception table:
from to target type
11 19 22 Class java/lang/Exception
leibnitz27
CFR version
CFR 0.153-SNAPSHOT (d6f6758)
Compiler
javac 11.0.19
Description
I'm writing to report a bug related to incorrect decompilation of certain conditionals in
catchblocks.I first encountered this problem when decompiling Scala code. Here's a Scala reproducer (Scala 2.12):
uses Scala nonlocal returns (which are implemented by throwing and catching NonLocalReturnControl exceptions).
Taking a look at
javap -voutput for thebarmethod:Here, the
catchblock is checking whether the caught exception's key matches the expected key for non-local return to this method: if it matches, it returns the exception's captured value as the method's return value, otherwise it rethrows the exception because the control flow exception is returning to some higher caller frame.CFR decompiles this as
which is incorrect because
exis being unconditionally rethrown (it should only be rethrown in theelsecase).A potential fix
After some investigation, I believe that this problem relates to
ConditionalRewriter.considerAsSimpleIfand may be similar to an already-solved past problem for switch statements. In that method, the following change results in correct decompilation for the test case provided above:I believe that this may be a generalization of the change in https://github.com/leibnitz27/cfr/commit/084260a9088caa7d995cd31f9fed3619a3c0b693 for SwitchTest38.
After this change, the Scala example's catch body is decompiled to
as expected.
This is the first time that I've delved into CFR's internals, so I'm not sure if this is a proper fix. I also haven't tested to see whether any other block types might have similar problems.
I've struggled a bit with test environment setup, so I haven't confirmed whether this change breaks any existing CFR tests.
(Thank you for your work on CFR! I have been a regular user for several years and it has proven to be very helpful for my work with Scala-compiled classes.)