On Wed, 3 Feb 2021 19:57:24 GMT, Anton Kozlov <akoz...@openjdk.org> wrote:
>> For platform files that were copied from other ports to this port, if the
>> file wasn't
>> changed I presume the copyright years are left alone. If the file required
>> changes
>> for this port, I expect the year to be updated to 2021. How are you
>> verifying that
>> these copyright years are being properly managed on the new files?
>>
>> For the new W^X helpers, e.g., WXWriteFromExecSetter, a short comment
>> explaining why one was landed in a particular place would help reviewers.
>> Also see my comment about creating a new ThreadToNativeWithWXExecFromVM
>> helper.
>>
>> I'm stopping my review with all the src/hotspot files done for now.
>
> Thank you all for your comments regarding W^X implementation. I've made a
> change that reduces the footprint of the implementation, also addressing most
> of the comments. I'll revisit them individually to make sure nothing is
> forgotten.
>
> The basic principle has not changed: when we execute JVM code (owned by
> libjvm.so, starting from JVM entry function), we switch to Write state. When
> we leave JVM to execute generated or JNI code, we switch to Executable state.
> I would like to highlight that JVM code does not mean the VM state of the
> java thread. After @stefank's suggestion, I could also drop a few W^X state
> switches, so now it should be more clear that switches are tied to JVM entry
> functions.
> I wonder if this is the right choice
> ...
> ```
> OopStorageParIterPerf::~OopStorageParIterPerf() {
> ...
> ```
>
The transition in OopStorageParIterPerf was made for gtest setup to execute in
WXWrite context. For tests themselves, defining macro set WXWrite.
I've simplified the scheme and now we switch to WXWrite once at the gtest
launcher. So this transition was dropped.
I've also refreshed my memory and tried to switch to WXWrite as close as
possible to each place where we'll be writing executable memory. There are a
lot of such places! As you correctly noted, code cache contains objects, not
plain data. For example, CodeCache memory management structures,
CompiledMethod, ... are there, so we need more WXWrite switches than we have
in the current approach. I had a comparable amount of them just to run
-version, but certainly not enough to run tier1 tests.
Following your advice, I don't require a known "from" state anymore. So a few
W^X transitions were dropped, e.g. when the JVM code calls a JNI entry
function, which expects to be called from the native code. I had to switch to
WXExec just only to satisfy the expectations. After the update, we don't need
this anymore.
W^X switches are mostly hidden by VM_ENTRY and similar macros. Some JVM
functions are not marked as entries for some reason, although they are called
directly from e.g. interpreter. I added W^X management to such functions.
-------------
PR: https://git.openjdk.java.net/jdk/pull/2200
