[Lldb-commits] [lldb] a92f783 - Fix the run locker setting for async launches that don't stop at the
Jim Ingham via lldb-commits
lldb-commits at lists.llvm.org
Tue Feb 28 17:35:16 PST 2023
Author: Jim Ingham
Date: 2023-02-28T17:34:49-08:00
New Revision: a92f7832f35c6c4792d8693e724c19802da75b36
URL: https://github.com/llvm/llvm-project/commit/a92f7832f35c6c4792d8693e724c19802da75b36
DIFF: https://github.com/llvm/llvm-project/commit/a92f7832f35c6c4792d8693e724c19802da75b36.diff
LOG: Fix the run locker setting for async launches that don't stop at the
initial stop. The code was using PrivateResume when it should have
used Resume.
This was allowing expression evaluation while the target was running,
and though that was caught a litle later on, we should never have gotten
that far. To make sure that this is caught immediately I made an error
SBValue when this happens, and test that we get this error.
Differential Revision: https://reviews.llvm.org/D144665
Added:
lldb/test/API/python_api/run_locker/Makefile
lldb/test/API/python_api/run_locker/TestRunLocker.py
lldb/test/API/python_api/run_locker/main.c
Modified:
lldb/source/API/SBFrame.cpp
lldb/source/API/SBTarget.cpp
lldb/source/Target/Target.cpp
Removed:
################################################################################
diff --git a/lldb/source/API/SBFrame.cpp b/lldb/source/API/SBFrame.cpp
index eb7ec3bbf8d68..bf833fbb95615 100644
--- a/lldb/source/API/SBFrame.cpp
+++ b/lldb/source/API/SBFrame.cpp
@@ -19,6 +19,7 @@
#include "lldb/Core/StreamFile.h"
#include "lldb/Core/ValueObjectRegister.h"
#include "lldb/Core/ValueObjectVariable.h"
+#include "lldb/Core/ValueObjectConstResult.h"
#include "lldb/Expression/ExpressionVariable.h"
#include "lldb/Expression/UserExpression.h"
#include "lldb/Host/Host.h"
@@ -988,6 +989,12 @@ SBValue SBFrame::EvaluateExpression(const char *expr) {
else
options.SetLanguage(frame->GetLanguage());
return EvaluateExpression(expr, options);
+ } else {
+ Status error;
+ error.SetErrorString("can't evaluate expressions when the "
+ "process is running.");
+ ValueObjectSP error_val_sp = ValueObjectConstResult::Create(nullptr, error);
+ result.SetSP(error_val_sp, false);
}
return result;
}
@@ -1051,7 +1058,6 @@ lldb::SBValue SBFrame::EvaluateExpression(const char *expr,
std::unique_lock<std::recursive_mutex> lock;
ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
-
StackFrame *frame = nullptr;
Target *target = exe_ctx.GetTargetPtr();
Process *process = exe_ctx.GetProcessPtr();
@@ -1075,13 +1081,30 @@ lldb::SBValue SBFrame::EvaluateExpression(const char *expr,
target->EvaluateExpression(expr, frame, expr_value_sp, options.ref());
expr_result.SetSP(expr_value_sp, options.GetFetchDynamicValue());
}
+ } else {
+ Status error;
+ error.SetErrorString("can't evaluate expressions when the "
+ "process is running.");
+ expr_value_sp = ValueObjectConstResult::Create(nullptr, error);
+ expr_result.SetSP(expr_value_sp, false);
}
+ } else {
+ Status error;
+ error.SetErrorString("sbframe object is not valid.");
+ expr_value_sp = ValueObjectConstResult::Create(nullptr, error);
+ expr_result.SetSP(expr_value_sp, false);
}
- LLDB_LOGF(expr_log,
- "** [SBFrame::EvaluateExpression] Expression result is "
- "%s, summary %s **",
- expr_result.GetValue(), expr_result.GetSummary());
+ if (expr_result.GetError().Success())
+ LLDB_LOGF(expr_log,
+ "** [SBFrame::EvaluateExpression] Expression result is "
+ "%s, summary %s **",
+ expr_result.GetValue(), expr_result.GetSummary());
+ else
+ LLDB_LOGF(expr_log,
+ "** [SBFrame::EvaluateExpression] Expression evaluation failed: "
+ "%s **",
+ expr_result.GetError().GetCString());
return expr_result;
}
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index cadaeb472e289..6e5942904db78 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -2219,12 +2219,25 @@ lldb::SBValue SBTarget::EvaluateExpression(const char *expr,
std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
ExecutionContext exe_ctx(m_opaque_sp.get());
-
frame = exe_ctx.GetFramePtr();
Target *target = exe_ctx.GetTargetPtr();
+ Process *process = exe_ctx.GetProcessPtr();
if (target) {
- target->EvaluateExpression(expr, frame, expr_value_sp, options.ref());
+ // If we have a process, make sure to lock the runlock:
+ if (process) {
+ Process::StopLocker stop_locker;
+ if (stop_locker.TryLock(&process->GetRunLock())) {
+ target->EvaluateExpression(expr, frame, expr_value_sp, options.ref());
+ } else {
+ Status error;
+ error.SetErrorString("can't evaluate expressions when the "
+ "process is running.");
+ expr_value_sp = ValueObjectConstResult::Create(nullptr, error);
+ }
+ } else {
+ target->EvaluateExpression(expr, frame, expr_value_sp, options.ref());
+ }
expr_result.SetSP(expr_value_sp, options.GetFetchDynamicValue());
}
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 2b3b92a3cad46..334bb0d54f2e3 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3223,7 +3223,7 @@ Status Target::Launch(ProcessLaunchInfo &launch_info, Stream *stream) {
// SyncResume hijacker.
m_process_sp->ResumeSynchronous(stream);
else
- error = m_process_sp->PrivateResume();
+ error = m_process_sp->Resume();
if (!error.Success()) {
Status error2;
error2.SetErrorStringWithFormat(
diff --git a/lldb/test/API/python_api/run_locker/Makefile b/lldb/test/API/python_api/run_locker/Makefile
new file mode 100644
index 0000000000000..695335e068c0c
--- /dev/null
+++ b/lldb/test/API/python_api/run_locker/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -std=c99
+
+include Makefile.rules
diff --git a/lldb/test/API/python_api/run_locker/TestRunLocker.py b/lldb/test/API/python_api/run_locker/TestRunLocker.py
new file mode 100644
index 0000000000000..191400d6c024f
--- /dev/null
+++ b/lldb/test/API/python_api/run_locker/TestRunLocker.py
@@ -0,0 +1,89 @@
+"""
+Test that the run locker really does work to keep
+us from running SB API that should only be run
+while stopped. This test is mostly concerned with
+what happens between launch and first stop.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class TestRunLocker(TestBase):
+
+ NO_DEBUG_INFO_TESTCASE = True
+
+ def test_run_locker(self):
+ """Test that the run locker is set correctly when we launch"""
+ self.build()
+ self.runlocker_test(False)
+
+ def test_run_locker_stop_at_entry(self):
+ """Test that the run locker is set correctly when we launch"""
+ self.build()
+ self.runlocker_test(False)
+
+ def setUp(self):
+ # Call super's setUp().
+ TestBase.setUp(self)
+ self.main_source_file = lldb.SBFileSpec("main.c")
+
+ def runlocker_test(self, stop_at_entry):
+ """The code to stop at entry handles events slightly
diff erently, so
+ we test both versions of process launch."""
+
+ target = lldbutil.run_to_breakpoint_make_target(self)
+
+ launch_info = target.GetLaunchInfo()
+ if stop_at_entry:
+ flags = launch_info.GetFlags()
+ launch_info.SetFlags(flags | lldb.eLaunchFlagStopAtEntry)
+
+ error = lldb.SBError()
+ # We are trying to do things when the process is running, so
+ # we have to run the debugger asynchronously.
+ self.dbg.SetAsync(True)
+
+ listener = lldb.SBListener("test-run-lock-listener")
+ launch_info.SetListener(listener)
+ process = target.Launch(launch_info, error)
+ self.assertSuccess(error, "Launched the process")
+
+ event = lldb.SBEvent()
+
+ event_result = listener.WaitForEvent(10, event)
+ self.assertTrue(event_result, "timed out waiting for launch")
+ state_type = lldb.SBProcess.GetStateFromEvent(event)
+ # We don't always see a launching...
+ if state_type == lldb.eStateLaunching:
+ event_result = listener.WaitForEvent(10, event)
+ self.assertTrue(event_result, "Timed out waiting for running after launching")
+ state_type = lldb.SBProcess.GetStateFromEvent(event)
+
+ self.assertState(state_type, lldb.eStateRunning, "Didn't get a running event")
+
+ # We aren't checking the entry state, but just making sure
+ # the running state is set properly if we continue in this state.
+
+ if stop_at_entry:
+ event_result = listener.WaitForEvent(10, event)
+ self.assertTrue(event_result, "Timed out waiting for stop at entry stop")
+ state_type = lldb.SBProcess.GetStateFromEvent(event)
+ self.assertState(state_type, eStateStopped, "Stop at entry stopped")
+ process.Continue()
+
+ # Okay, now the process is running, make sure we can't do things
+ # you aren't supposed to do while running, and that we get some
+ # actual error:
+ val = target.EvaluateExpression("SomethingToCall()")
+ error = val.GetError()
+ self.assertTrue(error.Fail(), "Failed to run expression")
+ print(f"Got Error: {error.GetCString()}")
+ self.assertIn("can't evaluate expressions when the process is running", error.GetCString(), "Stopped by stop locker")
+
+ # This should also fail if we try to use the script interpreter directly:
+ interp = self.dbg.GetCommandInterpreter()
+ result = lldb.SBCommandReturnObject()
+ ret = interp.HandleCommand("script var = lldb.frame.EvaluateExpression('SomethingToCall()'); var.GetError().GetCString()", result)
+ self.assertIn("can't evaluate expressions when the process is running", result.GetOutput())
diff --git a/lldb/test/API/python_api/run_locker/main.c b/lldb/test/API/python_api/run_locker/main.c
new file mode 100644
index 0000000000000..2cdb68c740da1
--- /dev/null
+++ b/lldb/test/API/python_api/run_locker/main.c
@@ -0,0 +1,15 @@
+#include <unistd.h>
+
+int
+SomethingToCall() {
+ return 100;
+}
+
+int
+main()
+{
+ while (1) {
+ sleep(1);
+ }
+ return SomethingToCall();
+}
More information about the lldb-commits
mailing list