date:20220227

[Lldb-commits] [lldb] 850bc76 - [lldb] BreakpointOptions::CommandData::CreateFromStructuredData - remove dead code + variable. NFCI.

2022-02-27 Thread Simon Pilgrim via lldb-commits


Author: Simon Pilgrim
Date: 2022-02-27T11:33:14Z
New Revision: 850bc76a356b050b92851ad5a6a8207da05685cd

URL: 
https://github.com/llvm/llvm-project/commit/850bc76a356b050b92851ad5a6a8207da05685cd
DIFF: 
https://github.com/llvm/llvm-project/commit/850bc76a356b050b92851ad5a6a8207da05685cd.diff

LOG: [lldb] BreakpointOptions::CommandData::CreateFromStructuredData - remove 
dead code + variable. NFCI.

The found_something bool is only ever read after it has always been set to true.

Looks to be a leftover debugging variable.

Fixes static analyzer warning: 
https://llvm.org/reports/scan-build/report-BreakpointOptions.cpp-CreateFromStructuredData-8-4055b9.html#EndPath

Added: 


Modified: 
lldb/source/Breakpoint/BreakpointOptions.cpp

Removed: 




diff  --git a/lldb/source/Breakpoint/BreakpointOptions.cpp 
b/lldb/source/Breakpoint/BreakpointOptions.cpp
index 3dcb1904c8f8f..6fc6948aaccc2 100644
--- a/lldb/source/Breakpoint/BreakpointOptions.cpp
+++ b/lldb/source/Breakpoint/BreakpointOptions.cpp
@@ -60,14 +60,10 @@ std::unique_ptr
 BreakpointOptions::CommandData::CreateFromStructuredData(
 const StructuredData::Dictionary &options_dict, Status &error) {
   std::unique_ptr data_up(new CommandData());
-  bool found_something = false;
 
   bool success = options_dict.GetValueForKeyAsBoolean(
   GetKey(OptionNames::StopOnError), data_up->stop_on_error);
 
-  if (success)
-found_something = true;
-
   llvm::StringRef interpreter_str;
   ScriptLanguage interp_language;
   success = options_dict.GetValueForKeyAsString(
@@ -78,7 +74,6 @@ BreakpointOptions::CommandData::CreateFromStructuredData(
 return data_up;
   }
 
-  found_something = true;
   interp_language = ScriptInterpreter::StringToLanguage(interpreter_str);
   if (interp_language == eScriptLanguageUnknown) {
 error.SetErrorStringWithFormatv("Unknown breakpoint command language: 
{0}.",
@@ -91,7 +86,6 @@ BreakpointOptions::CommandData::CreateFromStructuredData(
   success = options_dict.GetValueForKeyAsArray(GetKey(OptionNames::UserSource),
user_source);
   if (success) {
-found_something = true;
 size_t num_elems = user_source->GetSize();
 for (size_t i = 0; i < num_elems; i++) {
   llvm::StringRef elem_string;
@@ -101,10 +95,7 @@ BreakpointOptions::CommandData::CreateFromStructuredData(
 }
   }
 
-  if (found_something)
-return data_up;
-  else
-return std::unique_ptr();
+  return data_up;
 }
 
 const char *BreakpointOptions::g_option_names[(



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2022-02-27 Thread Greg Clayton via Phabricator via lldb-commits

clayborg accepted this revision.
clayborg added a comment.

Great, thanks for adding the test.


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

https://reviews.llvm.org/D120319

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D113616: [lldb] Hyphenate Objective-C exception breakpoint labels ✍️

2022-02-27 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

Very easy change as VS Code IDE displays these strings and no one references 
these by name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113616

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2022-02-27 Thread Ilya Nozhkin via Phabricator via lldb-commits

ilya-nozhkin added a comment.

Could someone, please, commit these changes to the main repository? I still 
don't have commit access. I'll apply for it right after merging these changes 
(LLVM Developer Policy states that it'd be better to have several patches 
already submitted before applying for commit access).


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

https://reviews.llvm.org/D120319

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2022-02-27 Thread Chuanqi Xu via Phabricator via lldb-commits

ChuanqiXu added a comment.

In D120319#3348265 , @ilya-nozhkin 
wrote:

> Could someone, please, commit these changes to the main repository? I still 
> don't have commit access. I'll apply for it right after merging these changes 
> (LLVM Developer Policy states that it'd be better to have several patches 
> already submitted before applying for commit access).

What's your name and email address for this patch? I'd love to commit it.


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

https://reviews.llvm.org/D120319

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2022-02-27 Thread Ilya Nozhkin via Phabricator via lldb-commits

ilya-nozhkin added a comment.

In D120319#3348267 , @ChuanqiXu wrote:

> What's your name and email address for this patch?

Ilya Nozhkin
nozhkin...@gmail.com


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

https://reviews.llvm.org/D120319

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2022-02-27 Thread Chuanqi Xu via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfd37d489cfef: Set error message if ValueObjectRegister fails 
to write back to register (authored by ilya-nozhkin, committed by ChuanqiXu).

Repository:
  rG LLVM Github Monorepo

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) {


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->

[Lldb-commits] [lldb] fd37d48 - Set error message if ValueObjectRegister fails to write back to register

2022-02-27 Thread Chuanqi Xu via lldb-commits


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

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

2022-02-27 Thread Chuanqi Xu via Phabricator via lldb-commits

ChuanqiXu added a comment.

In D120319#3348268 , @ilya-nozhkin 
wrote:

> In D120319#3348267 , @ChuanqiXu 
> wrote:
>
>> What's your name and email address for this patch?
>
> Ilya Nozhkin
> nozhkin...@gmail.com

Done~


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120319

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 850bc76 - [lldb] BreakpointOptions::CommandData::CreateFromStructuredData - remove dead code + variable. NFCI.

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

[Lldb-commits] [PATCH] D113616: [lldb] Hyphenate Objective-C exception breakpoint labels ✍️

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

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

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

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

[Lldb-commits] [lldb] fd37d48 - Set error message if ValueObjectRegister fails to write back to register

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

9 matches

Site Navigation

Mail list logo

Footer information