[Reproducers] Add a tool to transparently capture and replay lldb sessions

[JDevlieghere]

Summary

This patch introduces a small new utility (lldb-repro) to transparently capture and replay debugger sessions from the driver. This makes it possible to test the reproducers by running the test suite twice, once to capture a reproducer for every lldb invocation, and then running the test suite again that replays the previously recorded session.

$ ./bin/llvm-lit ../llvm-project/lldb/test/Shell/ -sv --param=lldb-run-with-repro=capture
$ ./bin/llvm-lit ../llvm-project/lldb/test/Shell/ -sv --param=lldb-run-with-repro=replay

[labath]

This seems like it could be useful. Some random questions inline..

lldb/test/Shell/lit.cfg.py

42–46

The difference between these two is super-unclear. Any chance to unify them?

lldb/tools/lldb-repro/lldb-repro.cpp

23

Is the r in the name intentional?

63

I am wondering if this shouldn't even be the default behavior. If I already passed --capture to lldb then it does not seem unreasonable to always write it out when lldb exits (we already do it after a crash, right?)

lldb/tools/lldb-repro/lldb-repro.h.cmake

12

Are you sure this will work fine with multi-config generators? You might be better off just relying on the fact that the lldb executable will sit right next to this binary...

[labath]

Considered making this a python script?

lldb/tools/lldb-repro/lldb-repro.h.cmake

12

Actually how, is this thing going to be invoked exactly? Couldn't the path to lldb be passed simply as argv[1]?

[JDevlieghere]

In D72823#1824152, @labath wrote:

Considered making this a python script?

I think that makes sense

lldb/test/Shell/lit.cfg.py

42–46

Not really, one is for the tool, the other is to enable reproducer capture by default. They're totally orthogonal, but I agree it's confusing.

lldb/tools/lldb-repro/lldb-repro.cpp

23

Nope, this is a typo

63

My ultimate goal is to have reproducers enabled by default. The capture flag is just away to make that behavior opt-in for now. You might use it to get a reproducer on a crash, but you don't necessary want to keep all reproducers around otherwise.

lldb/tools/lldb-repro/lldb-repro.h.cmake

12

It just needs patching up like lldb-dotest and lit. Assuming you mean argv[0], it think we could make that work if I replace "%lldb" with a path to lldb-repro.

[aprantl]

lldb/test/Shell/lit.cfg.py

42–46

How about adding this as a comment here?

lldb/tools/lldb-repro/lldb-repro.cpp

25

hash_combine()?

lldb/tools/lldb-repro/lldb-repro.h.cmake

5

Out of curiosity, is the Copyright 2020 line part of the recommended default file header?

[aprantl]

lldb/tools/lldb-repro/lldb-repro.h.cmake

5

To answer my own question: https://www.llvm.org/docs/CodingStandards.html#file-headers

No, that's a Swift-lldb-ism. That's great because those lines are annoying to keep up to date anyway.

[JDevlieghere]

• Convert to Python
• Integrate with lit

[aprantl]

lldb/utils/lldb-repro/lldb-repro.py

1 ↗

(On Diff #238554)

Could we add a top-level comment (and/or a Usage: message) that explains what this tool is used for? That's not really obvious for someone who hasn't read the commit message.

[JDevlieghere]

Add a comment explaining the purpose of the utility.

[aprantl]

lldb/utils/lldb-repro/lldb-repro.py

49 ↗

(On Diff #238597)

Thanks! Sorry for being such a nuisance, but I think this should be a doc-comment (""") at the top of the file?

[JDevlieghere]

lldb/utils/lldb-repro/lldb-repro.py

49 ↗

(On Diff #238597)

No worries, totally fair!

[labath]

lldb/test/Shell/helper/toolchain.py

13 ↗

(On Diff #238626)

Maybe _get_lldb_init_path? This way it looks like this function is doing some fancy initialization..

lldb/test/Shell/lit.cfg.py

42–46

Yes, that's better now, though, per the other comment, I think the env vars can be removed completely.

lldb/tools/lldb-repro/lldb-repro.cpp

63

Ok, that makes sense. That said, I think we will always want a way to disable this -- this just looks too much like one of those "this shit is tracking everything I do" features a lot of people will want to stay away from.

lldb/tools/lldb-repro/lldb-repro.h.cmake

12

No, I really meant argv[1]. :)

The idea was that %lldb would expand to /src/path/to/lldb-repro.py /build/path/to/lldb.exe --whatever. That way, you wouldn't need to rely on the "same directory" trick and could get rid of all the cmake code. In fact, we could even throw in a --capture/--replay argument to the command line, and ditch the environment variables too...

[JDevlieghere]

• Remove environment variables

[JDevlieghere]

lldb/tools/lldb-repro/lldb-repro.h.cmake

12

I like the idea but FindTool is a class that's resolved by lit, and the arguments are strings. So I kept the current approach that expects to find lldb next to lldb-repro.

[labath]

lldb/tools/lldb-repro/lldb-repro.h.cmake

12

Ah, yes, that does make things tricky.. Yeah, having both things in the same directory seems fine...

lldb/utils/lldb-repro/lldb-repro.py

24 ↗

(On Diff #238837)

I guess you don't need the /path/to/lldb part then?

[JDevlieghere]

lldb/utils/lldb-repro/lldb-repro.py

24 ↗

(On Diff #238837)

Right, forgot to undo that change again. Thx!

lldb/test/Shell/Reproducer/lit.local.cfg

 # Enable crash reports for the reproducer tests.
 if 'LLVM_DISABLE_CRASH_REPORT' in config.environment:
   del config.environment['LLVM_DISABLE_CRASH_REPORT']
 
 if 'LLDB_CAPTURE_REPRODUCER' in config.environment:
   del config.environment['LLDB_CAPTURE_REPRODUCER']
+
+if 'LLDB_REPRO_CAPTURE' in config.environment or \
+   'LLDB_REPRO_REPLAY' in config.environment:
+  config.unsupported = True

lldb/test/Shell/lit.cfg.py

 config.excludes = ['Inputs', 'CMakeLists.txt', 'README.txt', 'LICENSE.txt']
 
 # test_source_root: The root path where tests are located.
 config.test_source_root = os.path.dirname(__file__)
 
 # test_exec_root: The root path where tests should be run.
 config.test_exec_root = os.path.join(config.lldb_obj_root, 'test')
 
-# Propagate LLDB_CAPTURE_REPRODUCER
+# Propagate reproducer environment vars.
 if 'LLDB_CAPTURE_REPRODUCER' in os.environ:
   config.environment['LLDB_CAPTURE_REPRODUCER'] = os.environ[
       'LLDB_CAPTURE_REPRODUCER']
+if 'LLDB_REPRO_CAPTURE' in os.environ:
+  config.environment['LLDB_REPRO_CAPTURE'] = os.environ['LLDB_REPRO_CAPTURE']
+
+if 'LLDB_REPRO_REPLAY' in os.environ:
+  config.environment['LLDB_REPRO_REPLAY'] = os.environ['LLDB_REPRO_REPLAY']
 
 llvm_config.use_default_substitutions()
 toolchain.use_lldb_substitutions(config)
 toolchain.use_support_substitutions(config)
 
 
 if re.match(r'^arm(hf.*-linux)|(.*-linux-gnuabihf)', config.target_triple):
     config.available_features.add("armhf-linux")

lldb/tools/CMakeLists.txt

 add_subdirectory(argdumper)
 add_subdirectory(driver)
 add_subdirectory(intel-features)
 
 # We want lldb-test to be built only when it's needed,
 # i.e. if a target requires it as dependency. The typical
 # example is `check-lldb`. So, we pass EXCLUDE_FROM_ALL here.
 add_subdirectory(lldb-test EXCLUDE_FROM_ALL)
 
+if(LLDB_INCLUDE_TESTS)
   add_lldb_tool_subdirectory(lldb-instr)
+endif()
+
+add_lldb_tool_subdirectory(lldb-repro)
 add_lldb_tool_subdirectory(lldb-vscode)
 
 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   add_lldb_tool_subdirectory(darwin-debug)
   if(NOT LLDB_USE_SYSTEM_DEBUGSERVER)
     add_lldb_tool_subdirectory(debugserver)
   endif()
 endif()
 
 if (LLDB_CAN_USE_LLDB_SERVER)
   add_lldb_tool_subdirectory(lldb-server)
 endif()

lldb/tools/lldb-repro/CMakeLists.txt

+set(lldb_repro_input ${CMAKE_CURRENT_SOURCE_DIR}/lldb-repro.h.cmake)
+set(lldb_repro_output ${CMAKE_CURRENT_BINARY_DIR}/lldb-repro.h)
+configure_file(${lldb_repro_input} ${lldb_repro_output})
+
+add_lldb_tool(lldb-repro
+  lldb-repro.cpp
+
+  LINK_COMPONENTS
+    Support
+  )

lldb/tools/lldb-repro/lldb-repro.h.cmake

+//===-- lldb-repro.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_TOOLS_LLDB_REPRO_H
+#define LLDB_TOOLS_LLDB_REPRO_H
+
+#cmakedefine LLDB_TEST_EXECUTABLE "${LLDB_TEST_EXECUTABLE}"
+
+#endif

lldb/tools/lldb-repro/lldb-repro.cpp

+//===-- lldb-repro.cpp ----------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb-repro.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/DJB.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
+#include "llvm/Support/WithColor.h"
+#include "llvm/Support/raw_ostream.h"
+#include <string>
+
+using namespace llvm;
+
+unsigned ComputerHash(StringRef args, StringRef cwd) {
+  const unsigned hash = djbHash(cwd);
+  return djbHash(args, hash);
+}
+
+int main(int argc, const char **argv) {
+  std::string arg_str;
+  for (int i = 1; i < argc; ++i)
+    arg_str.append(argv[i]);
+
+  SmallString<256> cwd_str;
+  sys::fs::current_path(cwd_str);
+
+  // Compute the hash.
+  const unsigned hash = ComputerHash(arg_str, cwd_str.str());
+
+  // Compute the reproducer directory.
+  const char *tmpdir = getenv("TMPDIR");
+  if (!tmpdir) {
+    WithColor::error() << "unable to get the temp directory form TMPDIR\n";
+    return 1;
+  }
+
+  SmallString<256> repro_dir;
+  llvm::sys::path::system_temp_directory(true, repro_dir);
+  sys::path::append(repro_dir, std::to_string(hash));
+
+  // Construct the lldb argument list.
+  std::vector<StringRef> args;
+  args.push_back(LLDB_TEST_EXECUTABLE);
+  if (getenv("LLDB_REPRO_REPLAY")) {
+    args.push_back("--replay");
+    args.push_back(repro_dir.c_str());
+  } else if (getenv("LLDB_REPRO_CAPTURE")) {
+    // Remove any previous reproducer.
+    sys::fs::remove_directories(repro_dir);
+
+    args.push_back("--capture");
+    args.push_back("--capture-path");
+    args.push_back(repro_dir.c_str());
+    args.push_back("--reproducer-auto-generate");
+
+    // Append the original arguments.
+    for (int i = 1; i < argc; ++i)
+      args.push_back(argv[i]);
+  } else {
+    WithColor::error()
+        << "enable capture or replay by setting the environment variables "
+           "LLDB_REPRO_CAPTURE and LLDB_REPRO_REPLAY respectively\n";
+    return 1;
+  }
+
+  return llvm::sys::ExecuteAndWait(LLDB_TEST_EXECUTABLE, args);
+}