RFR: 8282475: SafeFetch should not rely on existence of Thread::current [v3]

David Holmes dholmes at openjdk.java.net
Tue Mar 8 09:06:06 UTC 2022


On Tue, 8 Mar 2022 08:36:41 GMT, Johannes Bechberger <duke at openjdk.java.net> wrote:

>> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.hpp line 57:
>> 
>>> 55:   static void current_thread_init_wx();
>>> 56: 
>>> 57:   static void current_thread_assert_wx_state(WXMode expected);
>> 
>> Can we move all these into the ThreadWXEnable class so they are not in the os namespace? Even the enum could move - though it will make the use-sites a bit more verbose. I won't insist on pushing this WX stuff even deeper, but if anyone else thinks it is a good idea ... :)
>
> I'm open for suggestions, but putting it there was the simplest way. The problem is that os is not a namespace, but a class. But this could and should probably be changed.

I was suggesting pushing everything in to os::ThreadWXEnable class.

>> src/hotspot/share/prims/jni.cpp line 97:
>> 
>>> 95: #include "utilities/macros.hpp"
>>> 96: #include "utilities/vmError.hpp"
>>> 97: #include "runtime/thread.inline.hpp"
>> 
>> Why do we need this? Why do we not include os.hpp?
>
> We need this, because it does not compile (linker error) otherwise. But I forgot to include os.hpp (but included by thread.inline.hpp).

But you didn't add anything that needs it - in fact you deleted `thread->enable_wx` - so perhaps the linker error was from a different variant of the fix?

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

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


More information about the serviceability-dev mailing list