[Lldb-commits] [PATCH] D120319: Set error message if ValueObjectRegister fails to write back to register

[Ilya Nozhkin via Phabricator via lldb-commits]

ilya-nozhkin updated this revision to Diff 411065.
ilya-nozhkin added a comment.

Replaced redundant check in the test with assert. Also, I've removed type annotations to be consistent with the code around.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120319/new/

https://reviews.llvm.org/D120319

Files:
  lldb/source/Core/ValueObjectRegister.cpp
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py


Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===================================================================
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -133,6 +133,29 @@
         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
Index: lldb/source/Core/ValueObjectRegister.cpp
===================================================================
--- lldb/source/Core/ValueObjectRegister.cpp
+++ lldb/source/Core/ValueObjectRegister.cpp
@@ -269,26 +269,30 @@
   // 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) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D120319.411065.patch
Type: text/x-patch
Size: 2749 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20220224/4bc5abfa/attachment-0001.bin>