diff --git a/CMakeLists.txt b/CMakeLists.txt index 1c3a4780745..3fd57588028 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1751,6 +1751,7 @@ set(TESTS_WITHOUT_PROGRAM run_end run_in_function sanity + save_as seekticks shm_checkpoint siginfo diff --git a/src/RecordCommand.cc b/src/RecordCommand.cc index adcda9d8ca6..aa0fd423cf3 100644 --- a/src/RecordCommand.cc +++ b/src/RecordCommand.cc @@ -61,6 +61,9 @@ RecordCommand RecordCommand::singleton( " _RR_TRACE_DIR gets ignored.\n" " Directory name is given name, not the\n" " application name.\n" + " --save-as= Name of the new recording's directory. If\n" + " a recording with that name already exists, normal\n" + " number appending is applied." " -p --print-trace-dir= print trace directory followed by a newline\n" " to given file descriptor\n" " --syscall-buffer-sig= the signal used for communication with the\n" @@ -134,7 +137,7 @@ struct RecordFlags { int print_trace_dir; - string output_trace_dir; + TraceOutputPath path; /* Whether to use file-cloning optimization during recording. */ bool use_file_cloning; @@ -200,7 +203,7 @@ struct RecordFlags { use_syscall_buffer(RecordSession::ENABLE_SYSCALL_BUF), syscall_buffer_size(0), print_trace_dir(-1), - output_trace_dir(""), + path{"", "", false, false}, use_file_cloning(true), use_read_cloning(true), bind_cpu(BIND_CPU), @@ -282,6 +285,7 @@ static bool parse_record_arg(vector& args, RecordFlags& flags) { { 18, "tsan", NO_PARAMETER }, { 19, "intel-pt", NO_PARAMETER }, { 20, "check-outside-mmaps", NO_PARAMETER }, + { 21, "save-as", HAS_PARAMETER }, { 'c', "num-cpu-ticks", HAS_PARAMETER }, { 'h', "chaos", NO_PARAMETER }, { 'i', "ignore-signal", HAS_PARAMETER }, @@ -293,6 +297,7 @@ static bool parse_record_arg(vector& args, RecordFlags& flags) { { 'u', "cpu-unbound", NO_PARAMETER }, { 'v', "env", HAS_PARAMETER }, { 'w', "wait", NO_PARAMETER }}; + ParsedOption opt; auto args_copy = args; if (!Command::parse_option(args_copy, options, &opt)) { @@ -327,7 +332,8 @@ static bool parse_record_arg(vector& args, RecordFlags& flags) { flags.print_trace_dir = opt.int_value; break; case 'o': - flags.output_trace_dir = opt.value; + flags.path.output_trace_dir = opt.value; + flags.path.usr_provided_outdir = true; break; case 0: flags.use_read_cloning = false; @@ -501,6 +507,10 @@ static bool parse_record_arg(vector& args, RecordFlags& flags) { case 20: flags.check_outside_mmaps = true; break; + case 21: + flags.path.name = opt.value; + flags.path.usr_provided_name = true; + break; case 's': flags.always_switch = true; break; @@ -684,11 +694,11 @@ static void* repeat_SIGTERM(__attribute__((unused)) void* p) { static WaitStatus record(const vector& args, const RecordFlags& flags) { LOG(info) << "Start recording..."; - + DEBUG_ASSERT(!flags.path.name.empty() && !flags.path.output_trace_dir.empty() && "No output dir or trace dir name set"); auto session = RecordSession::create( - args, flags.extra_env, flags.disable_cpuid_features, + args, flags.extra_env, flags.disable_cpuid_features, flags.path, flags.use_syscall_buffer, flags.syscallbuf_desched_sig, - flags.bind_cpu, flags.output_trace_dir, + flags.bind_cpu, flags.trace_id.get(), flags.stap_sdt, flags.unmap_vdso, flags.asan, flags.tsan, flags.intel_pt, flags.check_outside_mmaps); @@ -720,7 +730,7 @@ static WaitStatus record(const vector& args, const RecordFlags& flags) { bool done_initial_exec = session->done_initial_exec(); step_result = session->record_step(); // Only create latest-trace symlink if --output-trace-dir is not being used - if (!done_initial_exec && session->done_initial_exec() && flags.output_trace_dir.empty()) { + if (!done_initial_exec && session->done_initial_exec() && !flags.path.usr_provided_outdir) { session->trace_writer().make_latest_trace(); } if (term_requested) { @@ -881,6 +891,16 @@ int RecordCommand::run(vector& args) { flags.extra_env.push_back(padding); } + if(flags.path.name.empty()) { + flags.path.name = args[0]; + flags.path.usr_provided_name = false; + } + + if(flags.path.output_trace_dir.empty()) { + flags.path.usr_provided_outdir = false; + flags.path.output_trace_dir = trace_save_dir(); + } + WaitStatus status = record(args, flags); // Everything should have been cleaned up by now. diff --git a/src/RecordSession.cc b/src/RecordSession.cc index 4990247913d..5bb234cbdfc 100644 --- a/src/RecordSession.cc +++ b/src/RecordSession.cc @@ -2383,10 +2383,10 @@ static string lookup_by_path(const string& name) { /*static*/ RecordSession::shr_ptr RecordSession::create( const vector& argv, const vector& extra_env, const DisableCPUIDFeatures& disable_cpuid_features, + const TraceOutputPath& path_info, SyscallBuffering syscallbuf, unsigned char syscallbuf_desched_sig, BindCPU bind_cpu, - const string& output_trace_dir, const TraceUuid* trace_id, bool use_audit, bool unmap_vdso, @@ -2500,7 +2500,7 @@ static string lookup_by_path(const string& name) { shr_ptr session( new RecordSession(full_path, argv, env, disable_cpuid_features, syscallbuf, syscallbuf_desched_sig, bind_cpu, - output_trace_dir, trace_id, use_audit, unmap_vdso, + path_info, trace_id, use_audit, unmap_vdso, intel_pt, check_outside_mmaps)); session->excluded_ranges_ = std::move(exe_info.sanitizer_exclude_memory_ranges); session->fixed_global_exclusion_range_ = std::move(exe_info.fixed_global_exclusion_range); @@ -2514,13 +2514,13 @@ RecordSession::RecordSession(const std::string& exe_path, SyscallBuffering syscallbuf, int syscallbuf_desched_sig, BindCPU bind_cpu, - const string& output_trace_dir, + const TraceOutputPath& path, const TraceUuid* trace_id, bool use_audit, bool unmap_vdso, bool intel_pt_enabled, bool check_outside_mmaps) - : trace_out(argv[0], output_trace_dir, ticks_semantics_), + : trace_out(path, ticks_semantics_), scheduler_(*this), trace_id(trace_id), disable_cpuid_features_(disable_cpuid_features), diff --git a/src/RecordSession.h b/src/RecordSession.h index de150d88c25..23c7fb7c536 100644 --- a/src/RecordSession.h +++ b/src/RecordSession.h @@ -11,6 +11,7 @@ #include "Session.h" #include "ThreadGroup.h" #include "TraceFrame.h" +#include "TraceStream.h" #include "WaitStatus.h" namespace rr { @@ -63,10 +64,10 @@ class RecordSession final : public Session { const std::vector& argv, const std::vector& extra_env, const DisableCPUIDFeatures& features, + const TraceOutputPath& path_info, SyscallBuffering syscallbuf = ENABLE_SYSCALL_BUF, unsigned char syscallbuf_desched_sig = SIGPWR, BindCPU bind_cpu = BIND_CPU, - const std::string& output_trace_dir = "", const TraceUuid* trace_id = nullptr, bool use_audit = false, bool unmap_vdso = false, @@ -219,7 +220,7 @@ class RecordSession final : public Session { SyscallBuffering syscallbuf, int syscallbuf_desched_sig, BindCPU bind_cpu, - const std::string& output_trace_dir, + const TraceOutputPath& path_info, const TraceUuid* trace_id, bool use_audit, bool unmap_vdso, @@ -285,8 +286,6 @@ class RecordSession final : public Session { */ std::map detached_task_map; - std::string output_trace_dir; - bool use_audit_; bool unmap_vdso_; bool check_outside_mmaps_; diff --git a/src/TraceStream.cc b/src/TraceStream.cc index c9983c99ca9..cee2ba4fa81 100644 --- a/src/TraceStream.cc +++ b/src/TraceStream.cc @@ -1314,23 +1314,8 @@ bool TraceReader::read_raw_data_metadata_for_frame(RawDataMetadata& d) { return true; } -static string make_trace_dir(const string& exe_path, const string& output_trace_dir) { - if (!output_trace_dir.empty()) { - // save trace dir in given output trace dir with option -o - int ret = mkdir(output_trace_dir.c_str(), S_IRWXU | S_IRWXG); - if (ret == 0) { - return output_trace_dir; - } - if (EEXIST == errno) { - CLEAN_FATAL() << "Trace directory `" << output_trace_dir << "' already exists."; - } else if (EACCES == errno) { - CLEAN_FATAL() << "Permission denied to create trace directory `" << output_trace_dir << "'"; - } else { - FATAL() << "Unable to create trace directory `" << output_trace_dir << "'"; - } - } else { - // save trace dir set in _RR_TRACE_DIR or in the default trace dir - ensure_dir(trace_save_dir(), "trace directory", S_IRWXU); +static string create_unique_dir(const TraceOutputPath& path) { + ensure_dir(path.output_trace_dir, "trace directory", S_IRWXU); // Find a unique trace directory name. int nonce = 0; @@ -1338,7 +1323,7 @@ static string make_trace_dir(const string& exe_path, const string& output_trace_ string dir; do { stringstream ss; - ss << trace_save_dir() << "/" << basename(exe_path.c_str()) << "-" + ss << path.output_trace_dir << "/" << basename(path.name.c_str()) << "-" << nonce++; dir = ss.str(); ret = mkdir(dir.c_str(), S_IRWXU | S_IRWXG); @@ -1349,6 +1334,28 @@ static string make_trace_dir(const string& exe_path, const string& output_trace_ } return dir; +} + +static string make_trace_dir(const TraceOutputPath& path) { + if (path.usr_provided_outdir) { + // save trace dir in given output trace dir with option -o + int ret = mkdir(path.output_trace_dir.c_str(), S_IRWXU | S_IRWXG); + if(path.usr_provided_name) { + return create_unique_dir(path); + } + if (ret == 0) { + return path.output_trace_dir; + } + if (EEXIST == errno) { + CLEAN_FATAL() << "Trace directory `" << path.output_trace_dir << "' already exists."; + } else if (EACCES == errno) { + CLEAN_FATAL() << "Permission denied to create trace directory `" << path.output_trace_dir << "'"; + } else { + FATAL() << "Unable to create trace directory `" << path.output_trace_dir << "'"; + } + } else { + // save trace dir set in _RR_TRACE_DIR or in the default trace dir + return create_unique_dir(path); } return ""; // not reached @@ -1357,10 +1364,9 @@ static string make_trace_dir(const string& exe_path, const string& output_trace_ #define STR_HELPER(x) #x #define STR(x) STR_HELPER(x) -TraceWriter::TraceWriter(const std::string& file_name, - const string& output_trace_dir, +TraceWriter::TraceWriter(const TraceOutputPath& trace_output, TicksSemantics ticks_semantics_) - : TraceStream(make_trace_dir(file_name, output_trace_dir), + : TraceStream(make_trace_dir(trace_output), // Somewhat arbitrarily start the // global time from 1. 1), diff --git a/src/TraceStream.h b/src/TraceStream.h index 7dfde3f12ed..2bf5c2c60f8 100644 --- a/src/TraceStream.h +++ b/src/TraceStream.h @@ -19,6 +19,7 @@ #include "TraceFrame.h" #include "TraceTaskEvent.h" #include "remote_ptr.h" +#include "util.h" namespace rr { @@ -262,8 +263,7 @@ class TraceWriter : public TraceStream { * The trace name is determined by |file_name| and _RR_TRACE_DIR (if set) * or by setting -o=. */ - TraceWriter(const std::string& file_name, - const string& output_trace_dir, TicksSemantics ticks_semantics); + TraceWriter(const TraceOutputPath& path, TicksSemantics ticks_semantics); /** * Called after the calling thread is actually bound to |bind_to_cpu|. diff --git a/src/test/save_as.run b/src/test/save_as.run new file mode 100644 index 00000000000..fcb76f35011 --- /dev/null +++ b/src/test/save_as.run @@ -0,0 +1,27 @@ +source `dirname $0`/util.sh + +function dir_exists { checkDir=$1; + if [ -d "$checkDir" ]; then + echo "$checkDir" exists + else + failed "$checkDir doesn't exist" + exit 1 + fi +} + +RECORD_ARGS="--save-as test-name" +record hello +dir_exists "$workdir/test-name-0" + +record hello +dir_exists "$workdir/test-name-1" + + +# Check that --save-as plays nice with -o +RECORD_ARGS="-o $workdir/arbitrary-dir --save-as test-name" +record hello +dir_exists "$workdir/arbitrary-dir/test-name-0" +record hello +dir_exists "$workdir/arbitrary-dir/test-name-1" + +passed_msg "--save-as and --save-as -o combination succeeded" \ No newline at end of file diff --git a/src/util.h b/src/util.h index 4a4c1638c56..8125bddc910 100644 --- a/src/util.h +++ b/src/util.h @@ -696,6 +696,16 @@ void replace_in_buffer(MemoryRange src, const uint8_t* src_data, // Strip any directory part from the filename `s` void base_name(std::string& s); +struct TraceOutputPath { + // The root trace directory, where recorded traces will be saved + std::string output_trace_dir; + // The name of the recording that's currently being created + std::string name; + // Flags that are set, to signal if user requested different + // trace dirs or different names of the recordings + bool usr_provided_outdir; + bool usr_provided_name; +}; std::optional read_perf_event_paranoid();