[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

[Michał Górny via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG312257688eb0: [lldb] [Process] Introduce protocol extension 
support API (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100153/new/

https://reviews.llvm.org/D100153

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/test_categories.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py

Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -954,23 +954,58 @@
 self.set_inferior_startup_launch()
 self.breakpoint_set_and_remove_work(want_hardware=True)
 
-def test_qSupported_returns_known_stub_features(self):
+def get_qSupported_dict(self, features=[]):
 self.build()
 self.set_inferior_startup_launch()
 
 # Start up the stub and start/prep the inferior.
 procs = self.prep_debug_monitor_and_inferior()
-self.add_qSupported_packets()
+self.add_qSupported_packets(features)
 
 # Run the packet stream.
 context = self.expect_gdbremote_sequence()
 self.assertIsNotNone(context)
 
 # Retrieve the qSupported features.
-supported_dict = self.parse_qSupported_response(context)
+return self.parse_qSupported_response(context)
+
+def test_qSupported_returns_known_stub_features(self):
+supported_dict = self.get_qSupported_dict()
 self.assertIsNotNone(supported_dict)
 self.assertTrue(len(supported_dict) > 0)
 
+@add_test_categories(["fork"])
+def test_qSupported_fork_events(self):
+supported_dict = (
+self.get_qSupported_dict(['multiprocess+', 'fork-events+']))
+self.assertEqual(supported_dict.get('multiprocess', '-'), '+')
+self.assertEqual(supported_dict.get('fork-events', '-'), '+')
+self.assertEqual(supported_dict.get('vfork-events', '-'), '-')
+
+@add_test_categories(["fork"])
+def test_qSupported_fork_events_without_multiprocess(self):
+supported_dict = (
+self.get_qSupported_dict(['fork-events+']))
+self.assertEqual(supported_dict.get('multiprocess', '-'), '-')
+self.assertEqual(supported_dict.get('fork-events', '-'), '-')
+self.assertEqual(supported_dict.get('vfork-events', '-'), '-')
+
+@add_test_categories(["fork"])
+def test_qSupported_vfork_events(self):
+supported_dict = (
+self.get_qSupported_dict(['multiprocess+', 'vfork-events+']))
+self.assertEqual(supported_dict.get('multiprocess', '-'), '+')
+self.assertEqual(supported_dict.get('fork-events', '-'), '-')
+self.assertEqual(supported_dict.get('vfork-events', '-'), '+')
+
+@add_test_categories(["fork"])
+def test_qSupported_vfork_events_without_multiprocess(self):
+supported_dict = (
+self.get_qSupported_dict(['vfork-events+']))
+self.assertEqual(supported_dict.get('multiprocess', '-'), '-')
+self.assertEqual(supported_dict.get('fork-events', '-'), '-')
+self.assertEqual(supported_dict.get('vfork-events', '-'), '-')
+
 @skipIfWindows # No pty support to test any inferior output
 def test_written_M_content_reads_back_correctly(self):
 self.build()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -103,6 +103,8 @@
   bool m_thread_suffix_supported = false;
   bool m_list_threads_in_stop_reply = false;
 
+  NativeProcessProtocol::Extension m_extensions_supported = {};
+
   PacketResult SendONotification(const char *buffer, uint32_t len);
 
   PacketResult SendWResponse(NativeProcessProtocol *process);
@@ -264,6 +266,9 @@
   llvm::Expected ReadTid(StringExtractorGDBRemote ,
   bool allow_all, lldb::pid_t default_pid);
 
+  // Call SetEnabledExtensions() with appropriate flags on the process.
+  void SetEnabledExtensions(NativeProcessProtocol );
+
   // For GDBRemoteCommunicationServerLLGS only
   GDBRemoteCommunicationServerLLGS(const GDBRemoteCommunicationServerLLGS &) =
   delete;
Index: 

[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 340177.
mgorny added a comment.

Added `Extension` bit for `multiprocess`. Made `fork-events` and `vfork-events` 
both depend on it — i.e. be implicitly disabled if `multiprocess` wasn't 
reported as supported.

Simplify `GDBRemoteCommunicationServerLLGS::SetEnabledExtensions()` — we can 
just take the value instead of copying it bit-by-bit ;-).

Move `add_qSupported_packets()` change from later patch. Add tests for 
`qSupported` responses (particularly, rejecting `[v]fork-events` if 
`multiprocess` is not present). The new tests are in `fork` category that's 
skipped on all platforms where fork/vfork isn't enabled yet (i.e. everywhere in 
this patch).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100153/new/

https://reviews.llvm.org/D100153

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/test_categories.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py

Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -954,23 +954,58 @@
 self.set_inferior_startup_launch()
 self.breakpoint_set_and_remove_work(want_hardware=True)
 
-def test_qSupported_returns_known_stub_features(self):
+def get_qSupported_dict(self, features=[]):
 self.build()
 self.set_inferior_startup_launch()
 
 # Start up the stub and start/prep the inferior.
 procs = self.prep_debug_monitor_and_inferior()
-self.add_qSupported_packets()
+self.add_qSupported_packets(features)
 
 # Run the packet stream.
 context = self.expect_gdbremote_sequence()
 self.assertIsNotNone(context)
 
 # Retrieve the qSupported features.
-supported_dict = self.parse_qSupported_response(context)
+return self.parse_qSupported_response(context)
+
+def test_qSupported_returns_known_stub_features(self):
+supported_dict = self.get_qSupported_dict()
 self.assertIsNotNone(supported_dict)
 self.assertTrue(len(supported_dict) > 0)
 
+@add_test_categories(["fork"])
+def test_qSupported_fork_events(self):
+supported_dict = (
+self.get_qSupported_dict(['multiprocess+', 'fork-events+']))
+self.assertEqual(supported_dict.get('multiprocess', '-'), '+')
+self.assertEqual(supported_dict.get('fork-events', '-'), '+')
+self.assertEqual(supported_dict.get('vfork-events', '-'), '-')
+
+@add_test_categories(["fork"])
+def test_qSupported_fork_events_without_multiprocess(self):
+supported_dict = (
+self.get_qSupported_dict(['fork-events+']))
+self.assertEqual(supported_dict.get('multiprocess', '-'), '-')
+self.assertEqual(supported_dict.get('fork-events', '-'), '-')
+self.assertEqual(supported_dict.get('vfork-events', '-'), '-')
+
+@add_test_categories(["fork"])
+def test_qSupported_vfork_events(self):
+supported_dict = (
+self.get_qSupported_dict(['multiprocess+', 'vfork-events+']))
+self.assertEqual(supported_dict.get('multiprocess', '-'), '+')
+self.assertEqual(supported_dict.get('fork-events', '-'), '-')
+self.assertEqual(supported_dict.get('vfork-events', '-'), '+')
+
+@add_test_categories(["fork"])
+def test_qSupported_vfork_events_without_multiprocess(self):
+supported_dict = (
+self.get_qSupported_dict(['vfork-events+']))
+self.assertEqual(supported_dict.get('multiprocess', '-'), '-')
+self.assertEqual(supported_dict.get('fork-events', '-'), '-')
+self.assertEqual(supported_dict.get('vfork-events', '-'), '-')
+
 @skipIfWindows # No pty support to test any inferior output
 def test_written_M_content_reads_back_correctly(self):
 self.build()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -103,6 +103,8 @@
   bool m_thread_suffix_supported = false;
   bool m_list_threads_in_stop_reply = false;
 
+  NativeProcessProtocol::Extension m_extensions_supported = {};
+
   PacketResult SendONotification(const char *buffer, uint32_t len);
 
   PacketResult SendWResponse(NativeProcessProtocol *process);
@@ -264,6 +266,9 @@
   llvm::Expected ReadTid(StringExtractorGDBRemote ,

[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 339193.
mgorny marked an inline comment as done.
mgorny added a comment.

Implemented the suggested changes, including `bool()` use in 
`SetEnabledExtensions()`. Removed the client announcements of `fork-events` and 
`vfork-events` support.

I've left the server parts of `fork-events` and `vfork-events`. It won't 
trigger until both the process plugin and the client indicates support too.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100153/new/

https://reviews.llvm.org/D100153

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -103,6 +103,8 @@
   bool m_thread_suffix_supported = false;
   bool m_list_threads_in_stop_reply = false;
 
+  NativeProcessProtocol::Extension m_extensions_supported = {};
+
   PacketResult SendONotification(const char *buffer, uint32_t len);
 
   PacketResult SendWResponse(NativeProcessProtocol *process);
@@ -264,6 +266,9 @@
   llvm::Expected ReadTid(StringExtractorGDBRemote ,
   bool allow_all, lldb::pid_t default_pid);
 
+  // Call SetEnabledExtensions() with appropriate flags on the process.
+  void SetEnabledExtensions(NativeProcessProtocol );
+
   // For GDBRemoteCommunicationServerLLGS only
   GDBRemoteCommunicationServerLLGS(const GDBRemoteCommunicationServerLLGS &) =
   delete;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -259,6 +259,8 @@
 m_continue_process = m_current_process = m_debugged_process_up.get();
   }
 
+  SetEnabledExtensions(*m_current_process);
+
   // Handle mirroring of inferior stdout/stderr over the gdb-remote protocol as
   // needed. llgs local-process debugging may specify PTY paths, which will
   // make these file actions non-null process launch -i/e/o will also make
@@ -327,6 +329,7 @@
   }
   m_debugged_process_up = std::move(*process_or);
   m_continue_process = m_current_process = m_debugged_process_up.get();
+  SetEnabledExtensions(*m_current_process);
 
   // Setup stdout/stderr mapping from inferior.
   auto terminal_fd = m_debugged_process_up->GetTerminalFileDescriptor();
@@ -3557,7 +3560,7 @@
 
 std::vector GDBRemoteCommunicationServerLLGS::HandleFeatures(
 const llvm::ArrayRef client_features) {
-  auto ret =
+  std::vector ret =
   GDBRemoteCommunicationServerCommon::HandleFeatures(client_features);
   ret.insert(ret.end(), {
 "QThreadSuffixSupported+", "QListThreadsInStopReply+",
@@ -3566,5 +3569,36 @@
 "QPassSignals+", "qXfer:auxv:read+", "qXfer:libraries-svr4:read+",
 #endif
   });
+
+  // check for client features
+  using Extension = NativeProcessProtocol::Extension;
+  m_extensions_supported = {};
+  for (llvm::StringRef x : client_features)
+m_extensions_supported |= llvm::StringSwitch(x)
+  .Case("fork-events+", Extension::fork)
+  .Case("vfork-events+", Extension::vfork)
+  .Default({});
+  m_extensions_supported &= m_process_factory.GetSupportedExtensions();
+
+  // report only if actually supported
+  if (bool(m_extensions_supported & NativeProcessProtocol::Extension::fork))
+ret.push_back("fork-events+");
+  if (bool(m_extensions_supported & NativeProcessProtocol::Extension::vfork))
+ret.push_back("vfork-events+");
+
+  if (m_debugged_process_up)
+SetEnabledExtensions(*m_debugged_process_up);
   return ret;
 }
+
+void GDBRemoteCommunicationServerLLGS::SetEnabledExtensions(
+NativeProcessProtocol ) {
+  NativeProcessProtocol::Extension flags = {};
+  if (bool(m_extensions_supported & NativeProcessProtocol::Extension::fork))
+flags |= NativeProcessProtocol::Extension::fork;
+  if (bool(m_extensions_supported & NativeProcessProtocol::Extension::vfork))
+flags |= NativeProcessProtocol::Extension::vfork;
+
+  assert(!bool(flags & ~m_process_factory.GetSupportedExtensions()));
+  process.SetEnabledExtensions(flags);
+}
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++ 

[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3562-3567
+(process_features & NativeProcessProtocol::Extension::fork) ==
+NativeProcessProtocol::Extension::fork)
+  m_fork_events_supported = true;
+else if (x == "vfork-events+" &&
+ (process_features & NativeProcessProtocol::Extension::vfork) ==
+ NativeProcessProtocol::Extension::vfork)

mgorny wrote:
> mgorny wrote:
> > labath wrote:
> > > Maybe drop the `== Extension::[v]fork` part?
> > Can't do that, `enum class` doesn't convert to `bool`. In fact, I tried a 
> > few more or less crazy ideas to make this work, and none worked ;-).
> The next best thing I was able to do is check `!= 
> NativeProcessProtocol::Extension()` which is a little bit shorter. Maybe I 
> could try adding `operator==` and `!=` against, say, `nullptr` or some 
> special constant?
An explicit bool cast works though, and is less fancy. Maybe if we combine this 
the `m_enabled_extensions` idea, we could do something like:
```
using Extension = NativeProcessProtocol::Extension;
Extension client_extensions{};
for (StringRef x : client_features)
  client_extensions |= llvm::StringSwitch(x).Case("fork-events+", 
Extension::fork).Case("vfork-events+", Extension::vfork).Default({});
m_enabled_extensions = client_extensions & 
m_process_factory.GetSupportedExtensions();
if (bool(m_enabled_extensions & Extension::fork))
  ret.push_back("fork-events+");
if (bool(m_enabled_extensions & Extension::vfork))
  ret.push_back("vfork-events+");
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100153/new/

https://reviews.llvm.org/D100153

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

[Michał Górny via Phabricator via lldb-commits]

mgorny marked 4 inline comments as done.
mgorny added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3562-3567
+(process_features & NativeProcessProtocol::Extension::fork) ==
+NativeProcessProtocol::Extension::fork)
+  m_fork_events_supported = true;
+else if (x == "vfork-events+" &&
+ (process_features & NativeProcessProtocol::Extension::vfork) ==
+ NativeProcessProtocol::Extension::vfork)

mgorny wrote:
> labath wrote:
> > Maybe drop the `== Extension::[v]fork` part?
> Can't do that, `enum class` doesn't convert to `bool`. In fact, I tried a few 
> more or less crazy ideas to make this work, and none worked ;-).
The next best thing I was able to do is check `!= 
NativeProcessProtocol::Extension()` which is a little bit shorter. Maybe I 
could try adding `operator==` and `!=` against, say, `nullptr` or some special 
constant?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100153/new/

https://reviews.llvm.org/D100153

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

[Michał Górny via Phabricator via lldb-commits]

mgorny added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3562-3567
+(process_features & NativeProcessProtocol::Extension::fork) ==
+NativeProcessProtocol::Extension::fork)
+  m_fork_events_supported = true;
+else if (x == "vfork-events+" &&
+ (process_features & NativeProcessProtocol::Extension::vfork) ==
+ NativeProcessProtocol::Extension::vfork)

labath wrote:
> Maybe drop the `== Extension::[v]fork` part?
Can't do that, `enum class` doesn't convert to `bool`. In fact, I tried a few 
more or less crazy ideas to make this work, and none worked ;-).



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h:102-103
 
+  bool m_fork_events_supported = false;
+  bool m_vfork_events_supported = false;
+

labath wrote:
> I am wondering if this should just be `NativeProcessProtocol::Extension 
> m_enabled_extensions;` Can we think of an extension that would belong to 
> `NativeProcessProtocol::Extension`, but we would not want to store its state 
> in the GDBRemoteCommunicationServerLLGS class?
I can't think of any right now and I suppose even if there were one, there 
would probably be no big loss in using `Extension` here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100153/new/

https://reviews.llvm.org/D100153

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:389
+m_enabled_extensions = flags;
+return llvm::Error::success();
+  }

mgorny wrote:
> labath wrote:
> > Are you sure that returning success is the best "default" behavior? Maybe 
> > the base implementation should always return an error (as it does not 
> > implement any extensions)? Or return success, only if one enables an empty 
> > set of extensions?
> I'm not sure whether we need error handling here at all.
> 
> The current impl doesn't do anything but setting an instance var here. The 
> original impl controlled events to `PTRACE_SETOPTIONS` but in the end I've 
> decided to enable tracing fork/vfork unconditionally, and just using 
> extensions to decide whether to handle it locally or defer to client.
> 
> I suppose it might make sense to review other patches first and get back to 
> this one once we're sure what we need.
I think I agree with you. Let's just drop error handling altogether. Maybe add 
something like `assert(features && ~m_process_factory.GetSupportedExtensions() 
== {})` to `GDBRemoteCommunicationServerLLGS::SetEnabledExtensions`.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3562-3567
+(process_features & NativeProcessProtocol::Extension::fork) ==
+NativeProcessProtocol::Extension::fork)
+  m_fork_events_supported = true;
+else if (x == "vfork-events+" &&
+ (process_features & NativeProcessProtocol::Extension::vfork) ==
+ NativeProcessProtocol::Extension::vfork)

Maybe drop the `== Extension::[v]fork` part?



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h:102-103
 
+  bool m_fork_events_supported = false;
+  bool m_vfork_events_supported = false;
+

I am wondering if this should just be `NativeProcessProtocol::Extension 
m_enabled_extensions;` Can we think of an extension that would belong to 
`NativeProcessProtocol::Extension`, but we would not want to store its state in 
the GDBRemoteCommunicationServerLLGS class?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100153/new/

https://reviews.llvm.org/D100153

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 337122.
mgorny added a comment.

Rebase + clang-format.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100153/new/

https://reviews.llvm.org/D100153

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -99,6 +99,9 @@
   uint32_t m_next_saved_registers_id = 1;
   bool m_handshake_completed = false;
 
+  bool m_fork_events_supported = false;
+  bool m_vfork_events_supported = false;
+
   PacketResult SendONotification(const char *buffer, uint32_t len);
 
   PacketResult SendWResponse(NativeProcessProtocol *process);
@@ -256,6 +259,9 @@
   llvm::Expected ReadTid(StringExtractorGDBRemote ,
   bool allow_all = false);
 
+  // Call SetEnabledExtensions() with appropriate flags on the process.
+  void SetEnabledExtensions(NativeProcessProtocol );
+
   // For GDBRemoteCommunicationServerLLGS only
   GDBRemoteCommunicationServerLLGS(const GDBRemoteCommunicationServerLLGS &) =
   delete;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -251,6 +251,8 @@
 m_debugged_process_up = std::move(*process_or);
   }
 
+  SetEnabledExtensions(*m_debugged_process_up);
+
   // Handle mirroring of inferior stdout/stderr over the gdb-remote protocol as
   // needed. llgs local-process debugging may specify PTY paths, which will
   // make these file actions non-null process launch -i/e/o will also make
@@ -318,6 +320,7 @@
 return status;
   }
   m_debugged_process_up = std::move(*process_or);
+  SetEnabledExtensions(*m_debugged_process_up);
 
   // Setup stdout/stderr mapping from inferior.
   auto terminal_fd = m_debugged_process_up->GetTerminalFileDescriptor();
@@ -3545,5 +3548,49 @@
 "QPassSignals+", "qXfer:auxv:read+", "qXfer:libraries-svr4:read+",
 #endif
   });
+
+  // check for platform features
+  auto process_features = m_process_factory.GetSupportedExtensions();
+
+  // reset to defaults
+  m_fork_events_supported = false;
+  m_vfork_events_supported = false;
+
+  // check for client features
+  for (auto x : client_features) {
+if (x == "fork-events+" &&
+(process_features & NativeProcessProtocol::Extension::fork) ==
+NativeProcessProtocol::Extension::fork)
+  m_fork_events_supported = true;
+else if (x == "vfork-events+" &&
+ (process_features & NativeProcessProtocol::Extension::vfork) ==
+ NativeProcessProtocol::Extension::vfork)
+  m_vfork_events_supported = true;
+  }
+
+  // report only if actually supported
+  if (m_fork_events_supported)
+ret.push_back("fork-events+");
+  if (m_vfork_events_supported)
+ret.push_back("vfork-events+");
+
+  if (m_debugged_process_up)
+SetEnabledExtensions(*m_debugged_process_up);
   return ret;
 }
+
+void GDBRemoteCommunicationServerLLGS::SetEnabledExtensions(
+NativeProcessProtocol ) {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
+
+  NativeProcessProtocol::Extension flags = {};
+  if (m_fork_events_supported)
+flags |= NativeProcessProtocol::Extension::fork;
+  if (m_vfork_events_supported)
+flags |= NativeProcessProtocol::Extension::vfork;
+
+  llvm::Error error = process.SetEnabledExtensions(flags);
+  if (error)
+LLDB_LOG_ERROR(log, std::move(error),
+   "Enabling protocol extensions failed: {0}");
+}
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -353,7 +353,8 @@
 
   // build the qSupported packet
   std::vector features = {"xmlRegisters=i386,arm,mips,arc",
-   "multiprocess+"};
+   "multiprocess+", "fork-events+",
+   "vfork-events+"};
   StreamString packet;
   packet.PutCString("qSupported");
   for (uint32_t i = 0; i < features.size(); ++i) {
Index: 

[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

[Michał Górny via Phabricator via lldb-commits]

mgorny added inline comments.



Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:389
+m_enabled_extensions = flags;
+return llvm::Error::success();
+  }

labath wrote:
> Are you sure that returning success is the best "default" behavior? Maybe the 
> base implementation should always return an error (as it does not implement 
> any extensions)? Or return success, only if one enables an empty set of 
> extensions?
I'm not sure whether we need error handling here at all.

The current impl doesn't do anything but setting an instance var here. The 
original impl controlled events to `PTRACE_SETOPTIONS` but in the end I've 
decided to enable tracing fork/vfork unconditionally, and just using extensions 
to decide whether to handle it locally or defer to client.

I suppose it might make sense to review other patches first and get back to 
this one once we're sure what we need.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3592-3595
+  llvm::Error error = process.SetEnabledExtensions(flags);
+  if (error)
+LLDB_LOG_ERROR(log, std::move(error),
+   "Enabling protocol extensions failed: {0}");

labath wrote:
> ... or actually, I am wondering if this should not be a hard error/assertion. 
> In the current set up, I think it would be a programmer error if the factory 
> reports an extension as supported, but then the instance fails to enable it...
Technically, the original idea was that this would fail if `ptrace()` failed to 
set options. But as I said, this is no longer the case, so I'm not even sure if 
we need any error reporting here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100153/new/

https://reviews.llvm.org/D100153

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I like that (particularly the dependency graph :P). Let's just figure out the 
error handling..

In D100153#2678223 , @mgorny wrote:

> Note that I've included full `fork-events+` and `vfork-events+` as demo of 
> the API. I can split them further and/or move to more relevant commits as I 
> progress with the split.

Maybe we could start by feature-ifying one of the existing fields currently 
controlled by ifdefs. Say, `QPassSignals+`. It does not have a client 
component, so it won't cover everything, but I think it will give us something.




Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:389
+m_enabled_extensions = flags;
+return llvm::Error::success();
+  }

Are you sure that returning success is the best "default" behavior? Maybe the 
base implementation should always return an error (as it does not implement any 
extensions)? Or return success, only if one enables an empty set of extensions?



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3592-3595
+  llvm::Error error = process.SetEnabledExtensions(flags);
+  if (error)
+LLDB_LOG_ERROR(log, std::move(error),
+   "Enabling protocol extensions failed: {0}");

... or actually, I am wondering if this should not be a hard error/assertion. 
In the current set up, I think it would be a programmer error if the factory 
reports an extension as supported, but then the instance fails to enable it...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100153/new/

https://reviews.llvm.org/D100153

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 336516.
mgorny added a comment.

Fix uninitialized `flags` variable in 
`GDBRemoteCommunicationServerLLGS::SetEnabledExtensions()`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100153/new/

https://reviews.llvm.org/D100153

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -99,6 +99,9 @@
   uint32_t m_next_saved_registers_id = 1;
   bool m_handshake_completed = false;
 
+  bool m_fork_events_supported = false;
+  bool m_vfork_events_supported = false;
+
   PacketResult SendONotification(const char *buffer, uint32_t len);
 
   PacketResult SendWResponse(NativeProcessProtocol *process);
@@ -256,6 +259,9 @@
   llvm::Expected ReadTid(StringExtractorGDBRemote ,
   bool allow_all = false);
 
+  // Call SetEnabledExtensions() with appropriate flags on the process.
+  void SetEnabledExtensions(NativeProcessProtocol& process);
+
   // For GDBRemoteCommunicationServerLLGS only
   GDBRemoteCommunicationServerLLGS(const GDBRemoteCommunicationServerLLGS &) =
   delete;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -251,6 +251,8 @@
 m_debugged_process_up = std::move(*process_or);
   }
 
+  SetEnabledExtensions(*m_debugged_process_up);
+
   // Handle mirroring of inferior stdout/stderr over the gdb-remote protocol as
   // needed. llgs local-process debugging may specify PTY paths, which will
   // make these file actions non-null process launch -i/e/o will also make
@@ -318,6 +320,7 @@
 return status;
   }
   m_debugged_process_up = std::move(*process_or);
+  SetEnabledExtensions(*m_debugged_process_up);
 
   // Setup stdout/stderr mapping from inferior.
   auto terminal_fd = m_debugged_process_up->GetTerminalFileDescriptor();
@@ -3545,5 +3548,49 @@
 "QPassSignals+", "qXfer:auxv:read+", "qXfer:libraries-svr4:read+",
 #endif
   });
+
+  // check for platform features
+  auto process_features = m_process_factory.GetSupportedExtensions();
+
+  // reset to defaults
+  m_fork_events_supported = false;
+  m_vfork_events_supported = false;
+
+  // check for client features
+  for (auto x : client_features) {
+if (x == "fork-events+" &&
+(process_features & NativeProcessProtocol::Extension::fork) ==
+NativeProcessProtocol::Extension::fork)
+  m_fork_events_supported = true;
+else if (x == "vfork-events+" &&
+ (process_features & NativeProcessProtocol::Extension::vfork) ==
+ NativeProcessProtocol::Extension::vfork)
+  m_vfork_events_supported = true;
+  }
+
+  // report only if actually supported
+  if (m_fork_events_supported)
+ret.push_back("fork-events+");
+  if (m_vfork_events_supported)
+ret.push_back("vfork-events+");
+
+  if (m_debugged_process_up)
+SetEnabledExtensions(*m_debugged_process_up);
   return ret;
 }
+
+void GDBRemoteCommunicationServerLLGS::SetEnabledExtensions(
+NativeProcessProtocol ) {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
+
+  NativeProcessProtocol::Extension flags = {};
+  if (m_fork_events_supported)
+flags |= NativeProcessProtocol::Extension::fork;
+  if (m_vfork_events_supported)
+flags |= NativeProcessProtocol::Extension::vfork;
+
+  llvm::Error error = process.SetEnabledExtensions(flags);
+  if (error)
+LLDB_LOG_ERROR(log, std::move(error),
+   "Enabling protocol extensions failed: {0}");
+}
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -353,7 +353,9 @@
 
   // build the qSupported packet
   std::vector features = {"xmlRegisters=i386,arm,mips,arc",
-   "multiprocess+"};
+   "multiprocess+",
+   "fork-events+",
+   "vfork-events+"};
   StreamString packet;
   packet.PutCString("qSupported");
   for 

[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 336263.
mgorny added a comment.

Add to `_KNOWN_QSUPPORTED_STUB_FEATURES` and clang-format.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100153/new/

https://reviews.llvm.org/D100153

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -99,6 +99,9 @@
   uint32_t m_next_saved_registers_id = 1;
   bool m_handshake_completed = false;
 
+  bool m_fork_events_supported = false;
+  bool m_vfork_events_supported = false;
+
   PacketResult SendONotification(const char *buffer, uint32_t len);
 
   PacketResult SendWResponse(NativeProcessProtocol *process);
@@ -256,6 +259,9 @@
   llvm::Expected ReadTid(StringExtractorGDBRemote ,
   bool allow_all = false);
 
+  // Call SetEnabledExtensions() with appropriate flags on the process.
+  void SetEnabledExtensions(NativeProcessProtocol& process);
+
   // For GDBRemoteCommunicationServerLLGS only
   GDBRemoteCommunicationServerLLGS(const GDBRemoteCommunicationServerLLGS &) =
   delete;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -251,6 +251,8 @@
 m_debugged_process_up = std::move(*process_or);
   }
 
+  SetEnabledExtensions(*m_debugged_process_up);
+
   // Handle mirroring of inferior stdout/stderr over the gdb-remote protocol as
   // needed. llgs local-process debugging may specify PTY paths, which will
   // make these file actions non-null process launch -i/e/o will also make
@@ -318,6 +320,7 @@
 return status;
   }
   m_debugged_process_up = std::move(*process_or);
+  SetEnabledExtensions(*m_debugged_process_up);
 
   // Setup stdout/stderr mapping from inferior.
   auto terminal_fd = m_debugged_process_up->GetTerminalFileDescriptor();
@@ -3545,5 +3548,49 @@
 "QPassSignals+", "qXfer:auxv:read+", "qXfer:libraries-svr4:read+",
 #endif
   });
+
+  // check for platform features
+  auto process_features = m_process_factory.GetSupportedExtensions();
+
+  // reset to defaults
+  m_fork_events_supported = false;
+  m_vfork_events_supported = false;
+
+  // check for client features
+  for (auto x : client_features) {
+if (x == "fork-events+" &&
+(process_features & NativeProcessProtocol::Extension::fork) ==
+NativeProcessProtocol::Extension::fork)
+  m_fork_events_supported = true;
+else if (x == "vfork-events+" &&
+ (process_features & NativeProcessProtocol::Extension::vfork) ==
+ NativeProcessProtocol::Extension::vfork)
+  m_vfork_events_supported = true;
+  }
+
+  // report only if actually supported
+  if (m_fork_events_supported)
+ret.push_back("fork-events+");
+  if (m_vfork_events_supported)
+ret.push_back("vfork-events+");
+
+  if (m_debugged_process_up)
+SetEnabledExtensions(*m_debugged_process_up);
   return ret;
 }
+
+void GDBRemoteCommunicationServerLLGS::SetEnabledExtensions(
+NativeProcessProtocol ) {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
+
+  NativeProcessProtocol::Extension flags;
+  if (m_fork_events_supported)
+flags |= NativeProcessProtocol::Extension::fork;
+  if (m_vfork_events_supported)
+flags |= NativeProcessProtocol::Extension::vfork;
+
+  llvm::Error error = process.SetEnabledExtensions(flags);
+  if (error)
+LLDB_LOG_ERROR(log, std::move(error),
+   "Enabling protocol extensions failed: {0}");
+}
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -353,7 +353,9 @@
 
   // build the qSupported packet
   std::vector features = {"xmlRegisters=i386,arm,mips,arc",
-   "multiprocess+"};
+   "multiprocess+",
+   "fork-events+",
+   "vfork-events+"};
   StreamString packet;
   packet.PutCString("qSupported");
   for (uint32_t i = 0; i < features.size(); ++i) {

[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

Note that I've included full `fork-events+` and `vfork-events+` as demo of the 
API. I can split them further and/or move to more relevant commits as I 
progress with the split.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100153/new/

https://reviews.llvm.org/D100153

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski.
mgorny requested review of this revision.

Introduce a NativeProcessProtocol API for indicating support for
protocol extensions and enabling them.  LLGS calls
GetSupportedExtensions() method on the process factory to determine
which extensions are supported by the plugin.  If the future is both
supported by the plugin and reported as supported by the client, LLGS
enables it and reports to the client as supported by the server.

The extension is enabled on the process instance by calling
SetEnabledExtensions() method.  This is done after qSupported exchange
(if the debugger is attached to any process), as well as after launching
or attaching to a new inferior.

The patch adds 'fork' extension corresponding to 'fork-events+'
qSupported feature and 'vfork' extension for 'vfork-events+'.


https://reviews.llvm.org/D100153

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -99,6 +99,9 @@
   uint32_t m_next_saved_registers_id = 1;
   bool m_handshake_completed = false;
 
+  bool m_fork_events_supported = false;
+  bool m_vfork_events_supported = false;
+
   PacketResult SendONotification(const char *buffer, uint32_t len);
 
   PacketResult SendWResponse(NativeProcessProtocol *process);
@@ -256,6 +259,9 @@
   llvm::Expected ReadTid(StringExtractorGDBRemote ,
   bool allow_all = false);
 
+  // Call SetEnabledExtensions() with appropriate flags on the process.
+  void SetEnabledExtensions(NativeProcessProtocol& process);
+
   // For GDBRemoteCommunicationServerLLGS only
   GDBRemoteCommunicationServerLLGS(const GDBRemoteCommunicationServerLLGS &) =
   delete;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -251,6 +251,8 @@
 m_debugged_process_up = std::move(*process_or);
   }
 
+  SetEnabledExtensions(*m_debugged_process_up);
+
   // Handle mirroring of inferior stdout/stderr over the gdb-remote protocol as
   // needed. llgs local-process debugging may specify PTY paths, which will
   // make these file actions non-null process launch -i/e/o will also make
@@ -318,6 +320,7 @@
 return status;
   }
   m_debugged_process_up = std::move(*process_or);
+  SetEnabledExtensions(*m_debugged_process_up);
 
   // Setup stdout/stderr mapping from inferior.
   auto terminal_fd = m_debugged_process_up->GetTerminalFileDescriptor();
@@ -3545,5 +3548,45 @@
 "QPassSignals+", "qXfer:auxv:read+", "qXfer:libraries-svr4:read+",
 #endif
   });
+
+  // check for platform features
+  auto process_features = m_process_factory.GetSupportedExtensions();
+
+  // reset to defaults
+  m_fork_events_supported = false;
+  m_vfork_events_supported = false;
+
+  // check for client features
+  for (auto x : client_features) {
+if (x == "fork-events+" && (process_features & NativeProcessProtocol::Extension::fork) == NativeProcessProtocol::Extension::fork)
+  m_fork_events_supported = true;
+else if (x == "vfork-events+" && (process_features & NativeProcessProtocol::Extension::vfork) == NativeProcessProtocol::Extension::vfork)
+  m_vfork_events_supported = true;
+  }
+
+  // report only if actually supported
+  if (m_fork_events_supported)
+ret.push_back("fork-events+");
+  if (m_vfork_events_supported)
+ret.push_back("vfork-events+");
+
+  if (m_debugged_process_up)
+SetEnabledExtensions(*m_debugged_process_up);
   return ret;
 }
+
+void GDBRemoteCommunicationServerLLGS::SetEnabledExtensions(
+NativeProcessProtocol ) {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
+
+  NativeProcessProtocol::Extension flags;
+  if (m_fork_events_supported)
+flags |= NativeProcessProtocol::Extension::fork;
+  if (m_vfork_events_supported)
+flags |= NativeProcessProtocol::Extension::vfork;
+
+  llvm::Error error = process.SetEnabledExtensions(flags);
+  if (error)
+LLDB_LOG_ERROR(log, std::move(error),
+   "Enabling protocol extensions failed: {0}");
+}
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
---