RFR: 8319875: Add macOS implementation for jcmd System.map [v2]
Thomas Stuefe
stuefe at openjdk.org
Tue Oct 29 08:04:09 UTC 2024
On Mon, 28 Oct 2024 14:40:44 GMT, Simon Tooke <stooke at openjdk.org> wrote:
>> This is a port of #16301 to macOS.
>>
>> System.map and System.dump_map are implemented using the macOS API and provide roughly the same information in the same format. Most of the heavy lifting was implemented by @tstuefe in https://github.com/openjdk/jdk/pull/16301 - this PR adds the macOS implementation and enables the common code for macOS 64 bit.
>>
>> The System.map tests are also reworked to be cleaner for the three implementations.
>>
>> [Sample output](https://github.com/user-attachments/files/17517412/sample.txt)
>
> Simon Tooke has updated the pull request incrementally with one additional commit since the last revision:
>
> changes from review
Looking good, small nits remain. Could you share an example output (maybe one run with G1, one with ZGC?)
src/hotspot/os/bsd/memMapPrinter_macosx.cpp line 79:
> 77: char buf[PATH_MAX];
> 78: buf[0] = 0;
> 79: proc_regionfilename(getpid(), (uint64_t) _address, buf, sizeof(buf));
I'd probably guard after the API call, just to be sure, with `buf[sizeof(buf)-1] = '\0'`. I could not find a description of this API anywhere, so no idea what its rules about zero termination are when buffer overflows.
src/hotspot/os/bsd/memMapPrinter_macosx.cpp line 101:
> 99: if (valid_share_mode) {
> 100: int share_mode = rinfo.pri_share_mode;
> 101: //share_mode = share_mode == SM_LARGE_PAGE || share_mode == SM_PRIVATE_ALIASED ? SM_PRIVATE;
remnant?
src/hotspot/os/bsd/memMapPrinter_macosx.cpp line 196:
> 194:
> 195: bool is_private = region_info.pri_share_mode == SM_PRIVATE || region_info.pri_share_mode == SM_PRIVATE_ALIASED;
> 196: bool is_shared = region_info.pri_share_mode == SM_SHARED || region_info.pri_share_mode == SM_SHARED_ALIASED || region_info.pri_share_mode == SM_TRUESHARED || region_info.pri_share_mode == SM_COW;
give us a line break :-) ?
src/hotspot/os/bsd/memMapPrinter_macosx.cpp line 212:
> 210: }
> 211:
> 212: st->print_cr("Number of mappings: %u", _num_mappings);
If you reshuffle the lines (move this up), you can combine the conditions for err_vm ==/!= KERN_SUCCESS into one statement, would be easier to read
src/hotspot/os/bsd/memMapPrinter_macosx.cpp line 336:
> 334: }
> 335:
> 336: #endif // __APPLE__
whitespace error?
test/hotspot/jtreg/serviceability/dcmd/vm/SystemMapTestBase.java line 180:
> 178: macOSbase + macprivate + space + someNumber + space + "META.*",
> 179: // parts of metaspace should be uncommitted
> 180: //regexBase + "-" + space + "META.*",
?? remnant
-------------
PR Review: https://git.openjdk.org/jdk/pull/20953#pullrequestreview-2401027472
PR Review Comment: https://git.openjdk.org/jdk/pull/20953#discussion_r1820295899
PR Review Comment: https://git.openjdk.org/jdk/pull/20953#discussion_r1820296647
PR Review Comment: https://git.openjdk.org/jdk/pull/20953#discussion_r1820297843
PR Review Comment: https://git.openjdk.org/jdk/pull/20953#discussion_r1820299881
PR Review Comment: https://git.openjdk.org/jdk/pull/20953#discussion_r1820291338
PR Review Comment: https://git.openjdk.org/jdk/pull/20953#discussion_r1820301511
More information about the serviceability-dev
mailing list