Add --save-as param to record

CMakeLists.txt

@@ -1751,6 +1751,7 @@ set(TESTS_WITHOUT_PROGRAM
   run_end
   run_in_function
   sanity
+  save_as
   seekticks
   shm_checkpoint
   siginfo

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>           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=<NUM> print trace directory followed by a newline\n"
     "                             to given file descriptor\n"
     "  --syscall-buffer-sig=<NUM> 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<string>& 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<string>& 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<string>& 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<string>& 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<string>& 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<string>& 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<string>& 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.

src/RecordSession.cc

@@ -2383,10 +2383,10 @@ static string lookup_by_path(const string& name) {
 /*static*/ RecordSession::shr_ptr RecordSession::create(
     const vector<string>& argv, const vector<string>& 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),

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<std::string>& argv,
       const std::vector<std::string>& 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<pid_t, RecordTask*> detached_task_map;
 
-  std::string output_trace_dir;
-
   bool use_audit_;
   bool unmap_vdso_;
   bool check_outside_mmaps_;

src/TraceStream.cc

@@ -1314,31 +1314,16 @@ 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;
     int ret;
     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),

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=<OUTPUT_TRACE_DIR>.
    */
-  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|.

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 <trace dir>
+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"

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<int> read_perf_event_paranoid();