[lldb] Add accelerator plugin connection support - #201449
Conversation
9f8baac to
73137f7
Compare
| /// Name of the platform to select for the accelerator target. | ||
| std::optional<std::string> platform_name; |
There was a problem hiding this comment.
We aren't using this yet, so remove it.
| GDBServer &gdb_server, MainLoop &main_loop) | ||
| : LLDBServerAcceleratorPlugin(gdb_server, main_loop) {} | ||
| : LLDBServerAcceleratorPlugin(gdb_server, main_loop) { | ||
| // Stand up an in-process GDB server backed by a fake accelerator process so |
There was a problem hiding this comment.
What does "Stand up an in-process GDB server" mean?
| // dedicated, uniquely named function. When it is hit the plugin asks the client | ||
| // to create a second target and connect to the mock accelerator GDB server. | ||
| // Only this program defines it, so only this test triggers a connection. | ||
| void mock_gpu_accelerator_connect(void) {} |
There was a problem hiding this comment.
add a void mock_gpu_accelerator_initalize(void) {} to make sure it hits the initialize first.
| // Also set a breakpoint on the dedicated connection hook. This only resolves | ||
| // (and fires) in programs that define "mock_gpu_accelerator_connect"; when it | ||
| // is hit the plugin asks the client to create a second target and connect to | ||
| // the mock accelerator GDB server. | ||
| AcceleratorBreakpointInfo connect_bp; | ||
| connect_bp.identifier = kBreakpointIDConnect; | ||
| connect_bp.by_name = | ||
| AcceleratorBreakpointByName{std::nullopt, "mock_gpu_accelerator_connect"}; | ||
| actions.breakpoints.push_back(std::move(connect_bp)); | ||
|
|
There was a problem hiding this comment.
This should be moved the the breakpoint hit response for kBreakpointIDInitialize so that it only happens after the initialize function bp is hit. This makes this scenario more realistic to what real GPUs will do.
c71dc32 to
cc2b157
Compare
|
@llvm/pr-subscribers-lldb Author: satyanarayana reddy janga (satyajanga) ChangesSummary This builds on the accelerator plugin protocol (#201489) by letting a plugin ask the client to create and connect a second What this addsProtocol — a new Client — when an Mock server (for testing) — to exercise this end to end, the mock ScopeIntentionally minimal: the goal is to create and show the second (accelerator) TestingDemo of how this looks with this PR. (explicitly asking the native process to stop at every breakpoint for testability) Packet log, Patch is 49.17 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/201449.diff 18 Files Affected:
diff --git a/lldb/docs/resources/lldbgdbremote.md b/lldb/docs/resources/lldbgdbremote.md
index ef42ebd6ae41c..7dbf5dbd39311 100644
--- a/lldb/docs/resources/lldbgdbremote.md
+++ b/lldb/docs/resources/lldbgdbremote.md
@@ -2774,9 +2774,20 @@ packet when one is hit. Each breakpoint object has the following fields:
Exactly one of `by_name` or `by_address` must be provided for each
breakpoint.
+An `accelerator_action` may also include a `connect_info` object asking the
+client to create a new target and connect to a separate GDB server that
+serves the accelerator's state (for example a GPU debug stub). It has the
+following fields:
+
+| Key | Type | Description |
+|-----------------|--------|-------------|
+| `connect_url` | string | Connection URL to connect to, as used by `process connect <url>`. |
+| `exe_path` | string | Optional path to the executable to use when creating the accelerator target. If omitted, an empty target is created. |
+| `triple` | string | Optional target triple to use as the architecture for the accelerator target. |
+| `synchronous` | bool | If true, the client waits for the accelerator process to finish initializing before continuing. |
+
In future patches, each `accelerator_action` will include additional fields
-such as connection info for secondary debug sessions and synchronization
-options.
+such as synchronization options for the accelerator process.
**Priority To Implement:** Required for hardware accelerator debugging
support. Not needed for non-hardware-accelerator debugging.
diff --git a/lldb/include/lldb/Utility/AcceleratorGDBRemotePackets.h b/lldb/include/lldb/Utility/AcceleratorGDBRemotePackets.h
index 9ba36fc540a52..601bacef1ee12 100644
--- a/lldb/include/lldb/Utility/AcceleratorGDBRemotePackets.h
+++ b/lldb/include/lldb/Utility/AcceleratorGDBRemotePackets.h
@@ -85,6 +85,28 @@ bool fromJSON(const llvm::json::Value &value,
AcceleratorBreakpointHitArgs &data, llvm::json::Path path);
llvm::json::Value toJSON(const AcceleratorBreakpointHitArgs &data);
+/// Information the client needs to create a reverse connection to an
+/// accelerator GDB server (e.g. a separate process serving the accelerator's
+/// register and memory state). When an AcceleratorActions carries this, the
+/// client creates a new target and connects to \a connect_url.
+struct AcceleratorConnectionInfo {
+ /// Connection URL the client should connect to (as in "process connect
+ /// <url>"). Required; the remaining fields are optional.
+ std::string connect_url;
+ /// Path to the executable to use when creating the accelerator target. If
+ /// not set, an empty target is created.
+ std::optional<std::string> exe_path;
+ /// Target triple to use as the architecture for the accelerator target.
+ std::optional<std::string> triple;
+ /// If true, the client waits for the accelerator process to finish
+ /// initializing before continuing.
+ bool synchronous = false;
+};
+
+bool fromJSON(const llvm::json::Value &value, AcceleratorConnectionInfo &data,
+ llvm::json::Path path);
+llvm::json::Value toJSON(const AcceleratorConnectionInfo &data);
+
/// Actions to be performed in the native process on behalf of an accelerator
/// plugin. AcceleratorActions are returned in the following contexts:
///
@@ -114,6 +136,9 @@ struct AcceleratorActions {
int64_t identifier = 0;
/// New breakpoints to set. Nothing to set if this is empty.
std::vector<AcceleratorBreakpointInfo> breakpoints;
+ /// If set, the client should create a new target and connect to the
+ /// accelerator GDB server described here.
+ std::optional<AcceleratorConnectionInfo> connect_info;
};
bool fromJSON(const llvm::json::Value &value, AcceleratorActions &data,
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 16284f0052f5e..698892cfc4e19 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4197,6 +4197,61 @@ ProcessGDBRemote::HandleAcceleratorActions(const AcceleratorActions &actions) {
return error;
}
+ if (actions.connect_info) {
+ if (llvm::Error error = HandleAcceleratorConnection(actions))
+ return error;
+ }
+
+ return llvm::Error::success();
+}
+
+llvm::Error ProcessGDBRemote::HandleAcceleratorConnection(
+ const AcceleratorActions &actions) {
+ const AcceleratorConnectionInfo &connect_info = *actions.connect_info;
+ Debugger &debugger = GetTarget().GetDebugger();
+
+ // Create a new (empty) target for the accelerator and connect to the GDB
+ // server the plugin is serving.
+ llvm::StringRef exe_path =
+ connect_info.exe_path ? *connect_info.exe_path : llvm::StringRef();
+ llvm::StringRef triple =
+ connect_info.triple ? *connect_info.triple : llvm::StringRef();
+ TargetSP accelerator_target_sp;
+ Status error = debugger.GetTargetList().CreateTarget(
+ debugger, exe_path, triple, eLoadDependentsNo,
+ /*platform_options=*/nullptr, accelerator_target_sp);
+ if (error.Fail())
+ return error.takeError();
+ if (!accelerator_target_sp)
+ return llvm::createStringError("failed to create accelerator target");
+
+ PlatformSP platform_sp = accelerator_target_sp->GetPlatform();
+ if (!platform_sp)
+ return llvm::createStringError(
+ "no platform for the accelerator target connection");
+
+ ProcessSP process_sp =
+ connect_info.synchronous
+ ? platform_sp->ConnectProcessSynchronous(
+ connect_info.connect_url, GetPluginNameStatic(), debugger,
+ *debugger.GetAsyncOutputStream(), accelerator_target_sp.get(),
+ error)
+ : platform_sp->ConnectProcess(connect_info.connect_url,
+ GetPluginNameStatic(), debugger,
+ accelerator_target_sp.get(), error);
+ if (error.Fail())
+ return error.takeError();
+ if (!process_sp)
+ return llvm::createStringError("failed to connect to the accelerator");
+
+ accelerator_target_sp->SetTargetSessionName(actions.session_name);
+
+ // Broadcast the target creation event so DAP can create a child session
+ auto event_sp = std::make_shared<Event>(
+ Target::eBroadcastBitNewTargetCreated,
+ new Target::TargetEventData(GetTarget().shared_from_this(),
+ accelerator_target_sp));
+ GetTarget().BroadcastEvent(event_sp);
return llvm::Error::success();
}
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
index 0a3386082c388..525a45f7cce21 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -488,6 +488,10 @@ class ProcessGDBRemote : public Process,
/// breakpoints are still set.
llvm::Error HandleAcceleratorBreakpoints(const AcceleratorActions &actions);
+ /// Create a new target for an accelerator and connect it to the GDB server
+ /// described by the action's connection info.
+ llvm::Error HandleAcceleratorConnection(const AcceleratorActions &actions);
+
/// Breakpoint callback invoked when an accelerator-plugin-requested
/// breakpoint is hit. Resolves any requested symbol values, notifies the
/// plugin via the "jAcceleratorPluginBreakpointHit" packet, and handles the
diff --git a/lldb/source/Utility/AcceleratorGDBRemotePackets.cpp b/lldb/source/Utility/AcceleratorGDBRemotePackets.cpp
index 34067b1fa64c7..509d622ad2129 100644
--- a/lldb/source/Utility/AcceleratorGDBRemotePackets.cpp
+++ b/lldb/source/Utility/AcceleratorGDBRemotePackets.cpp
@@ -86,21 +86,42 @@ AcceleratorBreakpointHitArgs::GetSymbolValue(StringRef symbol_name) const {
return std::nullopt;
}
+bool fromJSON(const Value &value, AcceleratorConnectionInfo &data, Path path) {
+ ObjectMapper o(value, path);
+ return o && o.map("connect_url", data.connect_url) &&
+ o.mapOptional("exe_path", data.exe_path) &&
+ o.mapOptional("triple", data.triple) &&
+ o.map("synchronous", data.synchronous);
+}
+
+json::Value toJSON(const AcceleratorConnectionInfo &data) {
+ return Object{
+ {"connect_url", data.connect_url},
+ {"exe_path", data.exe_path},
+ {"triple", data.triple},
+ {"synchronous", data.synchronous},
+ };
+}
+
bool fromJSON(const Value &value, AcceleratorActions &data, Path path) {
ObjectMapper o(value, path);
return o && o.map("plugin_name", data.plugin_name) &&
o.map("session_name", data.session_name) &&
o.map("identifier", data.identifier) &&
- o.map("breakpoints", data.breakpoints);
+ o.map("breakpoints", data.breakpoints) &&
+ o.mapOptional("connect_info", data.connect_info);
}
json::Value toJSON(const AcceleratorActions &data) {
- return Object{
+ Object obj{
{"plugin_name", data.plugin_name},
{"session_name", data.session_name},
{"identifier", data.identifier},
{"breakpoints", data.breakpoints},
};
+ if (data.connect_info)
+ obj["connect_info"] = *data.connect_info;
+ return obj;
}
bool fromJSON(const Value &value, AcceleratorBreakpointHitResponse &data,
diff --git a/lldb/test/API/accelerator/mock/TestMockAcceleratorBreakpoints.py b/lldb/test/API/accelerator/mock/TestMockAcceleratorActions.py
similarity index 56%
rename from lldb/test/API/accelerator/mock/TestMockAcceleratorBreakpoints.py
rename to lldb/test/API/accelerator/mock/TestMockAcceleratorActions.py
index 885e73e3cec15..10dabd2e243a4 100644
--- a/lldb/test/API/accelerator/mock/TestMockAcceleratorBreakpoints.py
+++ b/lldb/test/API/accelerator/mock/TestMockAcceleratorActions.py
@@ -1,11 +1,15 @@
"""
-End-to-end test for accelerator plugin breakpoints.
+End-to-end test for accelerator plugin actions (breakpoints and connections).
Launches a real process against an lldb-server that has the mock accelerator
plugin enabled and verifies that the breakpoints requested by the plugin are
set in the native process, hit, and that hitting one breakpoint can request
further breakpoints. This exercises all three breakpoint types: by name, by
name scoped to a shared library, and by address.
+
+It also verifies that hitting the plugin's connection-trigger breakpoint causes
+the client to create a second (accelerator) target and reverse-connect it to
+the mock accelerator GDB server.
"""
import lldb
@@ -22,7 +26,7 @@ def uint64_to_int64(value):
return value
-class MockAcceleratorBreakpointsTestCase(TestBase):
+class MockAcceleratorActionsTestCase(TestBase):
NO_DEBUG_INFO_TESTCASE = True
def setUp(self):
@@ -60,8 +64,9 @@ def check_accelerator_breakpoint_stop(self, process, function_name, hit_count=No
@skipIfRemote
@add_test_categories(["llgs"])
- def test_accelerator_breakpoints(self):
- """The mock accelerator plugin drives breakpoints in the inferior."""
+ def test_accelerator_actions(self):
+ """The mock accelerator plugin drives breakpoints in the inferior and,
+ once initialized, a connection that creates a second target."""
self.build()
exe = self.getBuildArtifact("a.out")
target = self.dbg.CreateTarget(exe)
@@ -83,11 +88,59 @@ def test_accelerator_breakpoints(self):
self.assertEqual(target.GetNumBreakpoints(), 0)
# Hitting the mock_gpu_accelerator_initialize breakpoint caused the
- # plugin to request two more breakpoints: one by address (on
- # "mock_gpu_accelerator_compute", from the symbol value delivered with
- # the hit) and one by name scoped to the "a.out" shared library (on
- # "mock_gpu_accelerator_finish"). main() calls
- # mock_gpu_accelerator_compute() first.
+ # plugin to request three more breakpoints: the connection hook (hit
+ # next, since main() connects right after initializing), one by address
+ # (on "mock_gpu_accelerator_compute", from the symbol value delivered
+ # with the hit), and one by name scoped to the "a.out" shared library (on
+ # "mock_gpu_accelerator_finish"). Only the CPU target exists until the
+ # connection hook is hit.
+ self.assertEqual(self.dbg.GetNumTargets(), 1)
+ process.Continue()
+ self.check_accelerator_breakpoint_stop(
+ process, "mock_gpu_accelerator_connect", hit_count=1
+ )
+
+ # The accelerator target now exists alongside the CPU target.
+ self.assertEqual(self.dbg.GetNumTargets(), 2)
+ accelerator_target = None
+ for i in range(self.dbg.GetNumTargets()):
+ candidate = self.dbg.GetTargetAtIndex(i)
+ if candidate != target:
+ accelerator_target = candidate
+ break
+ self.assertTrue(accelerator_target.IsValid())
+
+ # The accelerator process must be successfully connected and stopped,
+ # not merely a valid (but dead) process object.
+ accelerator_process = accelerator_target.GetProcess()
+ self.assertTrue(accelerator_process.IsValid())
+ self.assertState(accelerator_process.GetState(), lldb.eStateStopped)
+
+ # The mock accelerator register context serves a fixed, known set of
+ # register values (value == 0x1000 + register index). Read each register
+ # back over the connection and verify it round-trips with the expected
+ # value, confirming the accelerator's registers are actually served.
+ accelerator_frame = accelerator_process.GetThreadAtIndex(0).GetFrameAtIndex(0)
+ expected_registers = {
+ "r0": 0x1000,
+ "r1": 0x1001,
+ "sp": 0x1002,
+ "fp": 0x1003,
+ "pc": 0x1004,
+ "flags": 0x1005,
+ }
+ for name, value in expected_registers.items():
+ reg = accelerator_frame.FindRegister(name)
+ self.assertTrue(reg.IsValid(), "register %s should exist" % name)
+ self.assertEqual(
+ reg.GetValueAsUnsigned(),
+ value,
+ "register %s should read back its expected value" % name,
+ )
+
+ # With the accelerator connected, continue through the remaining
+ # breakpoint types: by address (on mock_gpu_accelerator_compute), then by
+ # name scoped to a shared library (on mock_gpu_accelerator_finish).
process.Continue()
self.check_accelerator_breakpoint_stop(
process, "mock_gpu_accelerator_compute", hit_count=1
diff --git a/lldb/test/API/accelerator/mock/TestMockAcceleratorPlugin.py b/lldb/test/API/accelerator/mock/TestMockAcceleratorPlugin.py
index 1eeeb7f731eb7..d276d029efdc1 100644
--- a/lldb/test/API/accelerator/mock/TestMockAcceleratorPlugin.py
+++ b/lldb/test/API/accelerator/mock/TestMockAcceleratorPlugin.py
@@ -140,3 +140,49 @@ def test_jAcceleratorPluginBreakpointHit_response(self):
# "mock_gpu_accelerator_compute" symbol value.
self.assertIn(2, new_bps)
self.assertEqual(new_bps[2]["by_address"]["load_address"], 0x4000)
+
+ # The initialize hit also arms the connection-trigger breakpoint by name.
+ self.assertIn(4, new_bps)
+ self.assertEqual(
+ new_bps[4]["by_name"]["function_name"], "mock_gpu_accelerator_connect"
+ )
+
+ @add_test_categories(["llgs"])
+ def test_jAcceleratorPluginBreakpointHit_returns_connect_info(self):
+ self.build()
+ self.set_inferior_startup_launch()
+ self.prep_debug_monitor_and_inferior()
+
+ self.add_qSupported_packets()
+ self.expect_gdbremote_sequence()
+
+ # Simulate the connection-trigger breakpoint (identifier 4) being hit.
+ # The plugin responds with connect_info describing the reverse
+ # connection the client should make to the mock accelerator GDB server.
+ hit_args = {
+ "plugin_name": "mock",
+ "breakpoint": {"identifier": 4, "symbol_names": []},
+ "symbol_values": [],
+ }
+ hit_json = json.dumps(hit_args, separators=(",", ":"))
+ escaped_json = escape_binary(hit_json)
+ response = self.send_and_decode_json(
+ "jAcceleratorPluginBreakpointHit:" + escaped_json
+ )
+
+ self.assertTrue(response["disable_bp"])
+ self.assertFalse(response["auto_resume_native"])
+
+ actions = response["actions"]
+ self.assertEqual(actions["plugin_name"], "mock")
+ self.assertEqual(actions["session_name"], "Mock Accelerator Session")
+
+ # The connect_info must carry a reverse-connect URL to a local port and
+ # request a synchronous connection.
+ self.assertIn("connect_info", actions)
+ connect_info = actions["connect_info"]
+ self.assertTrue(
+ connect_info["connect_url"].startswith("connect://localhost:"),
+ connect_info["connect_url"],
+ )
+ self.assertTrue(connect_info["synchronous"])
diff --git a/lldb/test/API/accelerator/mock/main.c b/lldb/test/API/accelerator/mock/main.c
index 5f4c7b1fd3147..1e90585124472 100644
--- a/lldb/test/API/accelerator/mock/main.c
+++ b/lldb/test/API/accelerator/mock/main.c
@@ -1,5 +1,5 @@
-// The mock accelerator plugin sets its initialize breakpoint on this function.
-// Using a dedicated, uniquely named function (rather than "main") ensures the
+// The mock accelerator plugin sets its breakpoints on these dedicated, uniquely
+// named functions. Using dedicated functions (rather than "main") ensures the
// mock plugin only affects this test program and not other inferiors launched
// by lldb-server.
void mock_gpu_accelerator_initialize(void) {}
@@ -8,8 +8,14 @@ int mock_gpu_accelerator_compute(int x) { return x * 2; }
int mock_gpu_accelerator_finish(void) { return 0; }
+// When the plugin's connection-trigger breakpoint on this function is hit, it
+// asks the client to create a second target and connect to the mock accelerator
+// GDB server.
+void mock_gpu_accelerator_connect(void) {}
+
int main(void) {
mock_gpu_accelerator_initialize();
+ mock_gpu_accelerator_connect();
int result = mock_gpu_accelerator_compute(21);
mock_gpu_accelerator_finish();
return result == 42 ? 0 : 1;
diff --git a/lldb/tools/lldb-server/Plugins/Accelerator/Mock/CMakeLists.txt b/lldb/tools/lldb-server/Plugins/Accelerator/Mock/CMakeLists.txt
index 9b5a82a087e3d..d6cdc603213e3 100644
--- a/lldb/tools/lldb-server/Plugins/Accelerator/Mock/CMakeLists.txt
+++ b/lldb/tools/lldb-server/Plugins/Accelerator/Mock/CMakeLists.txt
@@ -1,5 +1,8 @@
add_lldb_library(lldbServerPluginMockAccelerator
LLDBServerMockAcceleratorPlugin.cpp
+ ProcessMockAccelerator.cpp
+ ThreadMockAccelerator.cpp
+ RegisterContextMockAccelerator.cpp
LINK_LIBS
lldbHost
diff --git a/lldb/tools/lldb-server/Plugins/Accelerator/Mock/LLDBServerMockAcceleratorPlugin.cpp b/lldb/tools/lldb-server/Plugins/Accelerator/Mock/LLDBServerMockAcceleratorPlugin.cpp
index f89fb90e26aa6..1364363b14315 100644
--- a/lldb/tools/lldb-server/Plugins/Accelerator/Mock/LLDBServerMockAcceleratorPlugin.cpp
+++ b/lldb/tools/lldb-server/Plugins/Accelerator/Mock/LLDBServerMockAcceleratorPlugin.cpp
@@ -7,13 +7,54 @@
//===----------------------------------------------------------------------===//
#include "LLDBServerMockAcceleratorPlugin.h"
+#include "ProcessMockAccelerator.h"
+#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h"
+#include "Plugins/Process/gdb-remote/ProcessGDBRemoteLog.h"
+#include "lldb/Host/ProcessLaunchInfo.h"
+#include "lldb/Host/Socket.h"
+#include "lldb/Host/common/TCPSocket.h"
+#include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
+#include "lldb/Utility/Args.h"
+#include "lldb/Utility/Connection.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+#include "llvm/Support/FormatVariadic.h"
+
+using namespace lldb;
using namespace lldb_private;
using namespace lldb_private::lldb_server;
+using namespace lldb_private::process_gdb_remote;
LLDBServerMockAcceleratorPlugin::LLDBServerMockAcceleratorPlugin(
GDBServer &gdb_server, MainLoop &main_loop)
- : LLDBServerAcceleratorPlugin(gdb_server, main_loop) {}
+ : LLDBServerAcceleratorPlugin(gdb_server, main_loop) {
+ // Run a second gdb...
[truncated]
|
cc2b157 to
3373576
Compare
DavidSpickett
left a comment
There was a problem hiding this comment.
Just looked at the docs so far.
In there any need for an action that tells the client to disconnect from the accelerator debug server? You mentioned a default empty thread for when it wasn't doing anything, so I guess that the model is you'll connect once and keep that connection for the whole session.
3373576 to
f5f2586
Compare
|
I don't have anything to say on the code side, someone else can review that. |
|
Well I am still curious about this -
But it's not a concern for this PR. |
|
@JDevlieghere could you please also review this, when you get a chance? |
| llvm::StringRef triple = | ||
| connect_info.triple ? *connect_info.triple : llvm::StringRef(); |
There was a problem hiding this comment.
same here, maybe change to std::string?
There was a problem hiding this comment.
update it to std::string
| PlatformSP platform_sp = accelerator_target_sp->GetPlatform(); | ||
| if (!platform_sp) | ||
| return llvm::createStringError( | ||
| "no platform for the accelerator target connection"); |
There was a problem hiding this comment.
If we don't require a triple and don't have an executable specified, then the platform that this target has might not be what we expect and might change once the GPU target sets a new executable. So I am not sure we can infer anything about the platform here. It will probably just default to the host platform.
If we want to make sure we get the right platform right away, we need to specify a target triple, or have a valid executable when creating the target.
So we probably don't need to verify that the platform is here, or we need to ensure we give the target enough info (exe or triple) to make sure it selects the right platform and verify the platform is correct.
There was a problem hiding this comment.
makes sense. removed the validation for this PR.
|
|
||
| accelerator_target_sp->SetTargetSessionName(actions.session_name); | ||
|
|
||
| // Broadcast the target creation event so DAP can create a child session |
There was a problem hiding this comment.
We shouldn't mention DAP here. We should say something more generic like:
// Broadcast the new target creation event so clients of the API can detect when
// new targets are created.
There was a problem hiding this comment.
you are right. updated it.
There was a problem hiding this comment.
We need the action ID to be unique. Please copy the way upstream does this into LLDBServerPlugin.h:
AcceleratorActions GetNewAcceleratorAction() {
return AcceleratorActions(GetPluginName(), ++m_gpu_action_identifier);
}
Anyone plug-ins that are creating actions should use this API. So this code becomes:
AcceleratorActions actions = GetNewAcceleratorAction();
| // server. | ||
| response.disable_bp = true; | ||
| response.auto_resume_native = false; | ||
| AcceleratorActions actions(GetPluginName(), kBreakpointIDConnect); |
There was a problem hiding this comment.
Use LLDBServerPlugin::GetNewAcceleratorAction() here to ensure a unique action ID:
AcceleratorActions actions = GetNewAcceleratorAction();
| std::unique_ptr<process_gdb_remote::GDBRemoteCommunicationServerLLGS> | ||
| m_gpu_server_up; |
There was a problem hiding this comment.
This should go into LLDBServerPlugin.h like in the llvm-server-plugin branch. The branch has code like:
class LLDBServerPlugin {
protected:
// Add a version field to allow the APIs to change over time.
using GDBServer = process_gdb_remote::GDBRemoteCommunicationServerLLGS;
using Manager = NativeProcessProtocol::Manager;
GDBServer &m_native_process;
MainLoop &m_main_loop;
std::unique_ptr<Manager> m_process_manager_up;
std::unique_ptr<GDBServer> m_gdb_server;
bool m_is_listening = false;
bool m_is_connected = false;
uint32_t m_gpu_action_identifier = 0;
And then sublcasses can usde the m_gdb_server member variable if they want to.
There was a problem hiding this comment.
I original added it in derived class since it is the only one needs the m_gpu_server_up , but updated it based on your feedback.
| namespace process_gdb_remote { | ||
| class GDBRemoteCommunicationServerLLGS; | ||
| } // namespace process_gdb_remote | ||
|
|
There was a problem hiding this comment.
Remove this and see how llvm-server-plugin branch does this in LLDBServerPlugin.h:
class LLDBServerPlugin {
protected:
// Add a version field to allow the APIs to change over time.
using GDBServer = process_gdb_remote::GDBRemoteCommunicationServerLLGS;
using Manager = NativeProcessProtocol::Manager;
GDBServer &m_native_process;
MainLoop &m_main_loop;
std::unique_ptr<Manager> m_process_manager_up;
std::unique_ptr<GDBServer> m_gdb_server;
bool m_is_listening = false;
bool m_is_connected = false;
uint32_t m_gpu_action_identifier = 0;
There was a problem hiding this comment.
updated this to be similar to llvm-server-plugin branch
b618020 to
9fe0f2c
Compare
| protected: | ||
| GDBServer &m_gdb_server; | ||
| // The CPU process's GDB server that this plugin is attached to. | ||
| GDBServer &m_native_gdb_server; | ||
| MainLoop &m_main_loop; |
There was a problem hiding this comment.
We probably should rename this m_native_main_loop. We want to encourage LLDBServerPlugins to have their own main loop if possible. If we don't do this, then the packets from native could be slowed down by the GPU and vice versa.
| GDBServer &gdb_server, MainLoop &main_loop) | ||
| : LLDBServerAcceleratorPlugin(gdb_server, main_loop) {} | ||
| : LLDBServerAcceleratorPlugin(gdb_server, main_loop) { |
There was a problem hiding this comment.
rename gdb_server to native_gdb_server here and in the .h file. Rename main_loop to native_main_loop
0694448 to
4a6e1f7
Compare
| Status error = debugger.GetTargetList().CreateTarget( | ||
| debugger, exe_path, triple, eLoadDependentsNo, | ||
| /*platform_options=*/nullptr, accelerator_target_sp); |
There was a problem hiding this comment.
Thinking about this more, we want to require a platform name in the AcceleratorConnectionInfo as a std::string platform_name; and fill the name with a call toOptionGroupPlatform.SetPlatformName(...) with this name prior to creating the target. Why? Because this ensures we get the right platform in order for us to call Platform::ConnectProcess or Platform::ConnectProcessSynchronous since each GPU vendor might need to do something before connecting to the GDB server in this case. But in general other GPU vendors might choose to actually launch their own GPU GDB server and we need to have the right platform for this. For the mock GPU, we should specify "host" as our platform name.
| /// Target triple to use as the architecture for the accelerator target. | ||
| std::optional<std::string> triple; |
There was a problem hiding this comment.
It might be nice to require the triple to be filled out. This would ensure that the platform matches the architecture. So make this not optional as it will help ensure that our platform is able to handle this triple. We can set this to the triple of the main executable for the mock plug-in.
| // Create a new (empty) target for the accelerator and connect to the GDB | ||
| // server the plugin is serving. | ||
| std::string exe_path = connect_info.exe_path.value_or(""); | ||
| std::string triple = connect_info.triple.value_or(""); |
There was a problem hiding this comment.
As stated above, this should be required so we can ensure that our architecture is compatible with the platform that is specified. See comment below about the platform for full details.
There was a problem hiding this comment.
made both fields required.
| // fixed set of registers. The client creates the accelerator target and | ||
| // connects it to this server. | ||
| m_process_manager_up = | ||
| std::make_unique<ProcessMockAccelerator::Manager>(m_native_main_loop); |
There was a problem hiding this comment.
Create our own main loop as an ivar in this class and use it here.
|
Add a test with an invalid platform name. Verify we fail to connect mock-test-app --platform --arch |
We can't do this with the program, we need to pass env vars to the lldb-server. |
Add AcceleratorConnectionInfo, describing how the client should create a new target and reverse-connect to a separate GDB server that serves an accelerator's state (e.g. a GPU debug stub). It carries the required connect URL plus an optional exe path, triple, and a synchronous flag. A connect_info field is added to AcceleratorActions so a plugin can ask the client to establish such a connection alongside (or instead of) setting breakpoints. This is the wire-format foundation; the client-side handling that acts on connect_info comes in a follow-up. Adds JSON serialization, unit tests, and documents the new fields in the jAcceleratorPluginInitialize packet.
7b7aca3 to
e40c507
Compare
Added the tests using the env variable. |
| // LaunchProcess() is how LLGS obtains its current process; it routes to | ||
| // ProcessMockAccelerator::Manager::Launch() (which ignores this info) and | ||
| // only requires a non-empty argument list, so a single placeholder is enough. | ||
| ProcessLaunchInfo info; | ||
| Args args; | ||
| args.AppendArgument("/pretend/path/to/mockgpu"); | ||
| info.SetArguments(args, /*first_arg_is_executable=*/true); | ||
| m_accelerator_gdb_server->SetLaunchInfo(info); | ||
| if (Status error = m_accelerator_gdb_server->LaunchProcess(); error.Fail()) | ||
| LLDB_LOG(log, "failed to create mock accelerator process: {0}", | ||
| error.AsCString()); | ||
|
|
||
| // Listen on an ephemeral local port for the client's connection, registering | ||
| // the accept handler now -- before the main loop thread starts -- so the | ||
| // loop's handles are only ever touched from its own thread. | ||
| llvm::Expected<std::unique_ptr<TCPSocket>> sock = | ||
| Socket::TcpListen("localhost:0"); | ||
| if (!sock) { | ||
| LLDB_LOG_ERROR(log, sock.takeError(), | ||
| "mock accelerator failed to listen: {0}"); | ||
| return; | ||
| } | ||
| m_listen_socket = std::move(*sock); | ||
| llvm::Expected<std::vector<MainLoopBase::ReadHandleUP>> handles = | ||
| m_listen_socket->Accept( | ||
| m_mock_main_loop, [this](std::unique_ptr<Socket> socket) { | ||
| std::unique_ptr<Connection> connection_up = | ||
| std::make_unique<ConnectionFileDescriptor>(std::move(socket)); | ||
| m_accelerator_gdb_server->InitializeConnection( | ||
| std::move(connection_up)); | ||
| }); | ||
| if (!handles) { | ||
| LLDB_LOG_ERROR(log, handles.takeError(), | ||
| "mock accelerator failed to accept: {0}"); | ||
| return; | ||
| } | ||
| m_read_handles = std::move(*handles); | ||
| m_connect_url = llvm::formatv("connect://localhost:{0}", | ||
| m_listen_socket->GetLocalPortNumber()); | ||
|
|
||
| // Service the mock accelerator server on its own thread. | ||
| llvm::Expected<HostThread> loop_thread = ThreadLauncher::LaunchThread( | ||
| "mock-accel.loop", [this]() -> lldb::thread_result_t { | ||
| m_mock_main_loop.Run(); | ||
| return {}; | ||
| }); | ||
| if (!loop_thread) { | ||
| LLDB_LOG_ERROR(log, loop_thread.takeError(), | ||
| "mock accelerator failed to start its main loop: {0}"); | ||
| m_connect_url.clear(); | ||
| return; | ||
| } | ||
| m_mock_main_loop_thread = *loop_thread; | ||
| } |
There was a problem hiding this comment.
So all of this work doesn't need to happen until our connection breakpoint is hit. We don't need to do this in the constructor. Can we move this into the handler for the connection breakpoint hit?
There was a problem hiding this comment.
Moved all the constructor code into CreateConnection()
| } | ||
|
|
||
| return response; | ||
| } | ||
|
|
||
| std::optional<AcceleratorConnectionInfo> | ||
| LLDBServerMockAcceleratorPlugin::CreateConnection() { |
There was a problem hiding this comment.
This is where we should move all of the code from the constructor. Also in all of that code make sure there are no instance variables that we don't need anymore since some work was being done in the constructor and then this function was using those instance variables.
There was a problem hiding this comment.
moved the connection code to this function.
but unfortunately all the instance variables (m_mock_main_loop, m_mock_main_loop_thread, m_listen_socket, and m_read_handles) I added are still needed to be alive for accepting the requests after CreateConnection returns.
e40c507 to
3a5823d
Compare
Act on the AcceleratorConnectionInfo returned by an accelerator plugin: when an AcceleratorActions carries connect_info, ProcessGDBRemote creates a new target and reverse-connects it to the GDB server the plugin points at, broadcasting the new target so listeners (e.g. IDEs) can pick it up. To exercise this end to end, the mock accelerator plugin runs a second gdb-remote server inside the lldb-server process, backed by a minimal synthetic accelerator process (ProcessMockAccelerator + one stopped ThreadMockAccelerator + a tiny RegisterContextMockAccelerator with fixed register values); no real process is launched. When the initialize breakpoint is hit, the plugin requests a breakpoint on a dedicated "mock_gpu_accelerator_connect" hook; hitting that hook returns connect_info so the client connects. The hook is added to the existing mock inferior used by the breakpoint test. The existing end-to-end breakpoint test is extended to continue to the connection hook and verify that a second (accelerator) target is created alongside the original one, that its process is connected and stopped, and that each of its registers reads back the expected value.
3a5823d to
0ee6e3d
Compare
Summary This builds on the accelerator plugin protocol (llvm#201489) by letting a plugin ask the client to create and connect a second target — the mechanism a real backend (e.g. a GPU debug stub) uses to surface the accelerator alongside the CPU process being debugged. ### What this adds **Protocol** — a new `AcceleratorConnectionInfo` describing how the client should bring up the accelerator target. **Client** — when an `AcceleratorActions` carries `connect_info`, `ProcessGDBRemote` creates a new (empty) target, reverse-connects it to the GDB server the plugin points. **Mock server (for testing)** — to exercise this end to end, the mock accelerator plugin stands up an in-process GDB server backed by a minimal fake process: `ProcessMockAccelerator`, `ThreadMockAccelerator` and `RegisterContextMockAccelerator`. ### Scope Intentionally minimal: the goal is to create and show the second (accelerator) target. There is no register/memory modeling beyond what a connection requires. Next set of PRs will build up on this. ### Testing Demo of how this looks with this PR. (explicitly asking the native process to stop at every breakpoint for testability) ``` satyajanga@devgpu011:toolchain $ ./bin/lldb (lldb) file /home/satyajanga/llvm-sand/build/Debug/fbcode-x86_64/toolchain/lldb-test-build.noindex/accelerator/mock/TestMockAcceleratorActions/a.out Current executable set to '/home/satyajanga/llvm-sand/build/Debug/fbcode-x86_64/toolchain/lldb-test-build.noindex/accelerator/mock/TestMockAcceleratorActions/a.out' (x86_64). (lldb) log enable -f /tmp/packets.log gdb-remote packets (lldb) r Process 1430531 launched: '/home/satyajanga/llvm-sand/build/Debug/fbcode-x86_64/toolchain/lldb-test-build.noindex/accelerator/mock/TestMockAcceleratorActions/a.out' (x86_64) Process 1430531 stopped (lldb) target list Current targets: * target #0: /home/satyajanga/llvm-sand/build/Debug/fbcode-x86_64/toolchain/lldb-test-build.noindex/accelerator/mock/TestMockAcceleratorActions/a.out ( arch=x86_64-unknown-linux-gnu, platform=host, pid=1430531, state=stopped ) (lldb) c Process 1430531 resuming Process 1 stopped * thread llvm#1, name = 'Mock Accelerator Thread', stop reason = trace frame #0: 0x0000000000001004 error: memory read failed for 0x1000 Target 0: (a.out) stopped. (lldb) target list Current targets: target #0: /home/satyajanga/llvm-sand/build/Debug/fbcode-x86_64/toolchain/lldb-test-build.noindex/accelerator/mock/TestMockAcceleratorActions/a.out ( arch=x86_64-unknown-linux-gnu, platform=host, pid=1430531, state=stopped ) * target llvm#1: <none> ( arch=x86_64-unknown-linux-gnu, platform=host, pid=1, state=stopped ) (lldb) c Process 1 resuming (lldb) target select 0 Current targets: * target #0: /home/satyajanga/llvm-sand/build/Debug/fbcode-x86_64/toolchain/lldb-test-build.noindex/accelerator/mock/TestMockAcceleratorActions/a.out ( arch=x86_64-unknown-linux-gnu, platform=host, pid=1430531, state=stopped ) target llvm#1: <none> ( arch=x86_64-unknown-linux-gnu, platform=host, pid=1, state=running ) (lldb) c Process 1430531 resuming Process 1430531 stopped (lldb) target list Current targets: * target #0: /home/satyajanga/llvm-sand/build/Debug/fbcode-x86_64/toolchain/lldb-test-build.noindex/accelerator/mock/TestMockAcceleratorActions/a.out ( arch=x86_64-unknown-linux-gnu, platform=host, pid=1430531, state=stopped ) target llvm#1: <none> ( arch=x86_64-unknown-linux-gnu, platform=host, pid=1, state=running ) (lldb) a.out │ internal accelerator-plugin (mock) breakpoint(-4). ``` Packet log, you can notice the actions set with breakpoint info and later responding with connection info. and you can also see the 5 GPRs ``` lldb < 104> send packet: $qSupported:xmlRegisters=i386,arm,mips,arc;multiprocess+;fork-events+;vfork-events+;swbreak+;hwbreak+#cd lldb < 299> read packet: $PacketSize=131072;QStartNoAckMode+;qEcho+;native-signals+;QThreadSuffixSupported+;QListThreadsInStopReply+;qXfer:features:read+;QNonStop+;jMultiBreakpoint+;QPassSignals+;qXfer:auxv:read+;qXfer:libraries-svr4:read+;qXfer:siginfo:read+;accelerator-plugins+;multiprocess+;fork-events+;vfork-events+llvm#86 .. lldb < 32> send packet: $jAcceleratorPluginInitialize#50 lldb < 238> read packet: $[{"breakpoints":[{"by_address":null,"by_name":{"function_name":"mock_gpu_accelerator_initialize","shlib":null}],"identifier":1,"symbol_names":["mock_gpu_accelerator_compute"]}]],"identifier":1,"plugin_name":"mock","session_name":""}]]#2f .. intern-state < 314> send packet: $jAcceleratorPluginBreakpointHit:{"breakpoint":{"by_address":null,"by_name":{"function_name":"mock_gpu_accelerator_initialize","shlib":null}],"identifier":1,"symbol_names":["mock_gpu_accelerator_compute"]}],"plugin_name":"mock","symbol_values":[{"name":"mock_gpu_accelerator_compute","value":93824992235840}]]}]#e7 intern-state < 487> read packet: ${"actions":{"breakpoints":[{"by_address":null,"by_name":{"function_name":"mock_gpu_accelerator_finish","shlib":"a.out"}],"identifier":3,"symbol_names":[]}],{"by_address":{"load_address":93824992235840}],"by_name":null,"identifier":2,"symbol_names":[]}],{"by_address":null,"by_name":{"function_name":"mock_gpu_accelerator_connect","shlib":null}],"identifier":4,"symbol_names":[]}]],"identifier":2,"plugin_name":"mock","session_name":""}],"auto_resume_native":false,"disable_bp":true}]#7d .. .. intern-state < 218> send packet: $jAcceleratorPluginBreakpointHit:{"breakpoint":{"by_address":null,"by_name":{"function_name":"mock_gpu_accelerator_connect","shlib":null}],"identifier":4,"symbol_names":[]}],"plugin_name":"mock","symbol_values":[]}]#d1 intern-state < 268> read packet: ${"actions":{"breakpoints":[],"connect_info":{"connect_url":"connect://localhost:32893","exe_path":null,"synchronous":true,"triple":null}],"identifier":4,"plugin_name":"mock","session_name":"Mock Accelerator Session"}],"auto_resume_native":false,"disable_bp":true}]#8e intern-state < 1> send packet: + intern-state history[1] tid=0x15d41f < 1> send packet: + intern-state < 19> send packet: $QStartNoAckMode#b0 intern-state < 1> read packet: + intern-state < 6> read packet: $OK#9a intern-state < 1> send packet: + intern-state < 104> send packet: $qSupported:xmlRegisters=i386,arm,mips,arc;multiprocess+;fork-events+;vfork-events+;swbreak+;hwbreak+#cd intern-state < 159> read packet: $PacketSize=131072;QStartNoAckMode+;qEcho+;native-signals+;QThreadSuffixSupported+;QListThreadsInStopReply+;qXfer:features:read+;QNonStop+;jMultiBreakpoint+#f3 intern-state < 26> send packet: $QThreadSuffixSupported#e4 intern-state < 6> read packet: $OK#9a intern-state < 27> send packet: $QListThreadsInStopReply#21 intern-state < 6> read packet: $OK#9a intern-state < 13> send packet: $qHostInfo#9b intern-state < 364> read packet: $triple:7838365f36342d2d6c696e75782d676e75;ptrsize:8;watchpoint_exceptions_received:after;endian:little;os_version:6.13.2;os_build:362e31332e322d305f66626b355f 68617264656e65645f7263325f305f67313733613962316463613431;os_kernel:233120534d5020576564204a756c2032332030393a32343a3330205044542032303235;hostname:6465766770753031312e656167322e66616365626f6f6b2e6 36f6d;#c4 intern-state < 10> send packet: $vCont?llvm#49 intern-state < 19> read packet: $vCont;c;C;s;S;t#11 intern-state < 27> send packet: $qVAttachOrWaitSupported#38 intern-state < 6> read packet: $OK#9a intern-state < 23> send packet: $QEnableErrorStrings#8c intern-state < 6> read packet: $OK#9a intern-state < 16> send packet: $qProcessInfo#dc intern-state < 7> read packet: $E01#a6 intern-state < 6> send packet: $qC#b4 intern-state < 7> read packet: $QC1#c5 intern-state < 5> send packet: $?#3f intern-state < 291> read packet: $T00thread:1;name:Mock Accelerator Thread;threads:1;thread-pcs:0000000000001004;00:0010000000000000;01:0110000000000000;02:0210000000000000;03:0310000000000000 ;04:0410000000000000;05:0510000000000000;reason:trace;description:6d6f636b20616363656c657261746f72207468726561642073746f70706564;llvm#83 intern-state < 16> send packet: $qProcessInfo#dc intern-state < 7> read packet: $E01#a6 intern-state < 16> send packet: $qProcessInfo#dc intern-state < 7> read packet: $E01#a6 intern-state < 43> send packet: $qXfer:features:read:target.xml:0,131071#78 intern-state < 889> read packet: $l<?xml version="1.0"?> <target version="1.0"> <architecture>x86_64</architecture> <feature> <reg name="r0" bitsize="64" regnum="0" offset="0" encoding="uint" format="hex" group="General Purpose Registers" /> <reg name="r1" bitsize="64" regnum="1" offset="8" encoding="uint" format="hex" group="General Purpose Registers" /> <reg name="sp" bitsize="64" regnum="2" offset="16" encoding="uint" format="hex" group="General Purpose Registers" generic="sp" /> <reg name="fp" bitsize="64" regnum="3" offset="24" encoding="uint" format="hex" group="General Purpose Registers" generic="fp" /> <reg name="pc" bitsize="64" regnum="4" offset="32" encoding="uint" format="hex" group="General Purpose Registers" generic="pc" /> <reg name="flags" bitsize="64" regnum="5" offset="40" encoding="uint" format="hex" group="General Purpose Registers" /> </feature> </target> llvm#82 intern-state < 19> send packet: $p0;thread:0001;llvm#89 intern-state < 20> read packet: $0010000000000000#01 intern-state < 16> send packet: $qProcessInfo#dc intern-state < 7> read packet: $E01#a6 intern-state < 19> send packet: $p0;thread:0001;llvm#89 intern-state < 20> read packet: $0010000000000000#01 intern-state < 16> send packet: $qProcessInfo#dc intern-state < 7> read packet: $E01#a6 intern-state < 16> send packet: $qProcessInfo#dc intern-state < 7> read packet: $E01#a6 intern-state < 16> send packet: $qProcessInfo#dc intern-state < 7> read packet: $E01#a6 intern-state < 26> send packet: $qStructuredDataPlugins#02 intern-state < 6> read packet: $[]#b8 intern-state < 26> send packet: $qMemoryRegionInfo:1004#d9 intern-state < 41> read packet: $error:6e6f7420696d706c656d656e746564;#f7 intern-state < 16> send packet: $jThreadsInfo#c1 intern-state < 198> read packet: $[{"description":"mock accelerator thread stopped","name":"Mock Accelerator Thread","reason":"trace","registers":{"2":"0210000000000000","3":"0310000000000000" ,"4":"0410000000000000"}],"tid":1}]]#4a ```
Summary
This builds on the accelerator plugin protocol (#201489) by letting a plugin ask the client to create and connect a second
target — the mechanism a real backend (e.g. a GPU debug stub) uses to surface
the accelerator alongside the CPU process being debugged.
What this adds
Protocol — a new
AcceleratorConnectionInfodescribing how the clientshould bring up the accelerator target.
Client — when an
AcceleratorActionscarriesconnect_info,ProcessGDBRemotecreates a new (empty) target, reverse-connects it to the GDBserver the plugin points.
Mock server (for testing) — to exercise this end to end, the mock
accelerator plugin stands up an in-process GDB server backed by a minimal fake
process:
ProcessMockAccelerator,ThreadMockAcceleratorandRegisterContextMockAccelerator.Scope
Intentionally minimal: the goal is to create and show the second (accelerator)
target. There is no register/memory modeling beyond what a connection requires. Next set of PRs will build up on this.
Testing
Demo of how this looks with this PR. (explicitly asking the native process to stop at every breakpoint for testability)
Packet log,
you can notice the actions set with breakpoint info and later responding with connection info. and you can also see the 5 GPRs