[Reproducers] Serialize process arguments in ProcessInfo

[JDevlieghere]

Summary

While debugging why TestProcessList.py failed during passive replay, I remembered that we don't serialize the arguments for ProcessInfo. This is necessary to make the test pass and to make platform process list -v behave the same during capture and replay.

$ ./bin/lldb --capture
(lldb) platform process list -v
303 matching processes were found on "host"
PID    PARENT USER       GROUP      EFF USER   EFF GROUP  TRIPLE                         ARGUMENTS
====== ====== ========== ========== ========== ========== ============================== ============================
40118  40116  jonas      staff      jonas      staff      x86_64-apple-macosx             -fish
40116  2502   jonas      staff      jonas      staff      x86_64-apple-macosx             fish -c reattach-to-user-namespace -l fish
...

$./bin/lldb --replay /var/folders/n4/g9s84l4125s3x9fqjjmncxzh0000gn/T/reproducer-71d635
(lldb) platform process list -v
303 matching processes were found on "host"
PID    PARENT USER       GROUP      EFF USER   EFF GROUP  TRIPLE                         ARGUMENTS
====== ====== ========== ========== ========== ========== ============================== ============================
40118  40116  jonas      staff      jonas      staff      x86_64-apple-macosx
40116  2502   jonas      staff      jonas      staff      x86_64-apple-macosx

[labath]

lldb/source/Utility/Args.cpp

691

The GetCommandString -> SetCommandString roundtrip is very lossy (mainly because of the Get part). That may be enough for platform process list, but I don't think it's a good general representation. I think it would be better to represent this as a vector of (string, quote_char) pairs.

lldb/test/Shell/Reproducer/TestProcessList.test

2–22

This isn't too bad, though for a change like this, it may be enough to write a (c++) unit test for the ProcessInfo<->yaml conversion -- it's going to run everywhere, and you can make proper assertions about the expected output.

[JDevlieghere]

• Serialize Args::ArgEntry
• Add unit test

lldb/include/lldb/Utility/Args.h

 //===-- Args.h --------------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_UTILITY_ARGS_H
 #define LLDB_UTILITY_ARGS_H
 
 #include "lldb/Utility/Environment.h"
 #include "lldb/lldb-private-types.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/YAMLTraits.h"
 #include <string>
 #include <utility>
 #include <vector>
 
 namespace lldb_private {
 
 /// \class Args Args.h "lldb/Utility/Args.h"
 /// A command line argument class.
 ///
 /// The Args class is designed to be fed a command line. The command line is
 /// copied into an internal buffer and then split up into arguments. Arguments
 /// are space delimited if there are no quotes (single, double, or backtick
 /// quotes) surrounding the argument. Spaces can be escaped using a \
 /// character to avoid having to surround an argument that contains a space
 /// with quotes.
 class Args {
 public:
   struct ArgEntry {
   private:
     friend class Args;
+    friend struct llvm::yaml::MappingTraits<Args>;
+    friend struct llvm::yaml::MappingTraits<Args::ArgEntry>;
+
     std::unique_ptr<char[]> ptr;
     char quote;
 
     char *data() { return ptr.get(); }
 
   public:
     ArgEntry() = default;
     ArgEntry(llvm::StringRef str, char quote);
public:
   // the help for EncodeEscapeSequences, then it will be encoded as octal.
   // Printable characters are left alone.
   static void ExpandEscapedCharacters(const char *src, std::string &dst);
 
   static std::string EscapeLLDBCommandArgument(const std::string &arg,
                                                char quote_char);
 
 private:
+  friend struct llvm::yaml::MappingTraits<Args>;
+
   std::vector<ArgEntry> m_entries;
   std::vector<char *> m_argv;
 };
 
 /// \class OptionsWithRaw Args.h "lldb/Utility/Args.h"
 /// A pair of an option list with a 'raw' string as a suffix.
 ///
 /// This class works similar to Args, but handles the case where we have a
private:
 
   // FIXME: This should be a StringRef, but some of the calling code expect a
   // C string here so only a real std::string is possible.
   std::string m_suffix;
 };
 
 } // namespace lldb_private
 
+namespace llvm {
+namespace yaml {
+template <> struct MappingTraits<lldb_private::Args::ArgEntry> {
+  class NormalizedArgEntry {
+  public:
+    NormalizedArgEntry(IO &) {}
+    NormalizedArgEntry(IO &, lldb_private::Args::ArgEntry &entry)
+        : value(entry.ref()), quote(entry.quote) {}
+    lldb_private::Args::ArgEntry denormalize(IO &) {
+      return lldb_private::Args::ArgEntry(value, quote);
+    }
+    StringRef value;
+    char quote;
+  };
+  static void mapping(IO &io, lldb_private::Args::ArgEntry &v);
+};
+template <> struct MappingTraits<lldb_private::Args> {
+  static void mapping(IO &io, lldb_private::Args &v);
+};
+} // namespace yaml
+} // namespace llvm
+
+LLVM_YAML_IS_SEQUENCE_VECTOR(lldb_private::Args::ArgEntry)
+
 #endif // LLDB_UTILITY_ARGS_H

lldb/source/Utility/Args.cpp

void OptionsWithRaw::SetFromString(llvm::StringRef arg_string) {
   }
 
   // If we didn't find a suffix delimiter, the whole string is the raw suffix.
   if (!found_suffix) {
     found_suffix = true;
     m_suffix = std::string(original_args);
   }
 }
+
+void llvm::yaml::MappingTraits<Args::ArgEntry>::mapping(IO &io,
+                                                        Args::ArgEntry &v) {
+  MappingNormalization<NormalizedArgEntry, Args::ArgEntry> keys(io, v);
+  io.mapRequired("value", keys->value);
+  io.mapRequired("quote", keys->quote);
+}
+
+void llvm::yaml::MappingTraits<Args>::mapping(IO &io, Args &v) {
+  io.mapRequired("entries", v.m_entries);
+
+  // Recompute m_argv vector.
+  v.m_argv.clear();
+  for (auto &entry : v.m_entries)
+    v.m_argv.push_back(entry.data());
+  v.m_argv.push_back(nullptr);
+}

lldb/source/Utility/ProcessInfo.cpp

void ProcessInstanceInfoMatch::Clear() {
   m_name_match_type = NameMatch::Ignore;
   m_match_all_users = false;
 }
 
 void llvm::yaml::MappingTraits<ProcessInstanceInfo>::mapping(
     IO &io, ProcessInstanceInfo &Info) {
   io.mapRequired("executable", Info.m_executable);
   io.mapRequired("arg0", Info.m_arg0);
+  io.mapRequired("args", Info.m_arguments);
   io.mapRequired("arch", Info.m_arch);
   io.mapRequired("uid", Info.m_uid);
   io.mapRequired("gid", Info.m_gid);
   io.mapRequired("pid", Info.m_pid);
   io.mapRequired("effective-uid", Info.m_euid);
   io.mapRequired("effective-gid", Info.m_egid);
   io.mapRequired("parent-pid", Info.m_parent_pid);
 }

lldb/test/Shell/Reproducer/TestProcessList.test

+# UNSUPPORTED: system-freebsd
+
+# Test that ProcessInfo is correctly serialized by comparing the output of
+# 'platform process list -v' during capture and replay. The test assumes that
+# there's at least two processes.
+
+# RUN: %lldb -x -b -o 'platform process list -v' -o 'reproducer generate' --capture --capture-path %t.repro > %t.log
+# RUN: %lldb --replay %t.repro >> %t.log
+# RUN: cat %t.log | FileCheck %s
+
+# CHECK: [[PROCS:[0-9]+]] matching processes were found
+# CHECK: PID    PARENT USER       GROUP      EFF USER   EFF GROUP  TRIPLE                         ARGUMENTS
+# CHECK-NEXT: ====== ====== ========== ========== ========== ========== ============================== ============================
+# CHECK-NEXT: [[PID0:[0-9]+]] [[PROC0:.*]]
+# CHECK-NEXT: [[PID1:[0-9]+]] [[PROC1:.*]]
+# CHECK: Reproducer written to
+# CHECK: [[PROCS]] matching processes were found
+# CHECK: PID    PARENT USER       GROUP      EFF USER   EFF GROUP  TRIPLE                         ARGUMENTS
+# CHECK-NEXT: ====== ====== ========== ========== ========== ========== ============================== ============================
+# CHECK-NEXT: [[PID0]] [[PROC0]]
+# CHECK-NEXT: [[PID1]] [[PROC1]]

lldb/unittests/Utility/ArgsTest.cpp

 
 TEST(ArgsTest, ReplaceArgumentAtIndexFarOutOfRange) {
   Args args;
   args.SetCommandString("foo ba b");
   args.ReplaceArgumentAtIndex(4, "baar");
   EXPECT_EQ(3u, args.GetArgumentCount());
   EXPECT_STREQ(args.GetArgumentAtIndex(2), "b");
 }
+
+TEST(ArgsTest, Yaml) {
+  std::string buffer;
+  llvm::raw_string_ostream os(buffer);
+
+  // Serialize.
+  Args args;
+  args.SetCommandString("this 'has' \"multiple\" args");
+  llvm::yaml::Output yout(os);
+  yout << args;
+  os.flush();
+
+  llvm::outs() << buffer;
+
+  // Deserialize.
+  Args deserialized;
+  llvm::yaml::Input yin(buffer);
+  yin >> deserialized;
+
+  EXPECT_EQ(4u, deserialized.GetArgumentCount());
+  EXPECT_STREQ(deserialized.GetArgumentAtIndex(0), "this");
+  EXPECT_STREQ(deserialized.GetArgumentAtIndex(1), "has");
+  EXPECT_STREQ(deserialized.GetArgumentAtIndex(2), "multiple");
+  EXPECT_STREQ(deserialized.GetArgumentAtIndex(3), "args");
+
+  llvm::ArrayRef<Args::ArgEntry> entries = deserialized.entries();
+  EXPECT_EQ(entries[0].GetQuoteChar(), '\0');
+  EXPECT_EQ(entries[1].GetQuoteChar(), '\'');
+  EXPECT_EQ(entries[2].GetQuoteChar(), '"');
+  EXPECT_EQ(entries[3].GetQuoteChar(), '\0');
+}