Author: Ilya Nozhkin Date: 2022-02-28T14:29:29+08:00 New Revision: fd37d489cfef3bf7d06b2b43dac2ad1381cdc56b
URL: https://github.com/llvm/llvm-project/commit/fd37d489cfef3bf7d06b2b43dac2ad1381cdc56b DIFF: https://github.com/llvm/llvm-project/commit/fd37d489cfef3bf7d06b2b43dac2ad1381cdc56b.diff LOG: Set error message if ValueObjectRegister fails to write back to register SetValueFromCString and SetData methods return false if register can't be written but they don't set a error message. It sometimes confuses callers of these methods because they try to get the error message in case of failure but Status::AsCString returns nullptr. For example, lldb-vscode crashes due to this bug if some register can't be written. It invokes SBError::GetCString in case of error and doesn't check whether the result is nullptr (see request_setVariable implementation in lldb-vscode.cpp for more info). Reviewed By: labath, clayborg Differential Revision: https://reviews.llvm.org/D120319 Added: Modified: lldb/source/Core/ValueObjectRegister.cpp lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py Removed: ################################################################################ diff --git a/lldb/source/Core/ValueObjectRegister.cpp b/lldb/source/Core/ValueObjectRegister.cpp index fc436385e2455..e342f7265dfb1 100644 --- a/lldb/source/Core/ValueObjectRegister.cpp +++ b/lldb/source/Core/ValueObjectRegister.cpp @@ -269,26 +269,30 @@ bool ValueObjectRegister::SetValueFromCString(const char *value_str, // The new value will be in the m_data. Copy that into our register value. error = m_reg_value.SetValueFromString(&m_reg_info, llvm::StringRef(value_str)); - if (error.Success()) { - if (m_reg_ctx_sp->WriteRegister(&m_reg_info, m_reg_value)) { - SetNeedsUpdate(); - return true; - } else - return false; - } else + if (!error.Success()) return false; + + if (!m_reg_ctx_sp->WriteRegister(&m_reg_info, m_reg_value)) { + error.SetErrorString("unable to write back to register"); + return false; + } + + SetNeedsUpdate(); + return true; } bool ValueObjectRegister::SetData(DataExtractor &data, Status &error) { error = m_reg_value.SetValueFromData(&m_reg_info, data, 0, false); - if (error.Success()) { - if (m_reg_ctx_sp->WriteRegister(&m_reg_info, m_reg_value)) { - SetNeedsUpdate(); - return true; - } else - return false; - } else + if (!error.Success()) return false; + + if (!m_reg_ctx_sp->WriteRegister(&m_reg_info, m_reg_value)) { + error.SetErrorString("unable to write back to register"); + return false; + } + + SetNeedsUpdate(); + return true; } bool ValueObjectRegister::ResolveValue(Scalar &scalar) { diff --git a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py index 46ecf2e5faeb8..d1948b6752753 100644 --- a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py +++ b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py @@ -133,6 +133,29 @@ def test_read_memory(self): self.assertEqual(len(bytesread), 16) self.dbg.DeleteTarget(target) + @skipIfLLVMTargetMissing("X86") + def test_write_register(self): + """Test that writing to register results in an error and that error + message is set.""" + target = self.dbg.CreateTarget("linux-x86_64.out") + process = target.LoadCore("linux-x86_64.core") + self.assertTrue(process, PROCESS_IS_VALID) + + thread = process.GetSelectedThread() + self.assertTrue(thread) + + frame = thread.GetSelectedFrame() + self.assertTrue(frame) + + reg_value = frame.FindRegister('eax') + self.assertTrue(reg_value) + + error = lldb.SBError() + success = reg_value.SetValueFromCString('10', error) + self.assertFalse(success) + self.assertTrue(error.Fail()) + self.assertIsNotNone(error.GetCString()) + @skipIfLLVMTargetMissing("X86") def test_FPR_SSE(self): # check x86_64 core file _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits