RFR: 8234930: Use MAP_JIT when allocating pages for code cache on macOS

[Thomas Stuefe]

On Tue, 22 Sep 2020 07:08:35 GMT, Anton Kozlov <akozlov at openjdk.org> wrote:

> Please review an updated RFR from https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-August/041463.html
> 
> On macOS, MAP_JIT cannot be used with MAP_FIXED[1]. So pd_reserve_memory have to provide MAP_JIT for mmap(NULL,
> PROT_NONE), the function was made aware of exec permissions.
> For executable and data regions, pd_commit_memory only unlocks the memory with mprotect, this should make no difference
> compared with old code.
> For data regions, pd_uncommit_memory still uses a new overlapping anonymous mmap which returns pages to the OS and
> immediately reflects this in diagnostic tools like ps.  For executable regions it would require MAP_FIXED|MAP_JIT, so
> instead madvise(MADV_FREE)+mprotect(PROT_NONE) are used. They should also allow OS to reclaim pages, but apparently
> this does not happen immediately. In practice, it should not be a problem for executable regions, as codecache does not
> shrink (if I haven't missed anything, by the implementation and in principle).  Tested:
> * local tier1
> * jdk-submit
> * codesign[2] with hardened runtime and allow-jit but without
> allow-unsigned-executable-memory entitlements[3] produce a working bundle.
> 
> (adding GC group as suggested by @dholmes-ora)
> 
> 
> [1] https://github.com/apple/darwin-xnu/blob/master/bsd/kern/kern_mman.c#L227
> [2]
>  
>   codesign \
>     --sign - \
>     --options runtime \
>     --entitlements ents.plist \
>     --timestamp \
>     $J/bin/* $J/lib/server/*.dylib $J/lib/*.dylib
> [3]
>   <?xml version="1.0" encoding="UTF-8"?>
>   <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
>   <plist version="1.0">
>     <dict>
>       <key>com.apple.security.cs.allow-jit</key>
>       <true/>
>       <key>com.apple.security.cs.disable-library-validation</key>
>       <true/>
>       <key>com.apple.security.cs.allow-dyld-environment-variables</key>
>       <true/>
>     </dict>
>   </plist>

Hi Anton,
 
I am sorry, but I dislike the API changes to os::xxx(). As it is now, this API is quite convoluted, undocumented and
inconsistent. There are ongoing efforts to clean this up (see eg JDK-8253650, JDK-8253638 and JDK-8253683). This mess
carries over into the upper layers too, e.g. ReservedSpace.

About the API changes:

- Why does os::uncommit_memory() now require "exec" as parameter? I know why you do it, but semantically it has no
  meaning. I should be able to uncommit memory without knowing the exact flags the mapping had been established with. So
  now the user has to carry this information and provide it back to the API when uncommitting? Right now probably all
  uncommits happen in areas where the exec information is implicit by its usage, but who says this is always the case?

- We now have to specify "exec" as parameter to os::reverve_memory(). This is another new restriction - before, I could
  theoretically reserve one mapping and commit various parts of it with and without exec flag. With this change, the
  whole mapping has to be either exec or !exec.
Can we on MacOS not use mmap at os::commit_memory, like we do on other platforms? In os::commit_memory, the exec
parameter exists but on MacOS it is ignored. It would be more consistent with other platforms if the exec parameter
were honored in commit, not requiring a new one at reserve. And I see mmap is used in uncommit, so changing mappings
via overlapped mmap seems to work?

Thanks, Thomas

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

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