RFR: 8297600: Check current thread in selected JRT_LEAF methods

David Holmes dholmes at openjdk.org
Sat Nov 26 08:21:13 UTC 2022


On Thu, 24 Nov 2022 19:23:29 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

> With [JDK-8275286](https://bugs.openjdk.org/browse/JDK-8275286), we added the `Thread::current()` checks for most of the JRT entries. But `JRT_LEAF` is still not checked, because not every `JRT_LEAF` carries a `JavaThread` argument. Having assertions there helps for two reasons. First, these methods can be called from the stub/compiler code, which might be erroneous with thread handling (especially in x86_32 that does not have a dedicated thread register). Second, in the post-Loom world, current thread can change suddenly, as evidenced here: https://mail.openjdk.org/pipermail/hotspot-compiler-dev/2022-November/060779.html.
> 
> We can add the thread checks to relevant `JRT_LEAF` methods that accept `JavaThread*` too.
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug `tier1`
>  - [x] Linux x86_64 fastdebug `tier2`
>  - [x] Linux x86_32 fastdebug `tier1`
>  - [x] Linux x86_32 fastdebug `tier2`

Unclear for many JVMCI functions that the thread argument is actually intended/required to be the current thread. It seems unused in many cases so why is it passed?

src/hotspot/share/jvmci/jvmciRuntime.cpp line 584:

> 582: 
> 583: JRT_LEAF(void, JVMCIRuntime::log_object(JavaThread* thread, oopDesc* obj, bool as_string, bool newline))
> 584:   assert(thread == JavaThread::current(), "pre-condition");

`thread` seems unused in this function and so it is not obvious it has to be the current thread.

src/hotspot/share/jvmci/jvmciRuntime.cpp line 611:

> 609: 
> 610: void JVMCIRuntime::write_barrier_pre(JavaThread* thread, oopDesc* obj) {
> 611:   assert(thread == JavaThread::current(), "pre-condition");

Not obvious thread is expected/required to be current

src/hotspot/share/jvmci/jvmciRuntime.cpp line 616:

> 614: 
> 615: void JVMCIRuntime::write_barrier_post(JavaThread* thread, volatile CardValue* card_addr) {
> 616:   assert(thread == JavaThread::current(), "pre-condition");

Not obvious thread is expected/required to be current

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

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


More information about the hotspot-dev mailing list