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