Skip to content

Commit

Permalink
Fix the run locker setting for async launches that don't stop at the
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jimingham committed Mar 1, 2023
1 parent 740e2e9 commit a92f783
Show file tree
Hide file tree
Showing 6 changed files with 152 additions and 8 deletions.
33 changes: 28 additions & 5 deletions lldb/source/API/SBFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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();
Expand All @@ -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;
}
Expand Down
17 changes: 15 additions & 2 deletions lldb/source/API/SBTarget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Target/Target.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 4 additions & 0 deletions lldb/test/API/python_api/run_locker/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
C_SOURCES := main.c
CFLAGS_EXTRAS := -std=c99

include Makefile.rules
89 changes: 89 additions & 0 deletions lldb/test/API/python_api/run_locker/TestRunLocker.py
Original file line number Diff line number Diff line change
@@ -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 differently, 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())
15 changes: 15 additions & 0 deletions lldb/test/API/python_api/run_locker/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#include <unistd.h>

int
SomethingToCall() {
return 100;
}

int
main()
{
while (1) {
sleep(1);
}
return SomethingToCall();
}

0 comments on commit a92f783

Please sign in to comment.