RFR: 8267341: macos attempt_reserve_memory_at(arg1, arg2, true) failure [v2]

[David Holmes]

On Wed, 5 Jan 2022 15:48:54 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> Re-enable **release_multi_mappings** gtest on macOS
>> 
>> This test would occasionally fail on macOS, but thanks to Dan's catch, it turned out that it actually only fails on those test machines that run macOS 10.13.6 or earlier.
>> 
>> The proposed fix simply makes a single call to `os::reserve_memory()` with the `executable` flag `True`, and if that failed forces `executable`  to `False` later in the actual test code (alternatively we could have just also skipped that test portion completely, but this way we actually do something rather than nothing at all).
>> 
>> Tested manually on macOS 10.13.6 and via Mach5
>
> Gerard Ziemski has updated the pull request incrementally with one additional commit since the last revision:
> 
>   incorporate Dan's feedback

Unless a change comes under the "trivial" rule (this does not) then yes two reviews are needed. :)

A couple of nits below but otherwise this seems a reasonable thing for the test to do.

Thanks,
David

test/hotspot/gtest/runtime/test_os.cpp line 367:

> 365:   // Workaround: try reserving memory with executable=true
> 366:   // to figure out if that's supported on this macOS version
> 367:   bool executable = true;

Doesn't this local interfere with the same named local at line 388? Seems bad style to have two locals with the same name regardless.

test/hotspot/gtest/runtime/test_os.cpp line 369:

> 367:   bool executable = true;
> 368:   size_t test_len = 128;
> 369:   address p_test = (address)os::reserve_memory(test_len, executable);

Nit: if you just declare `p_test` as a `char*` then no casts needed here or on line 372.

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

Marked as reviewed by dholmes (Reviewer).

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