[debugger] Hook up filter noun to debug agent
Change-Id: Icb5a369e94f71c74c965cabe49c57b1971a406f6
diff --git a/src/developer/debug/zxdb/client/BUILD.gn b/src/developer/debug/zxdb/client/BUILD.gn
index bad78d7..3cb45f7 100644
--- a/src/developer/debug/zxdb/client/BUILD.gn
+++ b/src/developer/debug/zxdb/client/BUILD.gn
@@ -65,6 +65,7 @@
"cloud_storage_symbol_server.cc",
"curl.cc",
"disassembler.cc",
+ "filter.cc",
"finish_physical_frame_thread_controller.cc",
"finish_thread_controller.cc",
"frame.cc",
@@ -207,6 +208,7 @@
"backtrace_cache_unittest.cc",
"breakpoint_impl_unittest.cc",
"disassembler_unittest.cc",
+ "filter_unittest.cc",
"finish_physical_frame_thread_controller_unittest.cc",
"finish_thread_controller_unittest.cc",
"frame_fingerprint_unittest.cc",
diff --git a/src/developer/debug/zxdb/client/filter.cc b/src/developer/debug/zxdb/client/filter.cc
new file mode 100644
index 0000000..45c0af7
--- /dev/null
+++ b/src/developer/debug/zxdb/client/filter.cc
@@ -0,0 +1,25 @@
+// Copyright 2019 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/developer/debug/zxdb/client/filter.h"
+
+#include "src/developer/debug/zxdb/client/session.h"
+
+namespace zxdb {
+
+Filter::Filter(Session* session) : ClientObject(session) {}
+
+void Filter::SetPattern(const std::string& pattern) {
+ pattern_ = pattern;
+
+ session()->system_impl().MarkFiltersDirty();
+}
+
+void Filter::SetJob(JobContext* job) {
+ job_ = job ? std::optional(job->GetWeakPtr()) : std::nullopt;
+
+ session()->system_impl().MarkFiltersDirty();
+}
+
+} // namespace zxdb
diff --git a/src/developer/debug/zxdb/client/filter.h b/src/developer/debug/zxdb/client/filter.h
index 0214ca3..5f3248c9 100644
--- a/src/developer/debug/zxdb/client/filter.h
+++ b/src/developer/debug/zxdb/client/filter.h
@@ -7,22 +7,32 @@
#include <optional>
+#include "src/developer/debug/zxdb/client/client_object.h"
#include "src/developer/debug/zxdb/client/job_context.h"
#include "src/lib/fxl/memory/weak_ptr.h"
namespace zxdb {
-class Filter {
+class Filter : public ClientObject {
public:
- void set_pattern(const std::string& pattern) { pattern_ = pattern; }
- void set_job(JobContext* job) {
- job_ = job ? std::optional(job->GetWeakPtr()) : std::nullopt;
- }
+ explicit Filter(Session* session);
+
+ void SetPattern(const std::string& pattern);
+ void SetJob(JobContext* job);
+
const std::string& pattern() { return pattern_; }
JobContext* job() { return job_ ? job_->get() : nullptr; }
+ bool valid() { return !pattern_.empty() && (!job_ || job_->get()); }
private:
std::string pattern_;
+
+ // This exists in one of 3 states:
+ // 1) Optional contains non-null pointer. That points to the job this
+ // applies to.
+ // 2) Optional is a nullopt. This filter applies to all jobs.
+ // 3) Optional contains a null pointer. This filter was meant to apply to a
+ // job that disappeared.
std::optional<fxl::WeakPtr<JobContext>> job_;
};
diff --git a/src/developer/debug/zxdb/client/filter_unittest.cc b/src/developer/debug/zxdb/client/filter_unittest.cc
new file mode 100644
index 0000000..068d395
--- /dev/null
+++ b/src/developer/debug/zxdb/client/filter_unittest.cc
@@ -0,0 +1,199 @@
+// Copyright 2019 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/developer/debug/zxdb/client/filter.h"
+
+#include "gtest/gtest.h"
+#include "src/developer/debug/shared/platform_message_loop.h"
+#include "src/developer/debug/zxdb/client/remote_api_test.h"
+#include "src/developer/debug/zxdb/client/session.h"
+
+namespace zxdb {
+
+namespace {
+
+using debug_ipc::MessageLoop;
+
+class FilterSink : public RemoteAPI {
+ public:
+ void JobFilter(
+ const debug_ipc::JobFilterRequest& request,
+ std::function<void(const Err&, debug_ipc::JobFilterReply)> cb) override {
+ filters[request.job_koid] = request.filters;
+
+ MessageLoop::Current()->PostTask(FROM_HERE, [cb]() {
+ cb(Err(), debug_ipc::JobFilterReply());
+ MessageLoop::Current()->QuitNow();
+ });
+ }
+
+ void Attach(
+ const debug_ipc::AttachRequest& request,
+ std::function<void(const Err&, debug_ipc::AttachReply)> cb) override {
+ debug_ipc::AttachReply reply;
+ reply.koid = request.koid;
+ reply.name = "test";
+ MessageLoop::Current()->PostTask(FROM_HERE,
+ [cb, reply]() { cb(Err(), reply); });
+ }
+
+ std::map<uint64_t, std::vector<std::string>> filters;
+};
+
+class FilterTest : public RemoteAPITest {
+ public:
+ FilterTest() = default;
+ ~FilterTest() override = default;
+
+ FilterSink& sink() { return *sink_; }
+
+ protected:
+ std::unique_ptr<RemoteAPI> GetRemoteAPIImpl() override {
+ auto sink = std::make_unique<FilterSink>();
+ sink_ = sink.get();
+ return std::move(sink);
+ }
+
+ private:
+ FilterSink* sink_; // Owned by the session.
+};
+
+} // namespace
+
+TEST_F(FilterTest, SetFilters) {
+ Filter* filter = session().system().CreateNewFilter();
+
+ auto contexts = session().system().GetJobContexts();
+ ASSERT_EQ(1u, contexts.size());
+ auto context = contexts[0];
+ bool job_context_alive;
+ Err ctx_err;
+
+ context->Attach(1234, [&job_context_alive, &ctx_err](
+ fxl::WeakPtr<JobContext> ctx, const Err& err) {
+ ctx_err = err;
+ job_context_alive = !!ctx;
+ MessageLoop::Current()->QuitNow();
+ });
+ MessageLoop::Current()->Run();
+
+ EXPECT_FALSE(ctx_err.has_error());
+ EXPECT_TRUE(job_context_alive);
+
+ filter->SetPattern("foo");
+ MessageLoop::Current()->Run();
+
+ ASSERT_EQ(1u, sink().filters[1234u].size());
+ EXPECT_EQ("foo", sink().filters[1234u][0]);
+}
+
+TEST_F(FilterTest, SetSpecificFilters) {
+ Filter* filter = session().system().CreateNewFilter();
+
+ auto contexts = session().system().GetJobContexts();
+ ASSERT_EQ(1u, contexts.size());
+ auto context_a = contexts[0];
+ auto context_b = session().system().CreateNewJobContext(context_a);
+ bool job_context_alive;
+ Err ctx_err;
+
+ context_a->Attach(1234, [&job_context_alive, &ctx_err](
+ fxl::WeakPtr<JobContext> ctx, const Err& err) {
+ ctx_err = err;
+ job_context_alive = !!ctx;
+ MessageLoop::Current()->QuitNow();
+ });
+ MessageLoop::Current()->Run();
+
+ EXPECT_FALSE(ctx_err.has_error());
+ EXPECT_TRUE(job_context_alive);
+
+ context_b->Attach(5678, [&job_context_alive, &ctx_err](
+ fxl::WeakPtr<JobContext> ctx, const Err& err) {
+ ctx_err = err;
+ job_context_alive = !!ctx;
+ MessageLoop::Current()->QuitNow();
+ });
+ MessageLoop::Current()->Run();
+
+ EXPECT_FALSE(ctx_err.has_error());
+ EXPECT_TRUE(job_context_alive);
+
+ filter->SetPattern("foo");
+ filter->SetJob(context_a);
+ MessageLoop::Current()->Run();
+
+ ASSERT_EQ(1u, sink().filters[1234u].size());
+ EXPECT_EQ("foo", sink().filters[1234u][0]);
+ ASSERT_EQ(0u, sink().filters[5678u].size());
+}
+
+TEST_F(FilterTest, SetBroadFilters) {
+ Filter* filter = session().system().CreateNewFilter();
+
+ auto contexts = session().system().GetJobContexts();
+ ASSERT_EQ(1u, contexts.size());
+ auto context_a = contexts[0];
+ auto context_b = session().system().CreateNewJobContext(context_a);
+ bool job_context_alive;
+ Err ctx_err;
+
+ context_a->Attach(1234, [&job_context_alive, &ctx_err](
+ fxl::WeakPtr<JobContext> ctx, const Err& err) {
+ ctx_err = err;
+ job_context_alive = !!ctx;
+ MessageLoop::Current()->QuitNow();
+ });
+ MessageLoop::Current()->Run();
+
+ EXPECT_FALSE(ctx_err.has_error());
+ EXPECT_TRUE(job_context_alive);
+
+ context_b->Attach(5678, [&job_context_alive, &ctx_err](
+ fxl::WeakPtr<JobContext> ctx, const Err& err) {
+ ctx_err = err;
+ job_context_alive = !!ctx;
+ MessageLoop::Current()->QuitNow();
+ });
+ MessageLoop::Current()->Run();
+
+ EXPECT_FALSE(ctx_err.has_error());
+ EXPECT_TRUE(job_context_alive);
+
+ filter->SetPattern("foo");
+ MessageLoop::Current()->Run();
+
+ ASSERT_EQ(1u, sink().filters[1234u].size());
+ EXPECT_EQ("foo", sink().filters[1234u][0]);
+ ASSERT_EQ(1u, sink().filters[5678u].size());
+ EXPECT_EQ("foo", sink().filters[5678u][0]);
+}
+
+TEST_F(FilterTest, SetExAnteFilters) {
+ Filter* filter = session().system().CreateNewFilter();
+
+ filter->SetPattern("foo");
+
+ auto contexts = session().system().GetJobContexts();
+ ASSERT_EQ(1u, contexts.size());
+ auto context = contexts[0];
+ bool job_context_alive;
+ Err ctx_err;
+
+ context->Attach(1234, [&job_context_alive, &ctx_err](
+ fxl::WeakPtr<JobContext> ctx, const Err& err) {
+ ctx_err = err;
+ job_context_alive = !!ctx;
+ MessageLoop::Current()->QuitNow();
+ });
+ MessageLoop::Current()->Run();
+
+ EXPECT_FALSE(ctx_err.has_error());
+ EXPECT_TRUE(job_context_alive);
+
+ ASSERT_EQ(1u, sink().filters[1234u].size());
+ EXPECT_EQ("foo", sink().filters[1234u][0]);
+}
+
+} // namespace zxdb
diff --git a/src/developer/debug/zxdb/client/job_context.cc b/src/developer/debug/zxdb/client/job_context.cc
index ae2456d..5c2361f 100644
--- a/src/developer/debug/zxdb/client/job_context.cc
+++ b/src/developer/debug/zxdb/client/job_context.cc
@@ -10,17 +10,10 @@
// Schema Definition -----------------------------------------------------------
-const char* ClientSettings::Job::kFilters = "filters";
-static const char* kFiltersDescription =
- R"( List of filters to be applied to a job. Filters are used against each new
- process being spawned under the attached job. If there is match, zxdb will
- automatically attach to the process.)";
-
namespace {
fxl::RefPtr<SettingSchema> CreateSchema() {
auto schema = fxl::MakeRefCounted<SettingSchema>();
- schema->AddList(ClientSettings::Job::kFilters, kFiltersDescription, {});
return schema;
}
diff --git a/src/developer/debug/zxdb/client/job_context.h b/src/developer/debug/zxdb/client/job_context.h
index 44764ba..e62d749 100644
--- a/src/developer/debug/zxdb/client/job_context.h
+++ b/src/developer/debug/zxdb/client/job_context.h
@@ -72,6 +72,10 @@
// executed when the detach is complete (or fails).
virtual void Detach(Callback callback) = 0;
+ // Set filters for this job. TODO: Remove this as soon as fidlcat can use the
+ // object model.
+ virtual void SendAndUpdateFilters(std::vector<std::string> filters) = 0;
+
// Provides the setting schema for this object.
static fxl::RefPtr<SettingSchema> GetSchema();
diff --git a/src/developer/debug/zxdb/client/job_context_impl.cc b/src/developer/debug/zxdb/client/job_context_impl.cc
index 4f1a49e..66083f1 100644
--- a/src/developer/debug/zxdb/client/job_context_impl.cc
+++ b/src/developer/debug/zxdb/client/job_context_impl.cc
@@ -24,7 +24,6 @@
system_(system),
is_implicit_root_(is_implicit_root),
impl_weak_factory_(this) {
- settings_.AddObserver(ClientSettings::Job::kFilters, this);
settings_.set_name("job");
}
@@ -161,8 +160,14 @@
});
}
+void JobContextImpl::SendAndUpdateFilters(std::vector<std::string> filters) {
+ SendAndUpdateFilters(filters, last_filter_set_failed_);
+}
+
void JobContextImpl::SendAndUpdateFilters(std::vector<std::string> filters,
bool force_send) {
+ last_filter_set_failed_ = false;
+
if (!job_.get()) {
filters_ = std::move(filters);
return;
@@ -183,11 +188,9 @@
FXL_LOG(ERROR) << "Error adding filter: "
<< debug_ipc::ZxStatusToString(reply.status);
if (weak_job_context) {
- // Agent failed, reset filters in settings.
- // This will also trigger another callback but would be a no-op
- // because |force_send| would be false.
- weak_job_context->settings_.SetList(ClientSettings::Job::kFilters,
- weak_job_context->filters_);
+ // Agent failed, mark that we had trouble setting filters and
+ // return.
+ weak_job_context->last_filter_set_failed_ = true;
}
return;
}
@@ -199,8 +202,7 @@
void JobContextImpl::OnSettingChanged(const SettingStore&,
const std::string& setting_name) {
- FXL_CHECK(setting_name == ClientSettings::Job::kFilters);
- SendAndUpdateFilters(settings_.GetList(setting_name));
+ FXL_NOTREACHED() << "No settings supported for jobs.";
}
void JobContextImpl::OnDetachReply(const Err& err, uint32_t status,
diff --git a/src/developer/debug/zxdb/client/job_context_impl.h b/src/developer/debug/zxdb/client/job_context_impl.h
index 09c3018..9a6e19b2 100644
--- a/src/developer/debug/zxdb/client/job_context_impl.h
+++ b/src/developer/debug/zxdb/client/job_context_impl.h
@@ -52,6 +52,7 @@
void AttachToSystemRoot(Callback callback) override;
void AttachToComponentRoot(Callback callback) override;
void Detach(Callback callback) override;
+ void SendAndUpdateFilters(std::vector<std::string> filters) override;
// SettingStoreObserver implementation
void OnSettingChanged(const SettingStore&,
@@ -66,6 +67,7 @@
std::unique_ptr<JobImpl> job_;
std::vector<std::string> filters_;
bool is_implicit_root_;
+ bool last_filter_set_failed_ = false;
fxl::WeakPtrFactory<JobContextImpl> impl_weak_factory_;
@@ -82,8 +84,7 @@
// If job is running this will update |filters_| only after getting OK from
// agent else it will set |filters_| and return.
- void SendAndUpdateFilters(std::vector<std::string> filters,
- bool force_send = false);
+ void SendAndUpdateFilters(std::vector<std::string> filters, bool force_send);
FXL_DISALLOW_COPY_AND_ASSIGN(JobContextImpl);
};
diff --git a/src/developer/debug/zxdb/client/setting_schema_definition.h b/src/developer/debug/zxdb/client/setting_schema_definition.h
index 1adbd73..af594b5 100644
--- a/src/developer/debug/zxdb/client/setting_schema_definition.h
+++ b/src/developer/debug/zxdb/client/setting_schema_definition.h
@@ -30,9 +30,7 @@
static const char* kSymbolCache;
};
- struct Job {
- static const char* kFilters;
- };
+ struct Job {};
struct Target {
static const char* kStoreBacktraces;
diff --git a/src/developer/debug/zxdb/client/system_impl.cc b/src/developer/debug/zxdb/client/system_impl.cc
index 9fd0866..7bfa7e5 100644
--- a/src/developer/debug/zxdb/client/system_impl.cc
+++ b/src/developer/debug/zxdb/client/system_impl.cc
@@ -213,6 +213,50 @@
return nullptr;
}
+void SystemImpl::MarkFiltersDirty() {
+ if (!debug_ipc::MessageLoop::Current()) {
+ filters_dirty_ = false;
+ return;
+ }
+
+ if (filters_dirty_) {
+ return;
+ }
+
+ filters_dirty_ = true;
+
+ debug_ipc::MessageLoop::Current()->PostTask(
+ FROM_HERE, [weak_this = weak_factory_.GetWeakPtr()]() {
+ if (!weak_this) {
+ return;
+ }
+
+ std::map<JobContext*, std::vector<std::string>> filters_by_job;
+ std::vector<std::string> filters;
+
+ for (const auto& filter : weak_this->filters_) {
+ if (!filter->valid()) {
+ continue;
+ }
+
+ if (!filter->job()) {
+ filters.push_back(filter->pattern());
+ } else {
+ filters_by_job[filter->job()].push_back(filter->pattern());
+ }
+ }
+
+ for (auto& ctx : weak_this->job_contexts_) {
+ auto& items = filters_by_job[ctx.get()];
+ std::copy(filters.begin(), filters.end(), std::back_inserter(items));
+
+ ctx->SendAndUpdateFilters(items);
+ }
+
+ weak_this->filters_dirty_ = false;
+ });
+}
+
void SystemImpl::NotifyDidCreateProcess(Process* process) {
for (auto& observer : observers())
observer.GlobalDidCreateProcess(process);
@@ -434,7 +478,8 @@
}
Filter* SystemImpl::CreateNewFilter() {
- Filter* to_return = filters_.emplace_back(std::make_unique<Filter>()).get();
+ Filter* to_return =
+ filters_.emplace_back(std::make_unique<Filter>(session())).get();
// Notify observers (may mutate filter list).
for (auto& observer : observers())
@@ -566,6 +611,8 @@
job_contexts_.push_back(std::move(job_context));
for (auto& observer : observers())
observer.DidCreateJobContext(for_observers);
+
+ MarkFiltersDirty();
}
void SystemImpl::OnSettingChanged(const SettingStore& store,
diff --git a/src/developer/debug/zxdb/client/system_impl.h b/src/developer/debug/zxdb/client/system_impl.h
index ca9663f..325e7d0 100644
--- a/src/developer/debug/zxdb/client/system_impl.h
+++ b/src/developer/debug/zxdb/client/system_impl.h
@@ -32,6 +32,9 @@
ProcessImpl* ProcessImplFromKoid(uint64_t koid) const;
+ // Lets us know that we should sync the filters with the debug agent.
+ void MarkFiltersDirty();
+
// Broadcasts the global process notifications.
void NotifyDidCreateProcess(Process* process);
void NotifyWillDestroyProcess(Process* process);
@@ -124,6 +127,7 @@
std::map<uint32_t, std::unique_ptr<BreakpointImpl>> breakpoints_;
std::vector<std::unique_ptr<Filter>> filters_;
+ bool filters_dirty_ = false;
SystemSymbols symbols_;
diff --git a/src/developer/debug/zxdb/console/console_main.cc b/src/developer/debug/zxdb/console/console_main.cc
index 21554d3..e153ece 100644
--- a/src/developer/debug/zxdb/console/console_main.cc
+++ b/src/developer/debug/zxdb/console/console_main.cc
@@ -65,6 +65,14 @@
return err;
}
+ for (const auto& filter : options.filter) {
+ std::string cmd = "filter attach " + filter;
+ actions->push_back(Action("Filter", [cmd](const Action&, const Session&,
+ Console* console) {
+ console->ProcessInputLine(cmd.c_str(), ActionFlow::PostActionCallback);
+ }));
+ }
+
return Err();
}
@@ -130,17 +138,6 @@
// Redundant adds are ignored.
session->system().settings().SetList(ClientSettings::System::kSymbolPaths,
std::move(paths));
-
- // Filters, there should already be a default job context.
- if (!options.filter.empty()) {
- JobContext* default_job = session->system().GetJobContexts()[0];
- Err err = default_job->settings().SetList(ClientSettings::Job::kFilters,
- options.filter);
-
- // The filters aren't currently validated when setting the list so an error
- // here means we used the API wrong.
- FXL_DCHECK(!err.has_error()) << err.msg();
- }
}
} // namespace
diff --git a/src/developer/debug/zxdb/console/verbs_process.cc b/src/developer/debug/zxdb/console/verbs_process.cc
index 9d916d3..cadf5153 100644
--- a/src/developer/debug/zxdb/console/verbs_process.cc
+++ b/src/developer/debug/zxdb/console/verbs_process.cc
@@ -416,10 +416,10 @@
} else {
JobContext* job = cmd.HasNoun(Noun::kJob) ? cmd.job_context() : nullptr;
filter = context->session()->system().CreateNewFilter();
- filter->set_job(job);
+ filter->SetJob(job);
}
- filter->set_pattern(cmd.args()[0]);
+ filter->SetPattern(cmd.args()[0]);
Console::get()->Output("Waiting for process matching /" + cmd.args()[0] +
"/");
diff --git a/tools/fidlcat/lib/interception_workflow.cc b/tools/fidlcat/lib/interception_workflow.cc
index d83c2ab..eb8cafb 100644
--- a/tools/fidlcat/lib/interception_workflow.cc
+++ b/tools/fidlcat/lib/interception_workflow.cc
@@ -10,6 +10,7 @@
#include "src/developer/debug/shared/platform_message_loop.h"
#include "src/developer/debug/shared/zx_status.h"
#include "src/developer/debug/zxdb/client/breakpoint.h"
+#include "src/developer/debug/zxdb/client/filter.h"
#include "src/developer/debug/zxdb/client/remote_api.h"
#include "src/developer/debug/zxdb/client/setting_schema_definition.h"
#include "src/developer/debug/zxdb/client/thread.h"
@@ -258,12 +259,7 @@
void InterceptionWorkflow::Filter(const std::vector<std::string>& filter,
SimpleErrorFunction and_then) {
zxdb::JobContext* default_job = session_->system().GetJobContexts()[0];
- zxdb::Err err = default_job->settings().SetList(
- zxdb::ClientSettings::Job::kFilters, filter);
-
- // The filters aren't currently validated when setting the list so an error
- // here means we used the API wrong.
- FXL_DCHECK(err.ok()) << err.msg();
+ default_job->SendAndUpdateFilters(filter);
GetTarget()->AddObserver(new SetupTargetObserver(std::move(and_then)));
AddObserver(GetTarget());
}
