JDevlieghere created this revision.
JDevlieghere added a reviewer: labath.
Herald added subscribers: teemperor, abidh.
Herald added a project: LLDB.

To ensure a reproducer works correctly, the version of LLDB used for capture 
and replay must match. Right now the reproducer already contains the LLDB 
version. However, this is purely informative. LLDB will happily replay a 
reproducer generated with a different version of LLDB, which can cause subtle 
differences.

This patch adds a version check which compares the current LLDB version with 
the one in the reproducer. If the version doesn't match, LLDB will refuse to 
replay. It also adds an escape hatch to make it possible to still replay the 
reproducer without having to mess with the recorded version. This might prove 
useful when you know two versions of LLDB match, even though the version string 
doesn't. This behavior is triggered by passing a new flag 
`-reproducer-skip-version-check` to the lldb driver.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D70934

Files:
  lldb/include/lldb/API/SBReproducer.h
  lldb/source/API/SBReproducer.cpp
  lldb/test/Shell/Reproducer/TestVersionCheck.test
  lldb/tools/driver/Driver.cpp
  lldb/tools/driver/Options.td

Index: lldb/tools/driver/Options.td
===================================================================
--- lldb/tools/driver/Options.td
+++ lldb/tools/driver/Options.td
@@ -232,5 +232,7 @@
 def replay: Separate<["--", "-"], "replay">,
   MetaVarName<"<filename>">,
   HelpText<"Tells the debugger to replay a reproducer from <filename>.">;
+def skip_version_check: F<"reproducer-skip-version-check">,
+  HelpText<"Skip the reproducer version check.">;
 
 def REM : R<["--"], "">;
Index: lldb/tools/driver/Driver.cpp
===================================================================
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -797,7 +797,9 @@
 
 llvm::Optional<int> InitializeReproducer(opt::InputArgList &input_args) {
   if (auto *replay_path = input_args.getLastArg(OPT_replay)) {
-    if (const char *error = SBReproducer::Replay(replay_path->getValue())) {
+    const bool skip_version_check = input_args.hasArg(OPT_skip_version_check);
+    if (const char *error =
+            SBReproducer::Replay(replay_path->getValue(), skip_version_check)) {
       WithColor::error() << "reproducer replay failed: " << error << '\n';
       return 1;
     }
Index: lldb/test/Shell/Reproducer/TestVersionCheck.test
===================================================================
--- /dev/null
+++ lldb/test/Shell/Reproducer/TestVersionCheck.test
@@ -0,0 +1,29 @@
+# REQUIRES: system-darwin
+
+# This tests the reproducer version check.
+
+# RUN: rm -rf %t.repro
+# RUN: %clang_host %S/Inputs/simple.c -g -o %t.out
+# RUN: %lldb -x -b -s %S/Inputs/FileCapture.in --capture --capture-path %t.repro %t.out | FileCheck %s --check-prefix CHECK --check-prefix CAPTURE
+
+# Make sure that replay works.
+# RUN: %lldb --replay %t.repro | FileCheck %s --check-prefix CHECK --check-prefix REPLAY
+
+# Change the reproducer version.
+# RUN: echo "bogus" >> %t.repro/version.txt
+
+# Make sure that replay works.
+# RUN: not %lldb --replay %t.repro 2>&1 | FileCheck %s --check-prefix ERROR
+
+# Make sure that we can circumvent the version check with -reproducer-skip-version-check.
+# RUN: %lldb --replay %t.repro -reproducer-skip-version-check | FileCheck %s --check-prefix CHECK --check-prefix REPLAY
+
+# CAPTURE: testing
+# REPLAY-NOT: testing
+
+# CHECK: Process {{.*}} exited
+
+# CAPTURE: Reproducer is in capture mode.
+# CAPTURE: Reproducer written
+
+# ERROR: error: reproducer replay failed: reproducer capture and replay version don't match
Index: lldb/source/API/SBReproducer.cpp
===================================================================
--- lldb/source/API/SBReproducer.cpp
+++ lldb/source/API/SBReproducer.cpp
@@ -22,8 +22,8 @@
 #include "lldb/API/SBFileSpec.h"
 #include "lldb/API/SBHostOS.h"
 #include "lldb/API/SBReproducer.h"
-
 #include "lldb/Host/FileSystem.h"
+#include "lldb/lldb-private.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -124,7 +124,7 @@
   return nullptr;
 }
 
-const char *SBReproducer::Replay(const char *path) {
+const char *SBReproducer::Replay(const char *path, bool skip_version_check) {
   static std::string error;
   if (auto e = Reproducer::Initialize(ReproducerMode::Replay, FileSpec(path))) {
     error = llvm::toString(std::move(e));
@@ -137,6 +137,22 @@
     return error.c_str();
   }
 
+  if (!skip_version_check) {
+    llvm::Expected<std::string> version = loader->LoadBuffer<VersionProvider>();
+    if (!version) {
+      error = llvm::toString(version.takeError());
+      return error.c_str();
+    }
+    if (lldb_private::GetVersion() != llvm::StringRef(*version).rtrim()) {
+      error = "reproducer capture and replay version don't match:\n";
+      error.append("reproducer captured with:\n");
+      error.append(*version);
+      error.append("reproducer replayed with:\n");
+      error.append(lldb_private::GetVersion());
+      return error.c_str();
+    }
+  }
+
   FileSpec file = loader->GetFile<SBProvider::Info>();
   if (!file) {
     error = "unable to get replay data from reproducer.";
Index: lldb/include/lldb/API/SBReproducer.h
===================================================================
--- lldb/include/lldb/API/SBReproducer.h
+++ lldb/include/lldb/API/SBReproducer.h
@@ -20,7 +20,7 @@
 public:
   static const char *Capture();
   static const char *Capture(const char *path);
-  static const char *Replay(const char *path);
+  static const char *Replay(const char *path, bool skip_version_check = false);
   static const char *GetPath();
   static bool Generate();
 };
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits] [PATCH] ... Jonas Devlieghere via Phabricator via lldb-commits

Reply via email to