On Tue, 8 Mar 2022 08:36:41 GMT, Johannes Bechberger <d...@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

Reply via email to