Skip to content

Commit

Permalink
Fixed use of -o and -k in LLDB under Windows when statically compiled…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
Levon Ter-Grigoryan authored and DavidSpickett committed Nov 24, 2021
1 parent 6f85d68 commit f23b829
Show file tree
Hide file tree
Showing 8 changed files with 158 additions and 112 deletions.
3 changes: 3 additions & 0 deletions lldb/bindings/interface/SBDebugger.i
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,9 @@ public:
}
}

SBError
SetInputString (const char* data);

SBError
SetInputFile (SBFile file);

Expand Down
2 changes: 2 additions & 0 deletions lldb/include/lldb/API/SBDebugger.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ class LLDB_API SBDebugger {

FILE *GetErrorFileHandle();

SBError SetInputString(const char *data);

SBError SetInputFile(SBFile file);

SBError SetOutputFile(SBFile file);
Expand Down
8 changes: 7 additions & 1 deletion lldb/include/lldb/Core/Debugger.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
62 changes: 31 additions & 31 deletions lldb/source/API/SBDebugger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)));
Expand Down Expand Up @@ -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, ());
Expand Down
89 changes: 89 additions & 0 deletions lldb/source/Core/Debugger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
17 changes: 17 additions & 0 deletions lldb/test/API/python_api/file_handle/TestFileHandle.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
88 changes: 8 additions & 80 deletions lldb/tools/driver/Driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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() ==
Expand Down

0 comments on commit f23b829

Please sign in to comment.