[lldb] Make sure SBError is valid when SBDebugger::InitializeWithErrorHandling succeeds #170156
Conversation
…rHandling succeeds Fixes llvm#169788 When this function fails to initialise the debugger, it sets the SBError using the returned error from the initialise function. This results in Success being false and isVaid being true. This is expected behaviour. When it does not fail to initialise, it was returning the default constructed SBError which has Success() == true but IsValid == false. IsValid should be true, to show that the success can be trusted. To fix this, construct the SBError using a default constructed Status, which results in Success and IsValid being true.
|
@llvm/pr-subscribers-lldb Author: David Spickett (DavidSpickett) ChangesFixes #169788 When this function fails to initialise the debugger, it sets the SBError using the returned error from the initialise function. This results in Success being false and isVaid being true. This is expected behaviour. When it does not fail to initialise, it was returning the default constructed SBError which has Success() == true but IsValid == false. IsValid should be true, to show that the success can be trusted. To fix this, construct the SBError using a default constructed Status, which results in Success and IsValid being true. Full diff: https://github.com/llvm/llvm-project/pull/170156.diff 2 Files Affected:
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 7a4bebfdf998e..f939955ba57c8 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -179,7 +179,7 @@ void SBDebugger::Initialize() {
lldb::SBError SBDebugger::InitializeWithErrorHandling() {
LLDB_INSTRUMENT();
- SBError error;
+ SBError error((Status()));
if (auto e = g_debugger_lifetime->Initialize(
std::make_unique<SystemInitializerFull>())) {
error.SetError(Status::FromError(std::move(e)));
diff --git a/lldb/unittests/DAP/TestBase.cpp b/lldb/unittests/DAP/TestBase.cpp
index 8cb459964f7d8..f4dde9559e9d3 100644
--- a/lldb/unittests/DAP/TestBase.cpp
+++ b/lldb/unittests/DAP/TestBase.cpp
@@ -72,6 +72,7 @@ void DAPTestBase::TearDown() {
void DAPTestBase::SetUpTestSuite() {
lldb::SBError error = SBDebugger::InitializeWithErrorHandling();
+ EXPECT_TRUE(error.IsValid());
EXPECT_TRUE(error.Success());
}
void DAPTestBase::TeatUpTestSuite() { SBDebugger::Terminate(); }
|
JDevlieghere
left a comment
JDevlieghere
left a comment
There was a problem hiding this comment.
LGTM, but I wonder if we shouldn't change the SBError default constructor to always point to a success/valid Status?
3 participants
Fixes #169788
When this function fails to initialise the debugger, it sets the SBError using the returned error from the initialise function. This results in Success being false and isVaid being true. This is expected behaviour.
When it does not fail to initialise, it was returning the default constructed SBError which has Success() == true but IsValid == false. IsValid should be true, to show that the success can be trusted.
To fix this, construct the SBError using a default constructed Status, which results in Success and IsValid being true.