[lldb] [llgs] Report QPassSignals and qXfer via extensions API

[mgorny]

Summary

Remove hardcoded platform list for QPassSignals, qXfer:auxv:read
and qXfer:libraries-svr4:read and instead query the process plugin
via the GetSupportedExtensions() API.

[labath]

I like this, though I am still unsure about the reason for existence of the "multiprocess" extension.

lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp

287

Technically, libraries_svr4 functionality is implemented in the base class, so it would be more correct to implement this as | NativeProcessELF::Factory::GetSupportedExtensions() (this would be a new class, which exists only to override GetSupportedExtensions).

lldb/test/API/tools/lldb-server/TestLldbGdbServer.py

977

I guess it would be even better to run this everywhere, but assert that the server returns the right value for the particular platform.

[mgorny]

lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp

287

Doesn't it require some extra logic in the NativeProcess* class though, like memory maps or something like that? If not, I sure can move it, I guess.

[labath]

lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp

287

Hmm.. that's a good question. Fortunately, we have a unit test for this class, so it should be easy to answer it. It seems it does require reading an auxiliary vector (and memory). However, I don't think that disqualifies it from this (svr4 support is the only reason for this class existence -- if a plugin did not support auxv reading, it would not inherit from it). OTOH, I can sort of see how it makes sense to implement this here, so I'm not going to push for that..

[mgorny]

Thanks for the review. I'm going to update the tests as you suggested before pushing.

lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp

287

Now that I think of it, I'm pretty sure Kamil told me once that NetBSD used not to support SVR4 libs properly (dyld stuff, I guess). I suppose it makes more sense to enable it per platform.

lldb/include/lldb/Host/common/NativeProcessProtocol.h

virtual Status GetFileLoadAddress(const llvm::StringRef &file_name,
                                     lldb::addr_t &load_addr) = 0;
 
   /// Extension flag constants, returned by Factory::GetSupportedExtensions()
   /// and passed to SetEnabledExtension()
   enum class Extension {
     multiprocess = (1u << 0),
     fork = (1u << 1),
     vfork = (1u << 2),
+    pass_signals = (1u << 3),
+    auxv = (1u << 4),
+    libraries_svr4 = (1u << 5),
 
-    LLVM_MARK_AS_BITMASK_ENUM(vfork)
+    LLVM_MARK_AS_BITMASK_ENUM(libraries_svr4)
   };
 
   class Factory {
   public:
     virtual ~Factory();
     /// Launch a process for debugging.
     ///
     /// \param[in] launch_info

lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp

NativeProcessFreeBSD::Factory::Attach(
   if (!status.Success())
     return status.ToError();
 
   return std::move(process_up);
 }
 
 NativeProcessFreeBSD::Extension
 NativeProcessFreeBSD::Factory::GetSupportedExtensions() const {
-  return Extension::multiprocess | Extension::fork | Extension::vfork;
+  return Extension::multiprocess | Extension::fork | Extension::vfork |
+         Extension::pass_signals | Extension::auxv | Extension::libraries_svr4;
 }
 
 // Public Instance Methods
 
 NativeProcessFreeBSD::NativeProcessFreeBSD(::pid_t pid, int terminal_fd,
                                            NativeDelegate &delegate,
                                            const ArchSpec &arch,
                                            MainLoop &mainloop)

lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp

if (!tids_or)
     return tids_or.takeError();
 
   return std::unique_ptr<NativeProcessLinux>(new NativeProcessLinux(
       pid, -1, native_delegate, Info.GetArchitecture(), mainloop, *tids_or));
 }
 
 NativeProcessLinux::Extension
 NativeProcessLinux::Factory::GetSupportedExtensions() const {
-  return Extension::multiprocess | Extension::fork | Extension::vfork;
+  return Extension::multiprocess | Extension::fork | Extension::vfork |
+         Extension::pass_signals | Extension::auxv | Extension::libraries_svr4;
 }
 
 // Public Instance Methods
 
 NativeProcessLinux::NativeProcessLinux(::pid_t pid, int terminal_fd,
                                        NativeDelegate &delegate,
                                        const ArchSpec &arch, MainLoop &mainloop,
                                        llvm::ArrayRef<::pid_t> tids)

lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp

NativeProcessNetBSD::Factory::Attach(
   if (!status.Success())
     return status.ToError();
 
   return std::move(process_up);
 }
 
 NativeProcessNetBSD::Extension
 NativeProcessNetBSD::Factory::GetSupportedExtensions() const {
-  return Extension::multiprocess | Extension::fork | Extension::vfork;
+  return Extension::multiprocess | Extension::fork | Extension::vfork |
+         Extension::pass_signals | Extension::auxv | Extension::libraries_svr4;
 }
 
 // Public Instance Methods
 
 NativeProcessNetBSD::NativeProcessNetBSD(::pid_t pid, int terminal_fd,
                                          NativeDelegate &delegate,
                                          const ArchSpec &arch,
                                          MainLoop &mainloop)

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

llvm::Expected<lldb::tid_t> GDBRemoteCommunicationServerLLGS::ReadTid(
   return tid;
 }
 
 std::vector<std::string> GDBRemoteCommunicationServerLLGS::HandleFeatures(
     const llvm::ArrayRef<llvm::StringRef> client_features) {
   std::vector<std::string> ret =
       GDBRemoteCommunicationServerCommon::HandleFeatures(client_features);
   ret.insert(ret.end(), {
-    "QThreadSuffixSupported+", "QListThreadsInStopReply+",
+                            "QThreadSuffixSupported+",
+                            "QListThreadsInStopReply+",
                             "qXfer:features:read+",
-#if defined(__linux__) || defined(__NetBSD__) || defined(__FreeBSD__)
-        "QPassSignals+", "qXfer:auxv:read+", "qXfer:libraries-svr4:read+",
-#endif
                         });
 
-  // check for client features
+  // report server-only features
   using Extension = NativeProcessProtocol::Extension;
+  Extension plugin_features = m_process_factory.GetSupportedExtensions();
+  if (bool(plugin_features & Extension::pass_signals))
+    ret.push_back("QPassSignals+");
+  if (bool(plugin_features & Extension::auxv))
+    ret.push_back("qXfer:auxv:read+");
+  if (bool(plugin_features & Extension::libraries_svr4))
+    ret.push_back("qXfer:libraries-svr4:read+");
+
+  // check for client features
   m_extensions_supported = {};
   for (llvm::StringRef x : client_features)
     m_extensions_supported |=
         llvm::StringSwitch<Extension>(x)
             .Case("multiprocess+", Extension::multiprocess)
             .Case("fork-events+", Extension::fork)
             .Case("vfork-events+", Extension::vfork)
             .Default({});
-  m_extensions_supported &= m_process_factory.GetSupportedExtensions();
+
+  m_extensions_supported &= plugin_features;
 
   // fork & vfork require multiprocess
   if (!bool(m_extensions_supported & Extension::multiprocess))
     m_extensions_supported &= ~(Extension::fork | Extension::vfork);
 
   // report only if actually supported
   if (bool(m_extensions_supported & Extension::multiprocess))
     ret.push_back("multiprocess+");

lldb/test/API/tools/lldb-server/TestLldbGdbServer.py

 
 import unittest2
 import gdbremote_testcase
 import lldbgdbserverutils
 from lldbsuite.support import seven
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test.lldbdwarf import *
-from lldbsuite.test import lldbutil
+from lldbsuite.test import lldbutil, lldbplatformutil
 
 
 class LldbGdbServerTestCase(gdbremote_testcase.GdbRemoteTestCaseBase, DwarfOpcodeParser):
 
     mydir = TestBase.compute_mydir(__file__)
 
     def test_thread_suffix_supported(self):
         server = self.connect_to_debug_monitor()
def get_qSupported_dict(self, features=[]):
         # Retrieve the qSupported features.
         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)
 
+    def test_qSupported_auvx(self):
+        expected = ('+' if lldbplatformutil.getPlatform()
+                    in ["freebsd", "linux", "netbsd"] else '-')
+        supported_dict = self.get_qSupported_dict()
+        self.assertEqual(supported_dict.get('qXfer:auxv:read', '-'), expected)
+
+    def test_qSupported_libraries_svr4(self):
+        expected = ('+' if lldbplatformutil.getPlatform()
+                    in ["freebsd", "linux", "netbsd"] else '-')
+        supported_dict = self.get_qSupported_dict()
+        self.assertEqual(supported_dict.get('qXfer:libraries-svr4:read', '-'),
+                         expected)
+
+    def test_qSupported_QPassSignals(self):
+        expected = ('+' if lldbplatformutil.getPlatform()
+                    in ["freebsd", "linux", "netbsd"] else '-')
+        supported_dict = self.get_qSupported_dict()
+        self.assertEqual(supported_dict.get('QPassSignals', '-'), expected)
+
     @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', '-'), '-')
 

lldb/test/API/tools/lldb-server/signal-filtering/TestGdbRemote_QPassSignals.py

def test_default_signals_behavior(self):
         self.set_inferior_startup_launch()
         procs = self.prep_debug_monitor_and_inferior()
         expected_signals = ["SIGSEGV", "SIGUSR1", "SIGUSR2",
             "SIGALRM", "SIGFPE", "SIGBUS", "SIGINT", "SIGHUP"]
         for signal_name in expected_signals:
             signo = lldbutil.get_signal_number(signal_name)
             self.expect_signal(signo)
         self.expect_exit_code(0)
-
-
-    @skipUnlessPlatform(["linux", "android"])
-    def test_support_q_pass_signals(self):
-        self.build()
-
-        # Start up the stub and start/prep the inferior.
-        self.set_inferior_startup_launch()
-        procs = self.prep_debug_monitor_and_inferior()
-        self.add_qSupported_packets()
-
-        # Run the packet stream.
-        context = self.expect_gdbremote_sequence()
-        self.assertIsNotNone(context)
-
-        # Retrieve the qSupported features and check QPassSignals+
-        supported_dict = self.parse_qSupported_response(context)
-        self.assertEqual(supported_dict["QPassSignals"], "+")