ilya-nozhkin created this revision.
ilya-nozhkin added reviewers: labath, clayborg.
Herald added a subscriber: pengfei.
ilya-nozhkin requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
`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).
I couldn't write any test for this issue on x86 architecture because all x86
registers are writable. It might be possible to add such test for some other
testable architecture but I don't know much about them and I don't have a
machine with any architecture other than x86 to actually run the test.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D120319
Files:
lldb/source/Core/ValueObjectRegister.cpp
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/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) {
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
