On Fri, 26 Feb 2021 22:01:09 GMT, Calvin Cheung <cche...@openjdk.org> wrote:

>> Hi, Please review
>> 
>>   Added jcmd option for dumping CDS archive during application runtime. 
>> Before this change, user has to dump shared archive in two steps: first run 
>> application with
>>   `java -XX:DumpLoadedClassList=<classlist> .... `  
>>  to collect shareable class names and saved in file `<classlist>` , then 
>>   `java -Xshare:dump -XX:SharedClassListFile=<classlist> 
>> -XX:SharedArchiveFile=<archivefile> ...`
>>   With this change, user can use jcmd to dump CDS without going through 
>> above steps. Also user can choose a moment during the app runtime  to dump 
>> an archive.
>>    The bug is associated with the CSR: 
>> https://bugs.openjdk.java.net/browse/JDK-8259798 which has been approved.
>>    New added jcmd option:
>>    `jcmd <pid or AppName> VM.cds static_dump <filename>`
>>    or
>>     `jcmd <pid or AppName> VM.cds dynamic_dump <filename>`
>>   To dump dynamic archive, requires start app with newly added flag 
>> `-XX:+RecordDynamicDumpInfo`, with this flag, some information related to 
>> dynamic dump like loader constraints will be recorded. Note the dumping 
>> process changed some object memory locations so for dumping dynamic archive, 
>> can only done once for a running app. For static dump, user can dump 
>> multiple times against same process. 
>>    The file name is optional, if the file name is not supplied, the file 
>> name will take format of `java_pid<number>_static.jsa` or 
>> `java_pid<number>_dynamic.jsa` for static and dynamic respectively. The 
>> `<number>` is the application process ID.
>> 
>>   Tests: tier1,tier2,tier3,tier4
>> 
>> Thanks
>> Yumin
>
> src/hotspot/share/memory/metaspaceShared.cpp line 783:
> 
>> 781:   char* start = buffer + strlen(buffer);
>> 782:   snprintf(start, buff_len, "%s ", arg);
>> 783: }
> 
> Maybe move the above function to the StringUtils class under share/utilities?
> Use `os::snprintf()` instead of `snprintf()`?

The calculation is also wrong, this would overflow. You need:
  char* start = buffer + strlen(buffer);
  snprintf(start, buff_len - (start - buffer), "%s ", arg);
- and maybe add an assert that strlen(buf) < bufflen. 
- and as Ioi wrote, I'd use either one of os::snprintf or jio_snprintf since 
both guarantee zero termination on truncation.
- or, just use strncat()

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

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

Reply via email to