[debugger] Use async fingerprints in finish controller.
The FinishThreadController previously did its own asynchronous search
for stack frames using IP/SP. This is moved to the asynchronous
fingerprint getter on the Stack which removes complexity from the finish
thread controller.
There should be no user-visible change.
TEST=unit
Change-Id: Id11ca7d3d06179bbb3b4ff42e6dac14a646a9336
diff --git a/bin/zxdb/client/finish_thread_controller.cc b/bin/zxdb/client/finish_thread_controller.cc
index 223ecb7..12b5ba4 100644
--- a/bin/zxdb/client/finish_thread_controller.cc
+++ b/bin/zxdb/client/finish_thread_controller.cc
@@ -12,30 +12,36 @@
namespace zxdb {
-FinishThreadController::FinishThreadController(const Stack& stack,
- size_t frame_to_finish) {
+FinishThreadController::FinishThreadController(Stack& stack,
+ size_t frame_to_finish)
+ : weak_factory_(this) {
FXL_DCHECK(frame_to_finish < stack.size());
- // TODO(brettw) use new async frame fingerprint computation here.
- if (frame_to_finish == stack.size() - 1) {
- if (stack.has_all_frames()) {
- // Request to finish the oldest stack frame.
- to_address_ = 0;
- from_frame_fingerprint_ = *stack.GetFrameFingerprint(frame_to_finish);
- } else {
- // Need to sync frames so we can find the calling one. Save the IP and
- // SP which will be used to re-find the frame in question.
- // TODO(brettw) this won't handle inline frame selection properly.
- frame_ip_ = stack[frame_to_finish]->GetAddress();
- frame_ip_ = stack[frame_to_finish]->GetStackPointer();
- }
- } else {
+ if (auto found_fingerprint = stack.GetFrameFingerprint(frame_to_finish)) {
// Common case where the frame to finish has a previous frame and the
- // frame fingerprint and return address are known.
- FXL_DCHECK(stack.size() > frame_to_finish + 1);
+ // frame fingerprint and return address are known. If the frame's
+ // fingerprint can be computed, that means that the previous stack frame is
+ // available (or known not to exist).
// TODO(brettw) this won't handle inline frame selection properly.
- to_address_ = stack[frame_to_finish + 1]->GetAddress();
- from_frame_fingerprint_ = *stack.GetFrameFingerprint(frame_to_finish);
+ from_frame_fingerprint_ = *found_fingerprint;
+ if (frame_to_finish == stack.size() - 1)
+ to_address_ = 0; // Request to finish oldest stack frame.
+ else
+ to_address_ = stack[frame_to_finish + 1]->GetAddress();
+ } else {
+ // Fingerprint needs an asynchronous request.
+ stack.GetFrameFingerprint(
+ frame_to_finish,
+ [weak_this = weak_factory_.GetWeakPtr()](
+ const Err& err, size_t new_index, FrameFingerprint fingerprint) {
+ if (!weak_this)
+ return;
+
+ weak_this->from_frame_fingerprint_ = fingerprint;
+ if (weak_this->thread())
+ weak_this->InitWithFingerprintAndThread();
+ // Otherwise the callback will be issued when the thread is set.
+ });
}
}
@@ -57,18 +63,11 @@
Thread* thread, std::function<void(const Err&)> cb) {
set_thread(thread);
- if (from_frame_fingerprint_.is_valid()) {
- // The fingerprint was already computed in the constructor, can skip
- // directly to setting up the breakpoint.
- InitWithFingerprint(std::move(cb));
- } else {
- // Need to request stack frames to get the frame fingerpring and return
- // address (both of which require previous frames). We can capture |this|
- // here since the thread owns this thread controller.
- thread->GetStack().SyncFrames([ this, cb = std::move(cb) ](const Err& err) {
- InitWithStack(err, this->thread()->GetStack(), std::move(cb));
- });
- }
+ init_callback_ = std::move(cb);
+
+ if (from_frame_fingerprint_)
+ InitWithFingerprintAndThread();
+ // Otherwise the callback will be called when the fingerprint is know.
}
ThreadController::ContinueOp FinishThreadController::GetContinueOp() {
@@ -77,62 +76,19 @@
return ContinueOp::Continue();
}
-void FinishThreadController::InitWithStack(const Err& err, const Stack& stack,
- std::function<void(const Err&)> cb) {
- // Note if this was called asynchronously the thread could be resumed
- // and it could have no frames, or totally different ones.
- if (err.has_error()) {
- cb(err);
- return;
- }
+void FinishThreadController::InitWithFingerprintAndThread() {
+ FXL_DCHECK(init_callback_ && from_frame_fingerprint_);
- // Find the frame corresponding to the requested one.
- constexpr size_t kNotFound = std::numeric_limits<size_t>::max();
- size_t requested_index = kNotFound;
- for (size_t i = 0; i < stack.size(); i++) {
- if (stack[i]->GetAddress() == frame_ip_ &&
- stack[i]->GetStackPointer() == frame_sp_) {
- requested_index = i;
- break;
- }
- }
- if (requested_index == kNotFound) {
- cb(Err("The stack changed before \"finish\" could start."));
- return;
- }
-
- if (requested_index == stack.size() - 1) {
- // "Finish" from the bottom-most stack frame just continues the
- // program to completion.
- cb(Err());
- return;
- }
-
- // The stack frame to exit to is just the next one up.
- to_address_ = stack[requested_index + 1]->GetAddress();
- if (!to_address_) {
- // Often the bottom-most stack frame will have a 0 IP which obviously
- // we can't return to. Treat this the same as when returning from the
- // last frame and just continue.
- cb(Err());
- return;
- }
- from_frame_fingerprint_ =
- *thread()->GetStack().GetFrameFingerprint(requested_index);
- InitWithFingerprint(std::move(cb));
-}
-
-void FinishThreadController::InitWithFingerprint(
- std::function<void(const Err&)> cb) {
if (to_address_ == 0) {
// There is no return address. This will happen when trying to finish the
// oldest stack frame. Nothing to do.
- cb(Err());
+ init_callback_(Err());
+ init_callback_ = nullptr;
return;
}
until_controller_ = std::make_unique<UntilThreadController>(
- InputLocation(to_address_), from_frame_fingerprint_,
+ InputLocation(to_address_), *from_frame_fingerprint_,
UntilThreadController::kRunUntilOlderFrame);
// Give the "until" controller a dummy callback and execute the callback
@@ -145,7 +101,9 @@
// We can run faster without waiting for the round-trip, and the IPC will
// serialize so the breakpoint set happens before the thread resume.
until_controller_->InitWithThread(thread(), [](const Err&) {});
- cb(Err());
+
+ init_callback_(Err());
+ init_callback_ = nullptr;
}
} // namespace zxdb
diff --git a/bin/zxdb/client/finish_thread_controller.h b/bin/zxdb/client/finish_thread_controller.h
index 3cfbb9f..e702b12 100644
--- a/bin/zxdb/client/finish_thread_controller.h
+++ b/bin/zxdb/client/finish_thread_controller.h
@@ -4,10 +4,12 @@
#pragma once
+#include <optional>
#include <vector>
#include "garnet/bin/zxdb/client/frame_fingerprint.h"
#include "garnet/bin/zxdb/client/thread_controller.h"
+#include "lib/fxl/memory/weak_ptr.h"
namespace zxdb {
@@ -22,7 +24,7 @@
public:
// Finishes the given frame of the stack, leaving control at frame
// |frame_to_finish + 1] when the controller is complete.
- FinishThreadController(const Stack& stack, size_t frame_to_finish);
+ FinishThreadController(Stack& stack, size_t frame_to_finish);
~FinishThreadController() override;
@@ -36,37 +38,34 @@
const char* GetName() const override { return "Finish"; }
private:
- // Callback for when the thread has loaded its stack frames. This will
- // compute the to_frame_fingerprint_.
- void InitWithStack(const Err& err, const Stack& stack, std::function<void(const Err&)> cb);
-
- bool HaveAddressAndFingerprint() const;
-
- // Does final initialization given the to_frame_fingerprint_ is known or we
- // know we don't need it.
- void InitWithFingerprint(std::function<void(const Err&)> cb);
-
- // The instruction and stack pointer of the frame when the address and
- // fingerprint are not known. The SP allows disambiguation for two frames
- // at the same address. These will be nonzero only when we need to
- // asynchronously request data to compute the address or fingerprint.
- uint64_t frame_ip_ = 0;
- uint64_t frame_sp_ = 0;
+ // Called when both the frame fingerprint and the thread are known. Does
+ // final initialization.
+ void InitWithFingerprintAndThread();
// The from_frame_fingerprint_ will indicate the frame we're trying to
// finish. This being valid indicates that the stack has been fetched and
- // the frame in question located.
+ // the frame in question located (the operation may be async).
+ //
+ // This uses std::optional so the fingerprint can be tested to see if it's
+ // been set, even if the stack returns the null fingerprint (might happen
+ // for the bottom of the stack).
//
// The to_address_ will be the address we're returning to. This will be
// computed at the same time as from_frame_fingerprint_, but may be 0 if
// the user is trying to finish the oldest stack frame (there's no address
// to return to).
uint64_t to_address_ = 0;
- FrameFingerprint from_frame_fingerprint_;
+ std::optional<FrameFingerprint> from_frame_fingerprint_;
+
+ // The fingerprint can be computed asynchronously with initialization. This
+ // holds the InitWithThread callback for InitWithFingerprintAndThread().
+ std::function<void(const Err&)> init_callback_;
// Will be non-null when stepping out. During initialization or when stepping
// out of the earliest stack frame, this can be null.
std::unique_ptr<UntilThreadController> until_controller_;
+
+ fxl::WeakPtrFactory<FinishThreadController> weak_factory_;
};
} // namespace zxdb
diff --git a/bin/zxdb/client/step_over_thread_controller.cc b/bin/zxdb/client/step_over_thread_controller.cc
index 6eab82f..33ce364 100644
--- a/bin/zxdb/client/step_over_thread_controller.cc
+++ b/bin/zxdb/client/step_over_thread_controller.cc
@@ -92,7 +92,7 @@
return kStop;
}
- const auto& stack = thread()->GetStack();
+ Stack& stack = thread()->GetStack();
if (stack.size() < 2) {
Log("In a newer frame but there are not enough frames to step out.");
return kStop;
diff --git a/bin/zxdb/client/step_thread_controller.cc b/bin/zxdb/client/step_thread_controller.cc
index a068652..5d8a862 100644
--- a/bin/zxdb/client/step_thread_controller.cc
+++ b/bin/zxdb/client/step_thread_controller.cc
@@ -120,7 +120,7 @@
// function that we call from here.
FXL_DCHECK(!finish_unsymolized_function_);
- const auto& stack = thread()->GetStack();
+ Stack& stack = thread()->GetStack();
if (stack.empty())
return kStop; // Agent sent bad state, give up trying to step.
diff --git a/bin/zxdb/console/verbs_thread.cc b/bin/zxdb/console/verbs_thread.cc
index c6caea2..bfc24ac 100644
--- a/bin/zxdb/console/verbs_thread.cc
+++ b/bin/zxdb/console/verbs_thread.cc
@@ -277,7 +277,7 @@
if (err.has_error())
return err;
- const Stack& stack = cmd.thread()->GetStack();
+ Stack& stack = cmd.thread()->GetStack();
size_t frame_index;
if (auto found_frame_index = stack.IndexForFrame(cmd.frame()))
frame_index = *found_frame_index;
