On Thu, 19 Mar 2026 13:37:31 GMT, Coleen Phillimore <[email protected]> wrote:
>> The return for aload_0 looks correct to me. This ignores the >> Bytecodes::can_trap() and treats it as false, because aload_0 cannot trap. >> Even the the fast bytecodes that can trap advance the bci first, so it is >> the next instruction (like getfield) that is used to find the handler. >> What does this have to do with monitors? Maybe nothing. I couldn't find >> anything to explain the reference to monitor analysis. >> It would be tempting to change the Bytecodes::can_trap() table to use >> "false" for aload_0, but then something else might break. It's also unclear >> if a single "can trap" value makes sense for fast bytecodes that fuse two >> operations into one. > > Okay this explanation helps with aload_0 in the switch statement. Rewriting > is ambiguous - I was thinking of the rewriter and that didn't help. So it > makes sense that the aload_0 should exit because it says it can trap but it > cannot trap (unless it's the first instruction in the exception handler and > the condition above handles that). > > The monitorexit and monitorenter can trap with async exceptions so I don't > know why we exit the do_exception_edge() method for them. The comments seem > to think they can't trap if the monitors match but it's ignoring async > exceptions. So I fixed this a different way. These bytecodes in GenerateOopMap::do_exception_edge() that can trap but are checked for whether they *will* trap were missing the notion that they can send async exceptions. I added extra code to disable async exception delivery for these particular bytecodes. These are the set of bytecodes that match the ciTypeFlow:can_trap() case. In any case, the async exception delivery is only a problem for the interpreter when single stepping or for breakpoints. So the ciTypeFlow code should be fine. They only thing that I think is missing is that maybe ciTypeFlow should understand the case when the bytecode is the first in the exception handler, which could throw an exception for a bad catch clause. I think this was handled in a different way by virtue of the fact that the test that failed is in compiler/exceptions directory. The compiler must handle this with code for JDK-8367002 which appears to deoptimize. So I believe it's only the Interpreter oopmap generation that needs to be aware of this exception edge. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30171#discussion_r2967205669
