jingham created this revision.
jingham added a reviewer: JDevlieghere.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
jingham requested review of this revision.
stop-hooks were not intended to be interactive, that would be really confusing
when there are many in flight. So this patch marks the results as
non-interactive.
Beyond correctness, then main reason for doing this patch is that when you go
to run a command based stop hook that includes a Python command, if the command
is marked as interactive, Python would try to act on stdin (fflush it) when
switching to the Python Session, and if stdin was backed by an lldb NativeFile,
the the fflush would deadlock against the I/O handler thread that was sitting
in read on the same file handle.
That is a more general problem that this patch does not address. The only
place it has shown up IRL is in this scenario, and since doing the right thing
for the stop-hook execution works around it, the more general fix can wait on
somebody else's free time. But I did add a test that will deadlock without
this patch, and succeeds with it.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D90332
Files:
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Target/Target.cpp
lldb/test/API/api/multithreaded/TestMultithreaded.py
lldb/test/API/api/multithreaded/some_cmd.py
lldb/test/API/api/multithreaded/test_stop-hook.cpp.template
Index: lldb/test/API/api/multithreaded/test_stop-hook.cpp.template
===================================================================
--- /dev/null
+++ lldb/test/API/api/multithreaded/test_stop-hook.cpp.template
@@ -0,0 +1,130 @@
+// LLDB C++ API Test: Verify that when the Debugger stdin
+// is set to a FILE *, lldb can still successfully run a
+// python command in a stop hook.
+
+#include <errno.h>
+#include <mutex>
+#include <stdio.h>
+#include <string>
+#include <vector>
+
+%include_SB_APIs%
+
+#include "common.h"
+
+using namespace lldb;
+
+void test(SBDebugger &dbg, std::vector<std::string> args) {
+ // The problem we had was that when the thread that was
+ // waiting on input went into the call to 'read' it had
+ // the file handle lock. Then when the python interpreter
+ // Initialized itself to run the python command, it tried
+ // to flush the file channel, and that deadlocked.
+ // This only happens when Async is true, since otherwise
+ // the process event is handled on the I/O read thread,
+ // which sidestepped the problem.
+ dbg.SetAsync(true);
+
+ SBTarget target = dbg.CreateTarget(args.at(0).c_str());
+ if (!target.IsValid())
+ throw Exception("invalid target");
+
+ SBBreakpoint breakpoint = target.BreakpointCreateByName("next");
+ if (!breakpoint.IsValid())
+ throw Exception("invalid breakpoint");
+
+ SBCommandInterpreter interp = dbg.GetCommandInterpreter();
+ SBCommandReturnObject result;
+
+ // Bring in the python command. We actually add two commands,
+ // one that runs in the stop hook and sets a variable when it
+ // runs, and one that reports out the variable so we can ensure
+ // that we did indeed run the stop hook.
+ const char *source_dir = "%SOURCE_DIR%";
+ SBFileSpec script_spec(source_dir);
+ script_spec.AppendPathComponent("some_cmd.py");
+ char path[PATH_MAX];
+ script_spec.GetPath(path, PATH_MAX);
+
+ std::string import_command("command script import ");
+ import_command.append(path);
+ interp.HandleCommand(import_command.c_str(), result);
+ if (!result.Succeeded())
+ throw Exception("Couldn't import %SOURCE_DIR%/some_cmd.py");
+
+ SBProcess process = target.LaunchSimple(nullptr, nullptr, nullptr);
+ if (!process.IsValid())
+ throw Exception("Couldn't launch process.");
+ if (process.GetState() != lldb::eStateStopped)
+ throw Exception("Process was not stopped");
+
+ process.SetSelectedThreadByIndexID(0);
+
+ // Now add the stop hook:
+ interp.HandleCommand("target stop-hook add -o some-cmd", result);
+ if (!result.Succeeded())
+ throw Exception("Couldn't add a stop hook.");
+
+ // Now switch the I/O over to a pipe, which will be handled by the
+ // NativeFile class:
+ int to_lldb_des[2];
+ int pipe_result = pipe(to_lldb_des);
+ FILE *fh_lldb_in = fdopen(to_lldb_des[0], "r");
+ FILE *fh_to_lldb = fdopen(to_lldb_des[1], "w");
+
+ // We need to reset the handle before destroying the debugger
+ // or the same deadlock will stall exiting:
+ class Cleanup {
+ public:
+ Cleanup(SBDebugger dbg, int filedes[2]) : m_dbg(dbg) {
+ m_file = m_dbg.GetInputFileHandle();
+ m_filedes[0] = filedes[0];
+ m_filedes[1] = filedes[1];
+ }
+ ~Cleanup() {
+ m_dbg.SetInputFileHandle(m_file, false);
+ close(m_filedes[0]);
+ close(m_filedes[1]);
+ }
+
+ private:
+ FILE *m_file;
+ SBDebugger m_dbg;
+ int m_filedes[2];
+ };
+ Cleanup cleanup(dbg, to_lldb_des);
+
+ dbg.SetInputFileHandle(fh_lldb_in, false);
+
+ // Now run the command interpreter. You have to pass true to
+ // start thread so we will run the I/O in a separate thread.
+ dbg.RunCommandInterpreter(false, true);
+
+ // Now issue a stepi, and fetch the running and stopped events:
+ fprintf(fh_to_lldb, "thread step-inst\n");
+
+ SBEvent proc_event;
+ StateType state;
+ bool got_event;
+
+ got_event = dbg.GetListener().WaitForEventForBroadcaster(
+ 100, process.GetBroadcaster(), proc_event);
+ if (!got_event)
+ throw Exception("Didn't get running event");
+ state = SBProcess::GetStateFromEvent(proc_event);
+ if (state != eStateRunning)
+ throw Exception("Event wasn't a running event.");
+
+ got_event = dbg.GetListener().WaitForEventForBroadcaster(
+ 100, process.GetBroadcaster(), proc_event);
+ if (!got_event)
+ throw Exception("Didn't get a stopped event");
+ state = SBProcess::GetStateFromEvent(proc_event);
+ if (state != eStateStopped)
+ throw Exception("Event wasn't a stop event.");
+
+ // At this point the stop hook should have run. Check that:
+ interp.HandleCommand("report-cmd", result);
+ if (!result.Succeeded())
+ throw Exception("Didn't actually call stop hook.");
+}
Index: lldb/test/API/api/multithreaded/some_cmd.py
===================================================================
--- /dev/null
+++ lldb/test/API/api/multithreaded/some_cmd.py
@@ -0,0 +1,33 @@
+""" Test command for checking the Python commands can run in a stop-hook """
+import lldb
+
+did_run = False
+
+class SomeCommand:
+ def __init__(self, debugger, unused):
+ self.dbg = debugger
+ def __call__(self, debugger, command, exe_ctx, result):
+ global did_run
+ did_run = True
+ result.PutCString("some output\n")
+
+ def get_short_help(self):
+ return "Test command - sets a variable."
+
+class OtherCommand:
+ def __init__(self, debugger, unused):
+ self.dbg = debugger
+ def __call__(self, debugger, command, exe_ctx, result):
+ global did_run
+ if did_run:
+ result.SetStatus(lldb.eReturnStatusSuccessFinishNoResult)
+ else:
+ result.SetStatus(lldb.eReturnStatusFailed)
+
+ def get_short_help(self):
+ return "Test command - sets a variable."
+
+def __lldb_init_module(debugger, unused):
+ print("Adding command some-cmd and report-cmd")
+ debugger.HandleCommand("command script add -c some_cmd.SomeCommand some-cmd")
+ debugger.HandleCommand("command script add -c some_cmd.OtherCommand report-cmd")
Index: lldb/test/API/api/multithreaded/TestMultithreaded.py
===================================================================
--- lldb/test/API/api/multithreaded/TestMultithreaded.py
+++ lldb/test/API/api/multithreaded/TestMultithreaded.py
@@ -23,9 +23,19 @@
self.generateSource('test_listener_event_description.cpp')
self.generateSource('test_listener_event_process_state.cpp')
self.generateSource('test_listener_resume.cpp')
+ self.generateSource('test_stop-hook.cpp')
mydir = TestBase.compute_mydir(__file__)
+ @skipIfRemote
+ @skipIfNoSBHeaders
+ # clang-cl does not support throw or catch (llvm.org/pr24538)
+ @skipIfWindows
+ def test_python_stop_hook(self):
+ """Test that you can run a python command in a stop-hook when stdin is File based. """
+ self.build_and_test('driver.cpp test_stop-hook.cpp',
+ 'test_python_stop_hook')
+
@skipIfRemote
@skipIfNoSBHeaders
# clang-cl does not support throw or catch (llvm.org/pr24538)
Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3286,6 +3286,7 @@
CommandReturnObject result(false);
result.SetImmediateOutputStream(output_sp);
+ result.SetInteractive(false);
Debugger &debugger = exc_ctx.GetTargetPtr()->GetDebugger();
CommandInterpreterRunOptions options;
options.SetStopOnContinue(true);
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===================================================================
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2307,6 +2307,8 @@
}
CommandReturnObject tmp_result(m_debugger.GetUseColor());
+ tmp_result.SetInteractive(result.GetInteractive());
+
// If override_context is not NULL, pass no_context_switching = true for
// HandleCommand() since we updated our context already.
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1927,6 +1927,7 @@
header.startswith("SB") and header.endswith(".h"))]
includes = '\n'.join(list)
new_content = content.replace('%include_SB_APIs%', includes)
+ new_content = new_content.replace('%SOURCE_DIR%', self.getSourceDir())
src = os.path.join(self.getBuildDir(), source)
with open(src, 'w') as f:
f.write(new_content)
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
