tatyana-krasnukha created this revision. tatyana-krasnukha added reviewers: jingham, JDevlieghere, labath. tatyana-krasnukha added a project: LLDB. Herald added a subscriber: lldb-commits. tatyana-krasnukha requested review of this revision.
SBDebugger asks CommandInterpreter for execution context, however, the interpreter's context will not be updated until a command will be executed (may never happen when using SB API). It means that the behavior of these functions depends on previous user actions. The context can stay uninitialized, point to a currently selected target, or point to one of the previously selected targets. This patch makes SBDebugger use execution context built upon the selected target. Notice, that even SBCommandInterpreter::GetProcess doesn't use CommandInterpreter's execution context. Also, add error logging to GetInternalVariableValue. Added test reproduces the issue. Without this fix, the last assertion fails because the interpreter's execution context is empty until running "target list", so, the value of the global property was updated instead of the process's local instance. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D92164 Files: lldb/packages/Python/lldbsuite/test/python_api/debugger/Makefile lldb/packages/Python/lldbsuite/test/python_api/debugger/main.cpp lldb/source/API/SBDebugger.cpp lldb/test/API/python_api/debugger/TestDebuggerAPI.py
Index: lldb/test/API/python_api/debugger/TestDebuggerAPI.py =================================================================== --- lldb/test/API/python_api/debugger/TestDebuggerAPI.py +++ lldb/test/API/python_api/debugger/TestDebuggerAPI.py @@ -43,3 +43,54 @@ target = lldb.SBTarget() self.assertFalse(target.IsValid()) self.dbg.DeleteTarget(target) + + def test_debugger_internal_variable(self): + """Ensure that SBDebugger reachs the same instance of properties + regardless CommandInterpreter's context initialization""" + self.build() + exe = self.getBuildArtifact("a.out") + + # Create a target by the debugger. + target = self.dbg.CreateTarget(exe) + self.assertTrue(target, VALID_TARGET) + + property_name = "target.process.memory-cache-line-size" + + def get_cache_line_size(): + value_list = lldb.SBStringList() + value_list = self.dbg.GetInternalVariableValue(property_name, + self.dbg.GetInstanceName()) + + self.assertEqual(value_list.GetSize(), 1) + try: + return int(value_list.GetStringAtIndex(0)) + except ValueError as error: + self.fail("Value is not a number: " + error) + + # Get global property value while there are no processes. + global_cache_line_size = get_cache_line_size() + + # Run a process via SB interface. CommandInterpreter's execution context + # remains empty. + error = lldb.SBError() + launch_info = lldb.SBLaunchInfo(None) + launch_info.SetLaunchFlags(lldb.eLaunchFlagStopAtEntry) + process = target.Launch(launch_info, error) + self.assertTrue(process, PROCESS_IS_VALID) + + # This should change the value of a process's local property. + new_cache_line_size = global_cache_line_size + 512 + error = self.dbg.SetInternalVariable(property_name, + str(new_cache_line_size), + self.dbg.GetInstanceName()) + self.assertTrue(error.Success(), + property_name + " value was changed successfully") + + # Check that it was set actually. + self.assertEqual(get_cache_line_size(), new_cache_line_size) + + # Run any command to initialize CommandInterpreter's execution context. + self.runCmd("target list") + + # Test the local property again, is it set to new_cache_line_size? + self.assertEqual(get_cache_line_size(), new_cache_line_size) Index: lldb/source/API/SBDebugger.cpp =================================================================== --- lldb/source/API/SBDebugger.cpp +++ lldb/source/API/SBDebugger.cpp @@ -1268,16 +1268,18 @@ SBError sb_error; DebuggerSP debugger_sp(Debugger::FindDebuggerWithInstanceName( ConstString(debugger_instance_name))); - Status error; - if (debugger_sp) { - ExecutionContext exe_ctx( - debugger_sp->GetCommandInterpreter().GetExecutionContext()); - error = debugger_sp->SetPropertyValue(&exe_ctx, eVarSetOperationAssign, - var_name, value); - } else { - error.SetErrorStringWithFormat("invalid debugger instance name '%s'", - debugger_instance_name); + + if (!debugger_sp) { + sb_error.SetErrorStringWithFormat("invalid debugger instance name '%s'", + debugger_instance_name); + return LLDB_RECORD_RESULT(sb_error); } + + ExecutionContext exe_ctx(debugger_sp->GetSelectedTarget(), true); + + Status error = debugger_sp->SetPropertyValue(&exe_ctx, eVarSetOperationAssign, + var_name, value); + if (error.Fail()) sb_error.SetError(error); return LLDB_RECORD_RESULT(sb_error); @@ -1290,27 +1292,33 @@ lldb::SBStringList, SBDebugger, GetInternalVariableValue, (const char *, const char *), var_name, debugger_instance_name); - SBStringList ret_value; DebuggerSP debugger_sp(Debugger::FindDebuggerWithInstanceName( ConstString(debugger_instance_name))); + if (!debugger_sp) { + Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API)); + if (log) + log->Error("invalid debugger instance name %s", debugger_instance_name); + return LLDB_RECORD_RESULT(SBStringList()); + } + + ExecutionContext exe_ctx(debugger_sp->GetSelectedTarget(), true); + Status error; - if (debugger_sp) { - ExecutionContext exe_ctx( - debugger_sp->GetCommandInterpreter().GetExecutionContext()); - lldb::OptionValueSP value_sp( - debugger_sp->GetPropertyValue(&exe_ctx, var_name, false, error)); - if (value_sp) { - StreamString value_strm; - value_sp->DumpValue(&exe_ctx, value_strm, OptionValue::eDumpOptionValue); - const std::string &value_str = std::string(value_strm.GetString()); - if (!value_str.empty()) { - StringList string_list; - string_list.SplitIntoLines(value_str); - return LLDB_RECORD_RESULT(SBStringList(&string_list)); - } - } + lldb::OptionValueSP value_sp( + debugger_sp->GetPropertyValue(&exe_ctx, var_name, false, error)); + + if (!value_sp) { + Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API)); + LLDB_LOG_ERROR(log, error.ToError(), "cannot get property value: {0}"); + return LLDB_RECORD_RESULT(SBStringList()); } - return LLDB_RECORD_RESULT(SBStringList()); + + StreamString value_strm; + value_sp->DumpValue(&exe_ctx, value_strm, OptionValue::eDumpOptionValue); + + StringList string_list; + string_list.SplitIntoLines(std::string(value_strm.GetString()); + return LLDB_RECORD_RESULT(SBStringList(&string_list)); } uint32_t SBDebugger::GetTerminalWidth() const { Index: lldb/packages/Python/lldbsuite/test/python_api/debugger/main.cpp =================================================================== --- /dev/null +++ lldb/packages/Python/lldbsuite/test/python_api/debugger/main.cpp @@ -0,0 +1,18 @@ +//===-- main.cpp ------------------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + + +// This simple program is to test the lldb Python API SBDebugger. + +int func(int val) { + return val - 1; +} + +int main (int argc, char const *argv[]) { + return func(argc); +} Index: lldb/packages/Python/lldbsuite/test/python_api/debugger/Makefile =================================================================== --- /dev/null +++ lldb/packages/Python/lldbsuite/test/python_api/debugger/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits