RFR: 8271055: Crash during deoptimization with "assert(bb->is_reachable()) failed: getting result from unreachable basicblock" with -XX:+VerifyStack [v3]

[Tobias Hartmann]

On Mon, 26 Jul 2021 07:43:39 GMT, Yi Yang <yyang at openjdk.org> wrote:

>> Hi, I'm trying to fix JDK-8271055. The root cause is that some basic blocks are not interpreted because there is no trap bytecode inside them, these basic blocks eventually become unreachable and lead to a crash. Maybe we need to coordinate compiler and hotspot groups to fix this crash.
>> 
>> ----
>> 
>> The attached test generates the following bytecode:
>> 
>>   void test();
>>     descriptor: ()V
>>     flags: (0x0000)
>>     Code:
>>       stack=2, locals=3, args_size=1
>>          0: iconst_1
>>          1: istore_1
>>          2: iload_1
>>          3: bipush        100
>>          5: if_icmpge     29
>>          8: iinc          1, 1
>>         11: goto          2
>>         14: astore_2
>>         15: iload_1
>>         16: bipush        100
>>         18: if_icmpge     27
>>         21: iinc          1, 1
>>         24: goto          15
>>         27: aload_2
>>         28: athrow
>>         29: return
>> 
>> 
>> 
>> Javac generates some unreachable basic blocks(BCI 14 - 28), and there is no exception table for them, VM knows nothing about them. I found the same bug report at JDK-8022186 which was marked as `Fixed`, but I think it was not properly fixed, In some similar cases, javac cannot work well, I have filed https://bugs.openjdk.java.net/browse/JDK-8271254 for another javac bug.
>> 
>> Going back to this bug, the proposed change is to insert a nop in empty try-block so that javac can generate the correct exception table. Now generated bytecodes look like this:
>> 
>>   void test();
>>     descriptor: ()V
>>     flags: (0x0000)
>>     Code:
>>       stack=2, locals=3, args_size=1
>>          0: iconst_1
>>          1: istore_1
>>          2: nop
>>          3: iload_1
>>          4: bipush        100
>>          6: if_icmpge     30
>>          9: iinc          1, 1
>>         12: goto          3
>>         15: astore_2
>>         16: iload_1
>>         17: bipush        100
>>         19: if_icmpge     28
>>         22: iinc          1, 1
>>         25: goto          16
>>         28: aload_2
>>         29: athrow
>>         30: return
>>       Exception table:
>>          from    to  target type
>>              2     3    15   any
>> 
>> 
>> However, this is not the end. Even if the exception table is correctly generated, VM only enters GenerateOopMap::do_exception_edge when the bytecode may be trapped. In order to setup handler block, we need to relax this restriction to allow even bytecodes such as nop to correctly enter GenerateOopMap::do_exception_edge. Otherwise, handler block will not be interpreted, its stack is problematic and finally hits the assertion.
>
> Yi Yang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   remove usenewcode

HotSpot and javac should be fixed independently, please file a new bug for javac.

Regarding the change to `GenerateOopMap::do_exception_edge`, is that sufficient to fix the bug even without the change to javac? Because HotSpot should also handle bytecode not generated by javac.

The fix looks reasonable to me but someone from runtime who knows that code better should have a look.

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

PR: https://git.openjdk.java.net/jdk/pull/4902