RFR: 8325108: POSIX map_memory_to_file calls release_memory incorrectly

[Thomas Stuefe]

On Thu, 1 Feb 2024 11:50:15 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

> I'm fairly sure that this branch is actually dead code as `MAP_FIXED` is appended to the flags of the mmap call, and according to my man pages on mmap this will lead to mmap failing instead of giving another mapping to the address. However, I'd rather be on the safe side than sorry so I only do the minimal changes necessary to fix the bug.

Yes, this can never happen. Its POSIX (https://pubs.opengroup.org/onlinepubs/009695399/functions/mmap.html):

"When MAP_FIXED is set in the flags argument, the implementation is informed that the value of pa shall be addr, exactly. If MAP_FIXED is set, mmap() may return MAP_FAILED and set errno to [EINVAL]. If a MAP_FIXED request is successful, the mapping established by mmap() replaces any previous mappings for the process' pages in the range [pa,pa+len)."

MAP_FIXED uses the address given. If it works, the resulting mapping starts at address. I would just assert here. Handling something that cannot happen makes little sense, and it is confusing to read.

BTW, MAP_FIXED is optional, but all OpenJDK Posix platforms must implement it since it is the base for mmap-based reserving and committing. 

> Also, not 100% sure about the MAP_FIXED comment, my man page (on macOS) only talks about deleting previous mappings and that Use of this option is discouraged. though hotspot seems to be using MAP_FIXED in several places already.

MAP_FIXED is essential in writing your own memory management. There is no other way to change mapping properties. Deleting previous mappings is exactly the point of this flag. How, e.g., would you commit otherwise? 

Comments like these in official man pages really lower my opinion of Apple.

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

PR Comment: https://git.openjdk.org/jdk/pull/17669#issuecomment-2258950834