[debugger] Differentiate async and known-unknown regs.
Register values can have three states:
- Known.
- Known unknown (register is not available).
- Requires async query.
Previously the first two states were conflated, requiring an async call
to differentiate unavailable and asynchronously availab.e
We are moving to keeping all known general registers on the frame so
all general registers will either be known or known-unknown. This is not
hooked up yet, but when it is, known non-general registers (e.g. vector
and debug) will require async calls to get the value for the top stack
frame.
No user-visible change, should be covered by existing tests. One test
was updated to account for the new synchronously known to be unknown
state.
Change-Id: I5ba58bc100e44fae5fd85a8f93cb5a49574e43b9
diff --git a/src/developer/debug/zxdb/client/frame_symbol_data_provider.cc b/src/developer/debug/zxdb/client/frame_symbol_data_provider.cc
index 45c9c07..ac1087e 100644
--- a/src/developer/debug/zxdb/client/frame_symbol_data_provider.cc
+++ b/src/developer/debug/zxdb/client/frame_symbol_data_provider.cc
@@ -36,24 +36,27 @@
frame_ = nullptr;
}
-std::optional<uint64_t> FrameSymbolDataProvider::GetRegister(
- debug_ipc::RegisterID id) {
+bool FrameSymbolDataProvider::GetRegister(debug_ipc::RegisterID id,
+ std::optional<uint64_t>* value) {
+ *value = std::nullopt;
if (!frame_)
- return std::nullopt;
+ return true; // Synchronously know we don't have the value.
// Some common registers are known without having to do a register request.
switch (debug_ipc::GetSpecialRegisterType(id)) {
case debug_ipc::SpecialRegisterType::kIP:
- return frame_->GetAddress();
+ *value = frame_->GetAddress();
+ return true;
case debug_ipc::SpecialRegisterType::kSP:
- return frame_->GetStackPointer();
+ *value = frame_->GetStackPointer();
+ return true;
case debug_ipc::SpecialRegisterType::kNone:
break;
}
// TODO(brettw) enable synchronous access if the registers are cached.
// See GetRegisterAsync().
- return std::nullopt;
+ return false;
}
void FrameSymbolDataProvider::GetRegisterAsync(debug_ipc::RegisterID id,
diff --git a/src/developer/debug/zxdb/client/frame_symbol_data_provider.h b/src/developer/debug/zxdb/client/frame_symbol_data_provider.h
index 5b3028a..6168193 100644
--- a/src/developer/debug/zxdb/client/frame_symbol_data_provider.h
+++ b/src/developer/debug/zxdb/client/frame_symbol_data_provider.h
@@ -2,7 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#pragma once
+#ifndef SRC_DEVELOPER_DEBUG_ZXDB_CLIENT_FRAME_SYMBOL_DATA_PROVIDER_H_
+#define SRC_DEVELOPER_DEBUG_ZXDB_CLIENT_FRAME_SYMBOL_DATA_PROVIDER_H_
#include "src/developer/debug/zxdb/client/process_symbol_data_provider.h"
@@ -19,7 +20,8 @@
void Disown() override;
// SymbolDataProvider implementation:
- std::optional<uint64_t> GetRegister(debug_ipc::RegisterID id) override;
+ bool GetRegister(debug_ipc::RegisterID id,
+ std::optional<uint64_t>* value) override;
void GetRegisterAsync(debug_ipc::RegisterID id,
GetRegisterCallback callback) override;
std::optional<uint64_t> GetFrameBase() override;
@@ -42,3 +44,5 @@
};
} // namespace zxdb
+
+#endif // SRC_DEVELOPER_DEBUG_ZXDB_CLIENT_FRAME_SYMBOL_DATA_PROVIDER_H_
diff --git a/src/developer/debug/zxdb/expr/symbol_eval_context_unittest.cc b/src/developer/debug/zxdb/expr/symbol_eval_context_unittest.cc
index 2914e80..deadb6c 100644
--- a/src/developer/debug/zxdb/expr/symbol_eval_context_unittest.cc
+++ b/src/developer/debug/zxdb/expr/symbol_eval_context_unittest.cc
@@ -178,10 +178,7 @@
ValueResult result;
GetNamedValue(eval_context, "present", kQuitLoop, &result);
- // Running the message loop should complete the callback.
- loop().Run();
-
- // The value should be not found.
+ // The value should be not found and this should be known synchronously.
EXPECT_TRUE(result.called);
EXPECT_TRUE(result.err.has_error());
EXPECT_EQ(ExprValue(), result.value);
diff --git a/src/developer/debug/zxdb/expr/symbol_variable_resolver.cc b/src/developer/debug/zxdb/expr/symbol_variable_resolver.cc
index b164bab..4aba705 100644
--- a/src/developer/debug/zxdb/expr/symbol_variable_resolver.cc
+++ b/src/developer/debug/zxdb/expr/symbol_variable_resolver.cc
@@ -10,13 +10,11 @@
#include "src/developer/debug/zxdb/expr/expr_value.h"
#include "src/developer/debug/zxdb/expr/resolve_ptr_ref.h"
-#include "src/lib/fxl/strings/string_printf.h"
-#include "src/developer/debug/zxdb/expr/expr_value.h"
-#include "src/developer/debug/zxdb/expr/resolve_ptr_ref.h"
#include "src/developer/debug/zxdb/symbols/symbol_context.h"
#include "src/developer/debug/zxdb/symbols/symbol_data_provider.h"
#include "src/developer/debug/zxdb/symbols/type.h"
#include "src/developer/debug/zxdb/symbols/variable.h"
+#include "src/lib/fxl/strings/string_printf.h"
namespace zxdb {
@@ -38,9 +36,13 @@
return;
}
- auto ip = data_provider_->GetRegister(debug_ipc::GetSpecialRegisterID(
- data_provider_->GetArch(), debug_ipc::SpecialRegisterType::kIP));
+ std::optional<uint64_t> ip;
+ data_provider_->GetRegister(
+ debug_ipc::GetSpecialRegisterID(data_provider_->GetArch(),
+ debug_ipc::SpecialRegisterType::kIP),
+ &ip);
if (!ip) {
+ // The IP should never require an async call.
OnComplete(state, Err("No location available."), ExprValue());
return;
}
diff --git a/src/developer/debug/zxdb/symbols/dwarf_expr_eval.cc b/src/developer/debug/zxdb/symbols/dwarf_expr_eval.cc
index 6893655..5e64cb6 100644
--- a/src/developer/debug/zxdb/symbols/dwarf_expr_eval.cc
+++ b/src/developer/debug/zxdb/symbols/dwarf_expr_eval.cc
@@ -9,13 +9,13 @@
#include <utility>
-#include "src/lib/fxl/logging.h"
-#include "src/lib/fxl/strings/string_printf.h"
#include "llvm/BinaryFormat/Dwarf.h"
#include "llvm/Support/DataExtractor.h"
#include "src/developer/debug/shared/message_loop.h"
#include "src/developer/debug/zxdb/symbols/arch.h"
#include "src/developer/debug/zxdb/symbols/symbol_data_provider.h"
+#include "src/lib/fxl/logging.h"
+#include "src/lib/fxl/strings/string_printf.h"
namespace zxdb {
@@ -282,9 +282,15 @@
// This function doesn't set the result_type_ because it is called from
// different contexts. The callers should set the result_type_ as appropriate
// for their operation.
- if (auto reg_data = data_provider_->GetRegister(reg)) {
- // Register data available synchronously.
- Push(*reg_data + offset);
+ if (std::optional<uint64_t> reg_data;
+ data_provider_->GetRegister(reg, ®_data)) {
+ // State known synchronously (could be available or known unavailable).
+ if (!reg_data) {
+ ReportError(fxl::StringPrintf("Register %d not available.",
+ dwarf_register_number));
+ } else {
+ Push(*reg_data + offset);
+ }
return Completion::kSync;
}
diff --git a/src/developer/debug/zxdb/symbols/mock_symbol_data_provider.cc b/src/developer/debug/zxdb/symbols/mock_symbol_data_provider.cc
index a559cb1b..af86946 100644
--- a/src/developer/debug/zxdb/symbols/mock_symbol_data_provider.cc
+++ b/src/developer/debug/zxdb/symbols/mock_symbol_data_provider.cc
@@ -8,10 +8,10 @@
#include <algorithm>
-#include "src/lib/fxl/strings/string_printf.h"
#include "src/developer/debug/ipc/protocol.h"
#include "src/developer/debug/shared/message_loop.h"
#include "src/developer/debug/zxdb/common/err.h"
+#include "src/lib/fxl/strings/string_printf.h"
namespace zxdb {
@@ -32,19 +32,24 @@
return debug_ipc::Arch::kArm64;
}
-std::optional<uint64_t> MockSymbolDataProvider::GetRegister(
- debug_ipc::RegisterID id) {
- if (GetSpecialRegisterType(id) == debug_ipc::SpecialRegisterType::kIP)
- return ip_;
+bool MockSymbolDataProvider::GetRegister(debug_ipc::RegisterID id,
+ std::optional<uint64_t>* value) {
+ *value = std::nullopt;
+
+ if (GetSpecialRegisterType(id) == debug_ipc::SpecialRegisterType::kIP) {
+ *value = ip_;
+ return true;
+ }
const auto& found = regs_.find(id);
if (found == regs_.end())
- return std::nullopt;
+ return true; // Known to be unknown.
if (!found->second.synchronous)
- return std::nullopt; // Force synchronous query.
+ return false;
- return found->second.value;
+ *value = found->second.value;
+ return true;
}
void MockSymbolDataProvider::GetRegisterAsync(debug_ipc::RegisterID id,
diff --git a/src/developer/debug/zxdb/symbols/mock_symbol_data_provider.h b/src/developer/debug/zxdb/symbols/mock_symbol_data_provider.h
index 095122c..ec19a4b9 100644
--- a/src/developer/debug/zxdb/symbols/mock_symbol_data_provider.h
+++ b/src/developer/debug/zxdb/symbols/mock_symbol_data_provider.h
@@ -2,12 +2,13 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#pragma once
+#ifndef SRC_DEVELOPER_DEBUG_ZXDB_SYMBOLS_MOCK_SYMBOL_DATA_PROVIDER_H_
+#define SRC_DEVELOPER_DEBUG_ZXDB_SYMBOLS_MOCK_SYMBOL_DATA_PROVIDER_H_
#include <map>
-#include "src/lib/fxl/memory/weak_ptr.h"
#include "src/developer/debug/zxdb/symbols/symbol_data_provider.h"
+#include "src/lib/fxl/memory/weak_ptr.h"
namespace zxdb {
@@ -25,6 +26,8 @@
// Adds the given canned result for the given register. Set synchronous if
// the register contents should be synchronously available, false if it
// should require a callback to retrieve.
+ //
+ // Any registers not set will be synchronously reported as unknown.
void AddRegisterValue(debug_ipc::RegisterID id, bool synchronous,
uint64_t value);
@@ -37,7 +40,8 @@
// SymbolDataProvider implementation.
debug_ipc::Arch GetArch() override;
- std::optional<uint64_t> GetRegister(debug_ipc::RegisterID id) override;
+ bool GetRegister(debug_ipc::RegisterID id,
+ std::optional<uint64_t>* value) override;
void GetRegisterAsync(debug_ipc::RegisterID id,
GetRegisterCallback callback) override;
std::optional<uint64_t> GetFrameBase() override;
@@ -75,3 +79,5 @@
};
} // namespace zxdb
+
+#endif // SRC_DEVELOPER_DEBUG_ZXDB_SYMBOLS_MOCK_SYMBOL_DATA_PROVIDER_H_
diff --git a/src/developer/debug/zxdb/symbols/module_symbols_impl.cc b/src/developer/debug/zxdb/symbols/module_symbols_impl.cc
index 4b04fec..44b4d0ea 100644
--- a/src/developer/debug/zxdb/symbols/module_symbols_impl.cc
+++ b/src/developer/debug/zxdb/symbols/module_symbols_impl.cc
@@ -48,15 +48,11 @@
// SymbolDataProvider implementation.
debug_ipc::Arch GetArch() override { return debug_ipc::Arch::kUnknown; }
- std::optional<uint64_t> GetRegister(debug_ipc::RegisterID) override {
- return std::nullopt;
- }
void GetRegisterAsync(debug_ipc::RegisterID,
GetRegisterCallback callback) override {
debug_ipc::MessageLoop::Current()->PostTask(
FROM_HERE, [cb = std::move(callback)]() { cb(GetContextError(), 0); });
}
- std::optional<uint64_t> GetFrameBase() override { return std::nullopt; }
void GetFrameBaseAsync(GetRegisterCallback callback) override {
debug_ipc::MessageLoop::Current()->PostTask(
FROM_HERE, [cb = std::move(callback)]() { cb(GetContextError(), 0); });
diff --git a/src/developer/debug/zxdb/symbols/symbol_data_provider.cc b/src/developer/debug/zxdb/symbols/symbol_data_provider.cc
index fad581f..b56b67c 100644
--- a/src/developer/debug/zxdb/symbols/symbol_data_provider.cc
+++ b/src/developer/debug/zxdb/symbols/symbol_data_provider.cc
@@ -20,9 +20,10 @@
return debug_ipc::Arch::kUnknown;
}
-std::optional<uint64_t> SymbolDataProvider::GetRegister(
- debug_ipc::RegisterID id) {
- return std::nullopt;
+bool SymbolDataProvider::GetRegister(debug_ipc::RegisterID id,
+ std::optional<uint64_t>* value) {
+ *value = std::nullopt;
+ return true; // Known to be unknown.
}
void SymbolDataProvider::GetRegisterAsync(debug_ipc::RegisterID,
diff --git a/src/developer/debug/zxdb/symbols/symbol_data_provider.h b/src/developer/debug/zxdb/symbols/symbol_data_provider.h
index ce071fb5..76073af 100644
--- a/src/developer/debug/zxdb/symbols/symbol_data_provider.h
+++ b/src/developer/debug/zxdb/symbols/symbol_data_provider.h
@@ -2,7 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#pragma once
+#ifndef SRC_DEVELOPER_DEBUG_ZXDB_SYMBOLS_SYMBOL_DATA_PROVIDER_H_
+#define SRC_DEVELOPER_DEBUG_ZXDB_SYMBOLS_SYMBOL_DATA_PROVIDER_H_
#include <stdint.h>
@@ -10,8 +11,8 @@
#include <optional>
#include <vector>
-#include "src/lib/fxl/memory/ref_counted.h"
#include "src/developer/debug/ipc/protocol.h"
+#include "src/lib/fxl/memory/ref_counted.h"
namespace zxdb {
@@ -42,10 +43,17 @@
virtual debug_ipc::Arch GetArch();
- // Request for synchronous register data. If the register data can be provided
- // synchronously, the data will be returned. If synchronous data is not
- // available, the caller should call GetRegisterAsync().
- virtual std::optional<uint64_t> GetRegister(debug_ipc::RegisterID id);
+ // Request for synchronous register data. If the value is not synchronously
+ // availble, the *value will always be a nullopt.
+ //
+ // A return value of false means that the value is not known synchronously.
+ // In this case, GetRegisterAsync should be called to retrieve the value.
+ //
+ // In the synchronous case, we could have the value, but we could also know
+ // that the value is not known (e.g. when that register was not saved for the
+ // stack frame). The *value will reflect this when the return value is true.
+ virtual bool GetRegister(debug_ipc::RegisterID id,
+ std::optional<uint64_t>* value);
// Request for register data with an asynchronous callback. The callback will
// be issued when the register data is available.
@@ -94,3 +102,5 @@
};
} // namespace zxdb
+
+#endif // SRC_DEVELOPER_DEBUG_ZXDB_SYMBOLS_SYMBOL_DATA_PROVIDER_H_
