A rare bug with the jdk has recently been filed again: 
https://github.com/adoptium/adoptium-support/issues/1538, which reports the 
already seen behavior in JDK-8352077: `unhandled instruction`. The original 
issue was closed with no reproduction. I have managed to write a reproduction 
with the new data.

It seems that it is caused by a missing null check in the `BreakpointInfo::set` 
path, which calls into:
`incr_number_of_breakpoints` which calls `build_method_counters`. Which has 
multiple ways to return `nullptr` if either the oom flag is already set or the 
allocation fails, which is done via allocate_no_exception since it runs on a 
VMThread, therefore it cant actually call a GC or raise the metadata limit. 
Therefore incr_number_of_breakpoints, never increases the number (since method 
counters is nullptr). Compilation is then paused, because the method cannot 
increase it's invocation counter, then after a purge caused by a gc pass, the 
mark is unset. After that it can correctly create it's counter, but since it 
never captured the breakpoints, the breakpoint is 0.

The method then can be picked up by a compiler thread since 
`number_of_breakpoints` is now 0 and a counter is real it falsely passes 
`Method::is_not_compilable`. Then it copies the bytecode containing the 
`breakpoint` instruction, and doesn't invoke the clean function since the 
breakpoint count reports '0'. This causes a crash in the `ciBytecodeStream` 
since `ciBytecodeStream::next_wide_or_table` assumes the only special bytecodes 
can be `wide` or a type of switch, and bytecode is a special length 
instruction. 

This also explains why this happens so rarely. For this to happen the following 
need to happen in succession:
- The debugger must try to set breakpoints
- The method must never have been run
- Metadata space needs to be at the limit and not resized when its setting the 
breakpoints and set the oom flag
- Class unloading must purge the flag afterwards
- The method must get compiled hot while the breakpoint is still installed 

3 of the 5 conditions already happen when debugging with jdwp normally, from 
platforms like intellij, the others are race / resource conditions that seem 
happen very rarely.

For the fix i propose to check the method counter allocation before actually 
requesting a breakpoint set. Here done by checking if method counters are not 
null before sending the vm request. I also had to expand the user facing method 
to make sure the OOM error code is propagated properly 
(JVMTI_ERROR_OUT_OF_MEMORY is a universal jvmti error code, so it can be 
returned by jvmti functions).

I have added a test for the bug, so it is reproducible. The original repro 
fails 10/10 on GA and passes with the fix 10/10 on release and fastdebug, and 
the JVMTI/JDWP/JDI/jdb suites pass cleanly.

I would also provide a alternative, which i didnt touch on here since it would 
require changing the reads of it aswell:
Set a new flag in MethodFlags::_statusfor 'has_breakpoints', so even if 
counters fail to allocate, it can bail out any downstream compilations of the 
method.

This is would also address that SetBreakpoint now returns OOM even if that is 
not true, because the flag is only cleared when the class is unloaded

I would also humbly request that the original issue to be re-opened or a new 
issue to be created for this

- [x] I confirm that I make this contribution in accordance with the [OpenJDK 
Interim AI Policy](https://openjdk.org/legal/ai).

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

Commit messages:
 - fix bug number
 - fix unhandled instruction by re-checking method counters in jvmti handler 
directly

Changes: https://git.openjdk.org/jdk/pull/33191/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=33191&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8352077
  Stats: 298 lines in 4 files changed: 297 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/33191.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/33191/head:pull/33191

PR: https://git.openjdk.org/jdk/pull/33191

Reply via email to