[Lldb-commits] [lldb] f23b829 - Fixed use of -o and -k in LLDB under Windows when statically compiled with vcruntime.

David Spickett via lldb-commits lldb-commits at lists.llvm.org
Wed Nov 24 08:17:13 PST 2021


Author: Levon Ter-Grigoryan
Date: 2021-11-24T16:17:08Z
New Revision: f23b829a2635a6d833bdc81a12d6217b52ae9e45

URL: https://github.com/llvm/llvm-project/commit/f23b829a2635a6d833bdc81a12d6217b52ae9e45
DIFF: https://github.com/llvm/llvm-project/commit/f23b829a2635a6d833bdc81a12d6217b52ae9e45.diff

LOG: Fixed use of -o and -k in LLDB under Windows when statically compiled with vcruntime.

Right now if the LLDB is compiled under the windows with static vcruntime library, the -o and -k commands will not work.

The problem is that the LLDB create FILE* in lldb.exe and pass it to liblldb.dll which is an object from CRT.
Since the CRT is statically linked each of these module has its own copy of the CRT with it's own global state and the LLDB should not share CRT objects between them.

In this change I moved the logic of creating FILE* out of commands stream from Driver class to SBDebugger.
To do this I added new method: SBError SBDebugger::SetInputStream(SBStream &stream)

Command to build the LLDB:
cmake -G Ninja -DLLVM_ENABLE_PROJECTS="clang;lldb;libcxx"  -DLLVM_USE_CRT_RELEASE="MT" -DLLVM_USE_CRT_MINSIZEREL="MT" -DLLVM_USE_CRT_RELWITHDEBINFO="MT" -DP
YTHON_HOME:FILEPATH=C:/Python38 -DCMAKE_C_COMPILER:STRING=cl.exe -DCMAKE_CXX_COMPILER:STRING=cl.exe ../llvm

Command which will fail:
lldb.exe -o help

See discord discussion for more details: https://discord.com/channels/636084430946959380/636732809708306432/854629125398724628
This revision is for the further discussion.

Reviewed By: teemperor

Differential Revision: https://reviews.llvm.org/D104413

Added: 
    

Modified: 
    lldb/bindings/interface/SBDebugger.i
    lldb/include/lldb/API/SBDebugger.h
    lldb/include/lldb/Core/Debugger.h
    lldb/source/API/SBDebugger.cpp
    lldb/source/Core/Debugger.cpp
    lldb/test/API/python_api/default-constructor/sb_debugger.py
    lldb/test/API/python_api/file_handle/TestFileHandle.py
    lldb/tools/driver/Driver.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/bindings/interface/SBDebugger.i b/lldb/bindings/interface/SBDebugger.i
index aae72dd513940..f21e60d628738 100644
--- a/lldb/bindings/interface/SBDebugger.i
+++ b/lldb/bindings/interface/SBDebugger.i
@@ -206,6 +206,9 @@ public:
         }
     }
 
+    SBError
+    SetInputString (const char* data);
+
     SBError
     SetInputFile (SBFile file);
 

diff  --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h
index 64081f79205d6..1c771330cddce 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -126,6 +126,8 @@ class LLDB_API SBDebugger {
 
   FILE *GetErrorFileHandle();
 
+  SBError SetInputString(const char *data);
+
   SBError SetInputFile(SBFile file);
 
   SBError SetOutputFile(SBFile file);

diff  --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index f0849c9ac950a..1ab21bec54c99 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -176,7 +176,13 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
 
   repro::DataRecorder *GetInputRecorder();
 
-  void SetInputFile(lldb::FileSP file, repro::DataRecorder *recorder = nullptr);
+  Status SetInputString(const char *data);
+
+  // This method will setup data recorder if reproducer enabled.
+  // On reply mode this method should take instructions from reproducer file.
+  Status SetInputFile(lldb::FileSP file);
+
+  void SetInputFile(lldb::FileSP file, repro::DataRecorder *recorder);
 
   void SetOutputFile(lldb::FileSP file);
 

diff  --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 4bb23c3e705c6..844b91de4cd01 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -327,12 +327,32 @@ void SBDebugger::SkipAppInitFiles(bool b) {
 void SBDebugger::SetInputFileHandle(FILE *fh, bool transfer_ownership) {
   LLDB_RECORD_METHOD(void, SBDebugger, SetInputFileHandle, (FILE *, bool), fh,
                      transfer_ownership);
-  SetInputFile((FileSP)std::make_shared<NativeFile>(fh, transfer_ownership));
+  if (m_opaque_sp)
+    m_opaque_sp->SetInputFile(
+        (FileSP)std::make_shared<NativeFile>(fh, transfer_ownership));
 }
 
-SBError SBDebugger::SetInputFile(FileSP file_sp) {
-  LLDB_RECORD_METHOD(SBError, SBDebugger, SetInputFile, (FileSP), file_sp);
-  return LLDB_RECORD_RESULT(SetInputFile(SBFile(file_sp)));
+SBError SBDebugger::SetInputString(const char *data) {
+  LLDB_RECORD_METHOD(SBError, SBDebugger, SetInputString, (const char *), data);
+  SBError sb_error;
+  if (data == nullptr) {
+    sb_error.SetErrorString("String data is null");
+    return LLDB_RECORD_RESULT(sb_error);
+  }
+
+  size_t size = strlen(data);
+  if (size == 0) {
+    sb_error.SetErrorString("String data is empty");
+    return LLDB_RECORD_RESULT(sb_error);
+  }
+
+  if (!m_opaque_sp) {
+    sb_error.SetErrorString("invalid debugger");
+    return LLDB_RECORD_RESULT(sb_error);
+  }
+
+  sb_error.SetError(m_opaque_sp->SetInputString(data));
+  return LLDB_RECORD_RESULT(sb_error);
 }
 
 // Shouldn't really be settable after initialization as this could cause lots
@@ -346,36 +366,15 @@ SBError SBDebugger::SetInputFile(SBFile file) {
     error.ref().SetErrorString("invalid debugger");
     return LLDB_RECORD_RESULT(error);
   }
-
-  repro::DataRecorder *recorder = nullptr;
-  if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator())
-    recorder = g->GetOrCreate<repro::CommandProvider>().GetNewRecorder();
-
-  FileSP file_sp = file.m_opaque_sp;
-
-  static std::unique_ptr<repro::MultiLoader<repro::CommandProvider>> loader =
-      repro::MultiLoader<repro::CommandProvider>::Create(
-          repro::Reproducer::Instance().GetLoader());
-  if (loader) {
-    llvm::Optional<std::string> nextfile = loader->GetNextFile();
-    FILE *fh = nextfile ? FileSystem::Instance().Fopen(nextfile->c_str(), "r")
-                        : nullptr;
-    // FIXME Jonas Devlieghere: shouldn't this error be propagated out to the
-    // reproducer somehow if fh is NULL?
-    if (fh) {
-      file_sp = std::make_shared<NativeFile>(fh, true);
-    }
-  }
-
-  if (!file_sp || !file_sp->IsValid()) {
-    error.ref().SetErrorString("invalid file");
-    return LLDB_RECORD_RESULT(error);
-  }
-
-  m_opaque_sp->SetInputFile(file_sp, recorder);
+  error.SetError(m_opaque_sp->SetInputFile(file.m_opaque_sp));
   return LLDB_RECORD_RESULT(error);
 }
 
+SBError SBDebugger::SetInputFile(FileSP file_sp) {
+  LLDB_RECORD_METHOD(SBError, SBDebugger, SetInputFile, (FileSP), file_sp);
+  return LLDB_RECORD_RESULT(SetInputFile(SBFile(file_sp)));
+}
+
 SBError SBDebugger::SetOutputFile(FileSP file_sp) {
   LLDB_RECORD_METHOD(SBError, SBDebugger, SetOutputFile, (FileSP), file_sp);
   return LLDB_RECORD_RESULT(SetOutputFile(SBFile(file_sp)));
@@ -1771,6 +1770,7 @@ template <> void RegisterMethods<SBDebugger>(Registry &R) {
   LLDB_REGISTER_METHOD(bool, SBDebugger, GetAsync, ());
   LLDB_REGISTER_METHOD(void, SBDebugger, SkipLLDBInitFiles, (bool));
   LLDB_REGISTER_METHOD(void, SBDebugger, SkipAppInitFiles, (bool));
+  LLDB_REGISTER_METHOD(SBError, SBDebugger, SetInputString, (const char *));
   LLDB_REGISTER_METHOD(void, SBDebugger, SetInputFileHandle, (FILE *, bool));
   LLDB_REGISTER_METHOD(FILE *, SBDebugger, GetInputFileHandle, ());
   LLDB_REGISTER_METHOD(FILE *, SBDebugger, GetOutputFileHandle, ());

diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 32dcfb1ce17b9..ae454fae3322f 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -45,6 +45,7 @@
 #include "lldb/Utility/Listener.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Reproducer.h"
+#include "lldb/Utility/ReproducerProvider.h"
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StreamCallback.h"
@@ -75,6 +76,14 @@
 #include <string>
 #include <system_error>
 
+// Includes for pipe()
+#if defined(_WIN32)
+#include <fcntl.h>
+#include <io.h>
+#else
+#include <unistd.h>
+#endif
+
 namespace lldb_private {
 class Address;
 }
@@ -810,6 +819,86 @@ void Debugger::SetAsyncExecution(bool async_execution) {
 
 repro::DataRecorder *Debugger::GetInputRecorder() { return m_input_recorder; }
 
+static inline int OpenPipe(int fds[2], std::size_t size) {
+#ifdef _WIN32
+  return _pipe(fds, size, O_BINARY);
+#else
+  (void)size;
+  return pipe(fds);
+#endif
+}
+
+Status Debugger::SetInputString(const char *data) {
+  Status result;
+  enum PIPES { READ, WRITE }; // Indexes for the read and write fds
+  int fds[2] = {-1, -1};
+
+  if (data == nullptr) {
+    result.SetErrorString("String data is null");
+    return result;
+  }
+
+  size_t size = strlen(data);
+  if (size == 0) {
+    result.SetErrorString("String data is empty");
+    return result;
+  }
+
+  if (OpenPipe(fds, size) != 0) {
+    result.SetErrorString(
+        "can't create pipe file descriptors for LLDB commands");
+    return result;
+  }
+
+  write(fds[WRITE], data, size);
+  // Close the write end of the pipe, so that the command interpreter will exit
+  // when it consumes all the data.
+  llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
+
+  // Open the read file descriptor as a FILE * that we can return as an input
+  // handle.
+  FILE *commands_file = fdopen(fds[READ], "rb");
+  if (commands_file == nullptr) {
+    result.SetErrorStringWithFormat("fdopen(%i, \"rb\") failed (errno = %i) "
+                                    "when trying to open LLDB commands pipe",
+                                    fds[READ], errno);
+    llvm::sys::Process::SafelyCloseFileDescriptor(fds[READ]);
+    return result;
+  }
+
+  return SetInputFile(
+      (FileSP)std::make_shared<NativeFile>(commands_file, true));
+}
+
+Status Debugger::SetInputFile(FileSP file_sp) {
+  Status error;
+  repro::DataRecorder *recorder = nullptr;
+  if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator())
+    recorder = g->GetOrCreate<repro::CommandProvider>().GetNewRecorder();
+
+  static std::unique_ptr<repro::MultiLoader<repro::CommandProvider>> loader =
+      repro::MultiLoader<repro::CommandProvider>::Create(
+          repro::Reproducer::Instance().GetLoader());
+  if (loader) {
+    llvm::Optional<std::string> nextfile = loader->GetNextFile();
+    FILE *fh = nextfile ? FileSystem::Instance().Fopen(nextfile->c_str(), "r")
+                        : nullptr;
+    // FIXME Jonas Devlieghere: shouldn't this error be propagated out to the
+    // reproducer somehow if fh is NULL?
+    if (fh) {
+      file_sp = std::make_shared<NativeFile>(fh, true);
+    }
+  }
+
+  if (!file_sp || !file_sp->IsValid()) {
+    error.SetErrorString("invalid file");
+    return error;
+  }
+
+  SetInputFile(file_sp, recorder);
+  return error;
+}
+
 void Debugger::SetInputFile(FileSP file_sp, repro::DataRecorder *recorder) {
   assert(file_sp && file_sp->IsValid());
   m_input_recorder = recorder;

diff  --git a/lldb/test/API/python_api/default-constructor/sb_debugger.py b/lldb/test/API/python_api/default-constructor/sb_debugger.py
index e6267c8475e2c..305c5c2fd5072 100644
--- a/lldb/test/API/python_api/default-constructor/sb_debugger.py
+++ b/lldb/test/API/python_api/default-constructor/sb_debugger.py
@@ -13,6 +13,7 @@ def fuzz_obj(obj):
     obj.SetInputFileHandle(None, True)
     obj.SetOutputFileHandle(None, True)
     obj.SetErrorFileHandle(None, True)
+    obj.SetInputString("")
     obj.GetInputFileHandle()
     obj.GetOutputFileHandle()
     obj.GetErrorFileHandle()

diff  --git a/lldb/test/API/python_api/file_handle/TestFileHandle.py b/lldb/test/API/python_api/file_handle/TestFileHandle.py
index 142fce28309cc..05bb12ea5c4a2 100644
--- a/lldb/test/API/python_api/file_handle/TestFileHandle.py
+++ b/lldb/test/API/python_api/file_handle/TestFileHandle.py
@@ -852,3 +852,20 @@ def test_sbstream(self):
         self.assertTrue(f.closed)
         with open(self.out_filename, 'r') as f:
             self.assertEqual(f.read().strip(), "Frobozz")
+
+    @skipIfReproducer # lldb::FileSP used in typemap cannot be instrumented.
+    def test_set_sbstream(self):
+        with open(self.out_filename, 'w') as outf:
+            outsbf = lldb.SBFile(outf.fileno(), "w", False)
+            status = self.dbg.SetOutputFile(outsbf)
+            self.assertTrue(status.Success())
+            self.dbg.SetInputString("version\nhelp\n")
+
+            opts = lldb.SBCommandInterpreterRunOptions()
+            self.dbg.RunCommandInterpreter(True, False, opts, 0, False, False)
+            self.dbg.GetOutputFile().Flush()
+
+        with open(self.out_filename, 'r') as f:
+            output = f.read()
+            self.assertTrue(re.search(r'Show a list of all debugger commands', output))
+            self.assertTrue(re.search(r'llvm revision', output))
\ No newline at end of file

diff  --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index a51c124f96158..977cc306bb4e6 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -24,7 +24,6 @@
 #include "llvm/Support/Format.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/Path.h"
-#include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
@@ -44,14 +43,6 @@
 #include <cstring>
 #include <fcntl.h>
 
-// Includes for pipe()
-#if defined(_WIN32)
-#include <fcntl.h>
-#include <io.h>
-#else
-#include <unistd.h>
-#endif
-
 #if !defined(__APPLE__)
 #include "llvm/Support/DataTypes.h"
 #endif
@@ -421,60 +412,6 @@ SBError Driver::ProcessArgs(const opt::InputArgList &args, bool &exiting) {
   return error;
 }
 
-static inline int OpenPipe(int fds[2], std::size_t size) {
-#ifdef _WIN32
-  return _pipe(fds, size, O_BINARY);
-#else
-  (void)size;
-  return pipe(fds);
-#endif
-}
-
-static ::FILE *PrepareCommandsForSourcing(const char *commands_data,
-                                          size_t commands_size) {
-  enum PIPES { READ, WRITE }; // Indexes for the read and write fds
-  int fds[2] = {-1, -1};
-
-  if (OpenPipe(fds, commands_size) != 0) {
-    WithColor::error()
-        << "can't create pipe file descriptors for LLDB commands\n";
-    return nullptr;
-  }
-
-  ssize_t nrwr = write(fds[WRITE], commands_data, commands_size);
-  if (size_t(nrwr) != commands_size) {
-    WithColor::error()
-        << format(
-               "write(%i, %p, %" PRIu64
-               ") failed (errno = %i) when trying to open LLDB commands pipe",
-               fds[WRITE], static_cast<const void *>(commands_data),
-               static_cast<uint64_t>(commands_size), errno)
-        << '\n';
-    llvm::sys::Process::SafelyCloseFileDescriptor(fds[READ]);
-    llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
-    return nullptr;
-  }
-
-  // Close the write end of the pipe, so that the command interpreter will exit
-  // when it consumes all the data.
-  llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
-
-  // Open the read file descriptor as a FILE * that we can return as an input
-  // handle.
-  ::FILE *commands_file = fdopen(fds[READ], "rb");
-  if (commands_file == nullptr) {
-    WithColor::error() << format("fdopen(%i, \"rb\") failed (errno = %i) "
-                                 "when trying to open LLDB commands pipe",
-                                 fds[READ], errno)
-                       << '\n';
-    llvm::sys::Process::SafelyCloseFileDescriptor(fds[READ]);
-    return nullptr;
-  }
-
-  // 'commands_file' now owns the read descriptor.
-  return commands_file;
-}
-
 std::string EscapeString(std::string arg) {
   std::string::size_type pos = 0;
   while ((pos = arg.find_first_of("\"\\", pos)) != std::string::npos) {
@@ -604,21 +541,15 @@ int Driver::MainLoop() {
   // Check if we have any data in the commands stream, and if so, save it to a
   // temp file
   // so we can then run the command interpreter using the file contents.
-  const char *commands_data = commands_stream.GetData();
-  const size_t commands_size = commands_stream.GetSize();
-
   bool go_interactive = true;
-  if ((commands_data != nullptr) && (commands_size != 0u)) {
-    FILE *commands_file =
-        PrepareCommandsForSourcing(commands_data, commands_size);
-
-    if (commands_file == nullptr) {
-      // We should have already printed an error in PrepareCommandsForSourcing.
+  if ((commands_stream.GetData() != nullptr) &&
+      (commands_stream.GetSize() != 0u)) {
+    SBError error = m_debugger.SetInputString(commands_stream.GetData());
+    if (error.Fail()) {
+      WithColor::error() << error.GetCString() << '\n';
       return 1;
     }
 
-    m_debugger.SetInputFileHandle(commands_file, true);
-
     // Set the debugger into Sync mode when running the command file. Otherwise
     // command files that run the target won't run in a sensible way.
     bool old_async = m_debugger.GetAsync();
@@ -651,12 +582,9 @@ int Driver::MainLoop() {
       SBStream crash_commands_stream;
       WriteCommandsForSourcing(eCommandPlacementAfterCrash,
                                crash_commands_stream);
-      const char *crash_commands_data = crash_commands_stream.GetData();
-      const size_t crash_commands_size = crash_commands_stream.GetSize();
-      commands_file =
-          PrepareCommandsForSourcing(crash_commands_data, crash_commands_size);
-      if (commands_file != nullptr) {
-        m_debugger.SetInputFileHandle(commands_file, true);
+      SBError error =
+          m_debugger.SetInputString(crash_commands_stream.GetData());
+      if (error.Success()) {
         SBCommandInterpreterRunResult local_results =
             m_debugger.RunCommandInterpreter(options);
         if (local_results.GetResult() ==


        


More information about the lldb-commits mailing list