RFR: 8358801: javac produces class that does not pass verifier.

[Jan Lahoda]

On Tue, 17 Jun 2025 15:57:20 GMT, Chen Liang <liach at openjdk.org> wrote:

>> Consider code like:
>> 
>> 
>> public class Main {
>> 
>>     private boolean test(String s, int i) {
>>         if (s.subSequence(0, 1) instanceof Runnable r) {
>>             return true;
>>         }
>> 
>>         Integer dummy;
>>         switch (i) {
>>             case 0:
>>                 String clashing = null;
>>                 return true;
>>             default:
>>                 return true;
>>         }
>>     }
>> 
>>     public static void main(String[] args) {
>>     }
>> }
>> 
>> 
>> javac will produce code that won't (rightfully) pass the verifier:
>> 
>> 
>> $ java Main.java
>> Exception in thread "main" java.lang.VerifyError: Inconsistent stackmap frames at branch target 49
>> Exception Details:
>>   Location:
>>     Main.test(Ljava/lang/String;I)Z @49: iconst_1
>>   Reason:
>>     Type top (current frame, locals[4]) is not assignable to 'java/lang/String' (stack map, locals[4])
>>   Current Frame:
>>     bci: @25
>>     flags: { }
>>     locals: { 'Main', 'java/lang/String', integer }
>>     stack: { integer }
>>   Stackmap Frame:
>>     bci: @49
>>     flags: { }
>>     locals: { 'Main', 'java/lang/String', integer, top, 'java/lang/String' }
>>     stack: { }
>>   Bytecode:
>>     0000000: 2b03 04b6 0007 3a04 1904 c100 0d99 000b
>>     0000010: 1904 c000 0d4e 04ac 1cab 0000 0000 0018
>>     0000020: 0000 0001 0000 0000 0000 0013 013a 0404
>>     0000030: ac04 ac
>>   Stackmap Table:
>>     same_frame(@24)
>>     same_frame(@44)
>>     append_frame(@49,Top,Object[#8])
>> 
>>         at java.base/java.lang.Class.getDeclaredMethods0(Native Method)
>>         at java.base/java.lang.Class.privateGetDeclaredMethods(Class.java:3035)
>>         at java.base/java.lang.Class.getMethodsRecursive(Class.java:3177)
>>         at java.base/java.lang.Class.findMethod(Class.java:2465)
>>         at java.base/java.lang.System$1.findMethod(System.java:1980)
>>         at java.base/jdk.internal.misc.MethodFinder.findMainMethod(MethodFinder.java:86)
>>         at jdk.compiler/com.sun.tools.javac.launcher.SourceLauncher.execute(SourceLauncher.java:194)
>>         at jdk.compiler/com.sun.tools.javac.launcher.SourceLauncher.run(SourceLauncher.java:138)
>>         at jdk.compiler/com.sun.tools.javac.launcher.SourceLauncher.main(SourceLauncher.java:76)
>> 
>> 
>> 
>> Now, the problem, as far as I can tell, is this: javac will desugar the pattern matching instanceof along the lines of:
>> 
>> 
>> if (... (var $temp = s.subSequence(0, 1) in ... && ...) ...) {
>>     return true;
>> }
>> 
>> 
>> (`$temp` is register/local variable...
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java line 763:
> 
>> 761:             //expression:
>> 762:             undefineVariablesInChain(result.falseJumps, limit);
>> 763:             undefineVariablesInChain(result.trueJumps, limit);
> 
> Should we move this truncation of variables to CondItem itself? I moved the truncation of continue/break to GenContext and found that helpful, maybe we can require an extra int arg for `makeCondItem(int, Chain, Chain)` for the limit?

To me, personally: while doing it here is not perfect (mostly because I would prefer if we didn't have to do this bookkeeping at all), it seems to me like a fairly local property - the let expr starts, and then does its own cleanup. It already calls `code.endScopes(limit);` above this change, so this is basically an extension to that. If we did it at `makeCondItem` time, we would need to pass the limit everywhere `makeCondItem` is called, making it non-local. Many parts of the code would now suddenly have to care whether the node is inside a let expression. That feels to me like increasing complexity.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25849#discussion_r2152954768