8334492: DiagnosticCommands (jcmd) should accept %p in output filenames and substitute PID

Reviewed-by: kevinw, stuefe, lmesnik
This commit is contained in:
Sonia Zaldana Calles 2024-07-30 18:40:37 +00:00
parent 93c19ac73c
commit f5c9e8f122
11 changed files with 157 additions and 39 deletions

View File

@ -1790,15 +1790,17 @@ void CodeCache::log_state(outputStream* st) {
#ifdef LINUX
void CodeCache::write_perf_map(const char* filename, outputStream* st) {
MutexLocker mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
// Perf expects to find the map file at /tmp/perf-<pid>.map
// if the file name is not specified.
char fname[32];
char fname[JVM_MAXPATHLEN];
if (filename == nullptr) {
jio_snprintf(fname, sizeof(fname), "/tmp/perf-%d.map", os::current_process_id());
// Invocation outside of jcmd requires pid substitution.
if (!Arguments::copy_expand_pid(DEFAULT_PERFMAP_FILENAME,
strlen(DEFAULT_PERFMAP_FILENAME),
fname, JVM_MAXPATHLEN)) {
st->print_cr("Warning: Not writing perf map as pid substitution failed.");
return;
}
filename = fname;
}
fileStream fs(filename, "w");
if (!fs.is_open()) {
st->print_cr("Warning: Failed to create %s for perf map", filename);

View File

@ -80,6 +80,10 @@ class ShenandoahParallelCodeHeapIterator;
class NativePostCallNop;
class DeoptimizationScope;
#ifdef LINUX
#define DEFAULT_PERFMAP_FILENAME "/tmp/perf-%p.map"
#endif
class CodeCache : AllStatic {
friend class VMStructs;
friend class JVMCIVMStructs;

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2011, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -127,6 +127,14 @@ static void fill_in_parser(DCmdParser* parser, oop argument)
} else {
parser->add_dcmd_option(argument);
}
} else if (strcmp(type, "FILE") == 0) {
DCmdArgument<char*>* argument =
new DCmdArgument<char*>(name, desc, "FILE", mandatory);
if (isarg) {
parser->add_dcmd_argument(argument);
} else {
parser->add_dcmd_option(argument);
}
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2011, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -189,9 +189,18 @@ template <> void DCmdArgument<char*>::parse_value(const char* str,
destroy_value();
} else {
// Use realloc as we may have a default set.
_value = REALLOC_C_HEAP_ARRAY(char, _value, len + 1, mtInternal);
int n = os::snprintf(_value, len + 1, "%.*s", (int)len, str);
assert((size_t)n <= len, "Unexpected number of characters in string");
if (strcmp(type(), "FILE") == 0) {
_value = REALLOC_C_HEAP_ARRAY(char, _value, JVM_MAXPATHLEN, mtInternal);
if (!Arguments::copy_expand_pid(str, len, _value, JVM_MAXPATHLEN)) {
stringStream error_msg;
error_msg.print("File path invalid or too long: %s", str);
THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), error_msg.base());
}
} else {
_value = REALLOC_C_HEAP_ARRAY(char, _value, len + 1, mtInternal);
int n = os::snprintf(_value, len + 1, "%.*s", (int)len, str);
assert((size_t)n <= len, "Unexpected number of characters in string");
}
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -27,6 +27,7 @@
#include "classfile/vmSymbols.hpp"
#include "memory/allocation.hpp"
#include "runtime/arguments.hpp"
#include "runtime/javaThread.hpp"
#include "runtime/os.hpp"
#include "utilities/exceptions.hpp"

View File

@ -473,7 +473,7 @@ void FinalizerInfoDCmd::execute(DCmdSource source, TRAPS) {
#if INCLUDE_SERVICES // Heap dumping/inspection supported
HeapDumpDCmd::HeapDumpDCmd(outputStream* output, bool heap) :
DCmdWithParser(output, heap),
_filename("filename","Name of the dump file", "STRING",true),
_filename("filename","Name of the dump file", "FILE",true),
_all("-all", "Dump all objects, including unreachable objects",
"BOOLEAN", false, "false"),
_gzip("-gz", "If specified, the heap dump is written in gzipped format "
@ -852,20 +852,15 @@ void CodeCacheDCmd::execute(DCmdSource source, TRAPS) {
}
#ifdef LINUX
#define DEFAULT_PERFMAP_FILENAME "/tmp/perf-<pid>.map"
PerfMapDCmd::PerfMapDCmd(outputStream* output, bool heap) :
DCmdWithParser(output, heap),
_filename("filename", "Name of the map file", "STRING", false, DEFAULT_PERFMAP_FILENAME)
_filename("filename", "Name of the map file", "FILE", false, DEFAULT_PERFMAP_FILENAME)
{
_dcmdparser.add_dcmd_argument(&_filename);
}
void PerfMapDCmd::execute(DCmdSource source, TRAPS) {
// The check for _filename.is_set() is because we don't want to use
// DEFAULT_PERFMAP_FILENAME, since it is meant as a description
// of the default, not the actual default.
CodeCache::write_perf_map(_filename.is_set() ? _filename.value() : nullptr, output());
CodeCache::write_perf_map(_filename.value(), output());
}
#endif // LINUX
@ -997,12 +992,12 @@ void ClassesDCmd::execute(DCmdSource source, TRAPS) {
}
#if INCLUDE_CDS
#define DEFAULT_CDS_ARCHIVE_FILENAME "java_pid<pid>_<subcmd>.jsa"
#define DEFAULT_CDS_ARCHIVE_FILENAME "java_pid%p_<subcmd>.jsa"
DumpSharedArchiveDCmd::DumpSharedArchiveDCmd(outputStream* output, bool heap) :
DCmdWithParser(output, heap),
_suboption("subcmd", "static_dump | dynamic_dump", "STRING", true),
_filename("filename", "Name of shared archive to be dumped", "STRING", false,
_filename("filename", "Name of shared archive to be dumped", "FILE", false,
DEFAULT_CDS_ARCHIVE_FILENAME)
{
_dcmdparser.add_dcmd_argument(&_suboption);
@ -1040,7 +1035,7 @@ void DumpSharedArchiveDCmd::execute(DCmdSource source, TRAPS) {
// call CDS.dumpSharedArchive
Handle fileh;
if (file != nullptr) {
fileh = java_lang_String::create_from_str(_filename.value(), CHECK);
fileh = java_lang_String::create_from_str(file, CHECK);
}
Symbol* cds_name = vmSymbols::jdk_internal_misc_CDS();
Klass* cds_klass = SystemDictionary::resolve_or_fail(cds_name, true /*throw error*/, CHECK);
@ -1105,7 +1100,7 @@ ThreadDumpToFileDCmd::ThreadDumpToFileDCmd(outputStream* output, bool heap) :
DCmdWithParser(output, heap),
_overwrite("-overwrite", "May overwrite existing file", "BOOLEAN", false, "false"),
_format("-format", "Output format (\"plain\" or \"json\")", "STRING", false, "plain"),
_filepath("filepath", "The file path to the output file", "STRING", true) {
_filepath("filepath", "The file path to the output file", "FILE", true) {
_dcmdparser.add_dcmd_option(&_overwrite);
_dcmdparser.add_dcmd_option(&_format);
_dcmdparser.add_dcmd_argument(&_filepath);
@ -1186,23 +1181,16 @@ void SystemMapDCmd::execute(DCmdSource source, TRAPS) {
MemMapPrinter::print_all_mappings(output());
}
static constexpr char default_filename[] = "vm_memory_map_<pid>.txt";
static constexpr char default_filename[] = "vm_memory_map_%p.txt";
SystemDumpMapDCmd::SystemDumpMapDCmd(outputStream* output, bool heap) :
DCmdWithParser(output, heap),
_filename("-F", "file path", "STRING", false, default_filename) {
_filename("-F", "file path", "FILE", false, default_filename) {
_dcmdparser.add_dcmd_option(&_filename);
}
void SystemDumpMapDCmd::execute(DCmdSource source, TRAPS) {
stringStream defaultname;
const char* name = nullptr;
if (_filename.is_set()) {
name = _filename.value();
} else {
defaultname.print("vm_memory_map_%d.txt", os::current_process_id());
name = defaultname.base();
}
const char* name = _filename.value();
fileStream fs(name);
if (fs.is_open()) {
if (!MemTracker::enabled()) {

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2021, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -190,6 +190,10 @@ public abstract class JCmdTestDumpBase {
PidJcmdExecutor cmdExecutor = new PidJcmdExecutor(String.valueOf(pid));
OutputAnalyzer output = cmdExecutor.execute(jcmd, true/*silent*/);
if (archiveFileName.contains("%p")) {
archiveFileName = archiveFileName.replace("%p", "%d").formatted(pid);
}
if (expectOK) {
output.shouldHaveExitValue(0);
checkFileExistence(archiveFileName, true);

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2021, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -87,6 +87,13 @@ public class JCmdTestStaticDump extends JCmdTestDumpBase {
}
app.stopApp();
// Test static dump with file name containing %p
print2ln(test_count++ + " Test static dump with given file name containing %p.");
app = createLingeredApp("-cp", allJars);
pid = app.getPid();
test("%p.jsa", pid, noBoot, EXPECT_PASS, STATIC_MESSAGES);
app.stopApp();
// Test static dump with flags with which dumping should fail
// This test will result classes.jsa in default server dir if -XX:SharedArchiveFile= not set.
print2ln(test_count++ + " Test static dump with flags with which dumping should fail.");

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -33,6 +33,7 @@
import java.math.BigInteger;
import jdk.test.lib.process.ProcessTools;
import jdk.test.whitebox.parser.DiagnosticCommand;
import jdk.test.whitebox.parser.DiagnosticCommand.DiagnosticArgumentType;
import jdk.test.whitebox.WhiteBox;
@ -49,6 +50,7 @@ public class ParserTest {
testQuotes();
testMemorySize();
testSingleLetterArg();
testFileName();
}
public static void main(String... args) throws Exception {
@ -159,6 +161,33 @@ public class ParserTest {
parse("value", "v", "flag v", ' ', args);
}
public void testFileName() throws Exception {
// --- Testing options
long pid = ProcessTools.getProcessId();
// Test pid gets injected into %p
String name = "name";
DiagnosticCommand arg = new DiagnosticCommand(name,
"desc", DiagnosticArgumentType.FILE,
false, null);
DiagnosticCommand[] args = {arg};
parse(name, "file%d.txt".formatted(pid), name + "=file%p.txt", args);
// Test custom file name with no %p
parse(name, "myFile.txt", name + "=myFile.txt", args);
// --- Testing arguments
// Test pid gets injected into %p
arg = new DiagnosticCommand(name, "desc", DiagnosticArgumentType.FILE, true,
false, null);
args = new DiagnosticCommand[]{arg};
parse(name, "file%d.txt".formatted(pid), "file%p.txt", args);
// Test custom file name with no %p
parse(name, "myFile.txt", "myFile.txt", args);
}
public void testMemorySize() throws Exception {
String name = "name";
String defaultValue = "1024";

View File

@ -0,0 +1,66 @@
/*
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2024, Red Hat Inc.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
/*
* @test
* @bug 8334492
* @summary Test to verify jcmd accepts %p in output filenames and substitutes for PID
* @library /test/lib
* @modules java.base/jdk.internal.misc
* java.management
* @run driver TestJcmdPIDSubstitution
*
*/
import jdk.test.lib.process.OutputAnalyzer;
import jdk.test.lib.process.ProcessTools;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
public class TestJcmdPIDSubstitution {
private static final String FILENAME = "myfile%p";
public static void main(String[] args) throws Exception {
verifyOutputFilenames("Thread.dump_to_file", FILENAME);
verifyOutputFilenames("GC.heap_dump", FILENAME);
verifyOutputFilenames("Compiler.perfmap", FILENAME);
verifyOutputFilenames("System.dump_map", "-F=%s".formatted(FILENAME));
}
private static void verifyOutputFilenames(String... args) throws Exception {
long pid = ProcessTools.getProcessId();
String test_dir = System.getProperty("test.dir", ".");
Path path = Paths.get("%s/myfile%d".formatted(test_dir, pid));
OutputAnalyzer output = JcmdBase.jcmd(args);
output.shouldHaveExitValue(0);
if (Files.exists(path)) {
Files.delete(path);
} else {
throw new Exception("File %s was not created as expected for diagnostic cmd %s".formatted(path, args[0]));
}
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -26,7 +26,7 @@ package jdk.test.whitebox.parser;
public class DiagnosticCommand {
public enum DiagnosticArgumentType {
JLONG, BOOLEAN, STRING, NANOTIME, STRINGARRAY, MEMORYSIZE
JLONG, BOOLEAN, STRING, NANOTIME, STRINGARRAY, MEMORYSIZE, FILE
}
private String name;