[lldb] Support gdbserver signals

[mgorny]

Summary

GDB and LLDB use different signal models. GDB uses a predefined set
of signal codes, and maps platform's signos to them. On the other hand,
LLDB has historically simply passed native signos.

In order to improve compatibility between LLDB and gdbserver, the GDB
signal model should be used. However, GDB does not provide a mapping
for all existing signals on Linux and unsupported signals are passed
as 'unknown'. Limiting LLDB to this behavior could be considered
a regression.

To get the best of both worlds, use the LLDB signal model when talking
to lldb-server, and the GDB signal model otherwise. For this purpose,
new versions of lldb-server indicate "native-signals+" via qSupported.
At the same time, we also detect older versions of lldb-server
via QThreadSuffixSupported for backwards compatibility. If neither test
succeeds, we assume gdbserver or another implementation using GDB model.

[mgorny]

Added client tests for GDB and LLDB signal variants. Moved signal init into DidLaunchOrAttach, in order to have normalized target triple already (needed for correct os-name in test).

[mgorny]

Included all signals from include/gdb/signals.def.

[JDevlieghere]

LGTM

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

1344

What about debugserver, I assume that would need to be updated as well?

[JDevlieghere]

Didn't mean to accept yet (originally I thought the native-signals check was inverted)

[mgorny]

Advertise native-signals+ in debugserver too.

[mgorny]

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

1344

I've attempted doing that. Please lemme know if that's ok.

[mgorny]

Rebased.

[labath]

It would be nice if the commit message/summary included more details about the differences in signal handling (the things we talked about on irc), but generally, I think this patch looks good.

The only thing that's missing is interoperability with older lldb-compatible stubs (debugservers mainly). I believe we already have some "standard" solution for that, like checking for QThreadSuffixSupported or some similar lldb-only packet (?)

[mgorny]

Add fallback to QThreadSuffixSupported and a respective test. Improve the summary.

[labath]

lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py

486 ↗

(On Diff #385917)

I believe this will always use remote-linux (or something), as the platform is determined by the binary. And even in case it isn't , it would probably be better to hardcode the platform by using the appropriate CreateTarget overload

lldb/source/Plugins/Process/Utility/GDBRemoteSignals.h

 //===-- GDBRemoteSignals.h --------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_GDBREMOTESIGNALS_H
 #define LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_GDBREMOTESIGNALS_H
 
 #include "lldb/Target/UnixSignals.h"
 
 namespace lldb_private {
 
-/// Empty set of Unix signals to be filled by PlatformRemoteGDBServer
+/// Initially carries signals defined by the GDB Remote Serial Protocol.
+/// Can be filled with platform's signals through PlatformRemoteGDBServer.
 class GDBRemoteSignals : public UnixSignals {
 public:
   GDBRemoteSignals();
 
   GDBRemoteSignals(const lldb::UnixSignalsSP &rhs);
 
 private:
   void Reset() override;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_GDBREMOTESIGNALS_H

lldb/source/Plugins/Process/Utility/GDBRemoteSignals.cpp

 
 using namespace lldb_private;
 
 GDBRemoteSignals::GDBRemoteSignals() : UnixSignals() { Reset(); }
 
 GDBRemoteSignals::GDBRemoteSignals(const lldb::UnixSignalsSP &rhs)
     : UnixSignals(*rhs) {}
 
-void GDBRemoteSignals::Reset() { m_signals.clear(); }
+void GDBRemoteSignals::Reset() {
+  m_signals.clear();
+  // clang-format off
+  //        SIGNO   NAME            SUPPRESS  STOP    NOTIFY  DESCRIPTION
+  //        ======  ==============  ========  ======  ======  ===================================================
+  AddSignal(1,      "SIGHUP",       false,    true,   true,   "hangup");
+  AddSignal(2,      "SIGINT",       true,     true,   true,   "interrupt");
+  AddSignal(3,      "SIGQUIT",      false,    true,   true,   "quit");
+  AddSignal(4,      "SIGILL",       false,    true,   true,   "illegal instruction");
+  AddSignal(5,      "SIGTRAP",      true,     true,   true,   "trace trap (not reset when caught)");
+  AddSignal(6,      "SIGABRT",      false,    true,   true,   "abort()/IOT trap", "SIGIOT");
+  AddSignal(7,      "SIGEMT",       false,    true,   true,   "emulation trap");
+  AddSignal(8,      "SIGFPE",       false,    true,   true,   "floating point exception");
+  AddSignal(9,      "SIGKILL",      false,    true,   true,   "kill");
+  AddSignal(10,     "SIGBUS",       false,    true,   true,   "bus error");
+  AddSignal(11,     "SIGSEGV",      false,    true,   true,   "segmentation violation");
+  AddSignal(12,     "SIGSYS",       false,    true,   true,   "invalid system call");
+  AddSignal(13,     "SIGPIPE",      false,    true,   true,   "write to pipe with reading end closed");
+  AddSignal(14,     "SIGALRM",      false,    false,  false,  "alarm");
+  AddSignal(15,     "SIGTERM",      false,    true,   true,   "termination requested");
+  AddSignal(16,     "SIGURG",       false,    true,   true,   "urgent data on socket");
+  AddSignal(17,     "SIGSTOP",      true,     true,   true,   "process stop");
+  AddSignal(18,     "SIGTSTP",      false,    true,   true,   "tty stop");
+  AddSignal(19,     "SIGCONT",      false,    false,  true,   "process continue");
+  AddSignal(20,     "SIGCHLD",      false,    false,  true,   "child status has changed", "SIGCLD");
+  AddSignal(21,     "SIGTTIN",      false,    true,   true,   "background tty read");
+  AddSignal(22,     "SIGTTOU",      false,    true,   true,   "background tty write");
+  AddSignal(23,     "SIGIO",        false,    true,   true,   "input/output ready/Pollable event", "SIGPOLL");
+  AddSignal(24,     "SIGXCPU",      false,    true,   true,   "CPU resource exceeded");
+  AddSignal(25,     "SIGXFSZ",      false,    true,   true,   "file size limit exceeded");
+  AddSignal(26,     "SIGVTALRM",    false,    true,   true,   "virtual time alarm");
+  AddSignal(27,     "SIGPROF",      false,    false,  false,  "profiling time alarm");
+  AddSignal(28,     "SIGWINCH",     false,    true,   true,   "window size changes");
+  AddSignal(29,     "SIGLOST",      false,    true,   true,   "resource lost");
+  AddSignal(30,     "SIGUSR1",      false,    true,   true,   "user defined signal 1");
+  AddSignal(31,     "SIGUSR2",      false,    true,   true,   "user defined signal 2");
+  AddSignal(32,     "SIGPWR",       false,    true,   true,   "power failure");
+  //
+#if 0
+  AddSignal(34,     "SIGRTMIN",     false,    false,  false,  "real time signal 0");
+  AddSignal(35,     "SIGRTMIN+1",   false,    false,  false,  "real time signal 1");
+  AddSignal(36,     "SIGRTMIN+2",   false,    false,  false,  "real time signal 2");
+  AddSignal(37,     "SIGRTMIN+3",   false,    false,  false,  "real time signal 3");
+  AddSignal(38,     "SIGRTMIN+4",   false,    false,  false,  "real time signal 4");
+  AddSignal(39,     "SIGRTMIN+5",   false,    false,  false,  "real time signal 5");
+  AddSignal(40,     "SIGRTMIN+6",   false,    false,  false,  "real time signal 6");
+  AddSignal(41,     "SIGRTMIN+7",   false,    false,  false,  "real time signal 7");
+  AddSignal(42,     "SIGRTMIN+8",   false,    false,  false,  "real time signal 8");
+  AddSignal(43,     "SIGRTMIN+9",   false,    false,  false,  "real time signal 9");
+  AddSignal(44,     "SIGRTMIN+10",  false,    false,  false,  "real time signal 10");
+  AddSignal(45,     "SIGRTMIN+11",  false,    false,  false,  "real time signal 11");
+  AddSignal(46,     "SIGRTMIN+12",  false,    false,  false,  "real time signal 12");
+  AddSignal(47,     "SIGRTMIN+13",  false,    false,  false,  "real time signal 13");
+  AddSignal(48,     "SIGRTMIN+14",  false,    false,  false,  "real time signal 14");
+  AddSignal(49,     "SIGRTMIN+15",  false,    false,  false,  "real time signal 15");
+  AddSignal(50,     "SIGRTMAX-14",  false,    false,  false,  "real time signal 16"); // switching to SIGRTMAX-xxx to match "kill -l" output
+  AddSignal(51,     "SIGRTMAX-13",  false,    false,  false,  "real time signal 17");
+  AddSignal(52,     "SIGRTMAX-12",  false,    false,  false,  "real time signal 18");
+  AddSignal(53,     "SIGRTMAX-11",  false,    false,  false,  "real time signal 19");
+  AddSignal(54,     "SIGRTMAX-10",  false,    false,  false,  "real time signal 20");
+  AddSignal(55,     "SIGRTMAX-9",   false,    false,  false,  "real time signal 21");
+  AddSignal(56,     "SIGRTMAX-8",   false,    false,  false,  "real time signal 22");
+  AddSignal(57,     "SIGRTMAX-7",   false,    false,  false,  "real time signal 23");
+  AddSignal(58,     "SIGRTMAX-6",   false,    false,  false,  "real time signal 24");
+  AddSignal(59,     "SIGRTMAX-5",   false,    false,  false,  "real time signal 25");
+  AddSignal(60,     "SIGRTMAX-4",   false,    false,  false,  "real time signal 26");
+  AddSignal(61,     "SIGRTMAX-3",   false,    false,  false,  "real time signal 27");
+  AddSignal(62,     "SIGRTMAX-2",   false,    false,  false,  "real time signal 28");
+  AddSignal(63,     "SIGRTMAX-1",   false,    false,  false,  "real time signal 29");
+  AddSignal(64,     "SIGRTMAX",     false,    false,  false,  "real time signal 30");
+#endif
+  // clang-format on
+}

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

public:
   bool GetThreadExtendedInfoSupported();
 
   bool GetLoadedDynamicLibrariesInfosSupported();
 
   bool GetSharedCacheInfoSupported();
 
   bool GetMemoryTaggingSupported();
 
+  bool UsesNativeSignals();
+
   lldb::DataBufferSP ReadMemoryTags(lldb::addr_t addr, size_t len,
                                     int32_t type);
 
   Status WriteMemoryTags(lldb::addr_t addr, size_t len, int32_t type,
                          const std::vector<uint8_t> &tags);
 
   /// Use qOffsets to query the offset used when relocating the target
   /// executable. If successful, the returned structure will contain at least
protected:
   LazyBool m_supports_augmented_libraries_svr4_read = eLazyBoolCalculate;
   LazyBool m_supports_jThreadExtendedInfo = eLazyBoolCalculate;
   LazyBool m_supports_jLoadedDynamicLibrariesInfos = eLazyBoolCalculate;
   LazyBool m_supports_jGetSharedCacheInfo = eLazyBoolCalculate;
   LazyBool m_supports_QPassSignals = eLazyBoolCalculate;
   LazyBool m_supports_error_string_reply = eLazyBoolCalculate;
   LazyBool m_supports_multiprocess = eLazyBoolCalculate;
   LazyBool m_supports_memory_tagging = eLazyBoolCalculate;
+  LazyBool m_uses_native_signals = eLazyBoolCalculate;
 
   bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1,
       m_supports_qUserName : 1, m_supports_qGroupName : 1,
       m_supports_qThreadStopInfo : 1, m_supports_z0 : 1, m_supports_z1 : 1,
       m_supports_z2 : 1, m_supports_z3 : 1, m_supports_z4 : 1,
       m_supports_QEnvironment : 1, m_supports_QEnvironmentHexEncoded : 1,
       m_supports_qSymbol : 1, m_qSymbol_requests_done : 1,
       m_supports_qModuleInfo : 1, m_supports_jThreadsInfo : 1,

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

if (!did_exec) {
     m_avoid_g_packets = eLazyBoolCalculate;
     m_supports_multiprocess = eLazyBoolCalculate;
     m_supports_qXfer_auxv_read = eLazyBoolCalculate;
     m_supports_qXfer_libraries_read = eLazyBoolCalculate;
     m_supports_qXfer_libraries_svr4_read = eLazyBoolCalculate;
     m_supports_qXfer_features_read = eLazyBoolCalculate;
     m_supports_qXfer_memory_map_read = eLazyBoolCalculate;
     m_supports_augmented_libraries_svr4_read = eLazyBoolCalculate;
+    m_uses_native_signals = eLazyBoolCalculate;
     m_supports_qProcessInfoPID = true;
     m_supports_qfProcessInfo = true;
     m_supports_qUserName = true;
     m_supports_qGroupName = true;
     m_supports_qThreadStopInfo = true;
     m_supports_z0 = true;
     m_supports_z1 = true;
     m_supports_z2 = true;
void GDBRemoteCommunicationClient::GetRemoteQSupported() {
   m_supports_qXfer_libraries_svr4_read = eLazyBoolNo;
   m_supports_augmented_libraries_svr4_read = eLazyBoolNo;
   m_supports_qXfer_features_read = eLazyBoolNo;
   m_supports_qXfer_memory_map_read = eLazyBoolNo;
   m_supports_multiprocess = eLazyBoolNo;
   m_supports_qEcho = eLazyBoolNo;
   m_supports_QPassSignals = eLazyBoolNo;
   m_supports_memory_tagging = eLazyBoolNo;
+  m_uses_native_signals = eLazyBoolNo;
 
   m_max_packet_size = UINT64_MAX; // It's supposed to always be there, but if
                                   // not, we assume no limit
 
   // build the qSupported packet
   std::vector<std::string> features = {"xmlRegisters=i386,arm,mips,arc",
                                        "multiprocess+"};
   StreamString packet;
for (llvm::StringRef x : server_features) {
       else if (x == "qEcho")
         m_supports_qEcho = eLazyBoolYes;
       else if (x == "QPassSignals+")
         m_supports_QPassSignals = eLazyBoolYes;
       else if (x == "multiprocess+")
         m_supports_multiprocess = eLazyBoolYes;
       else if (x == "memory-tagging+")
         m_supports_memory_tagging = eLazyBoolYes;
+      else if (x == "native-signals+")
+        m_uses_native_signals = eLazyBoolYes;
       // Look for a list of compressions in the features list e.g.
       // qXfer:features:read+;PacketSize=20000;qEcho+;SupportedCompressions=zlib-
       // deflate,lzma
       else if (x.consume_front("SupportedCompressions=")) {
         llvm::SmallVector<llvm::StringRef, 4> compressions;
         x.split(compressions, ',');
         if (!compressions.empty())
           MaybeEnableCompression(compressions);
Status GDBRemoteCommunicationClient::ConfigureRemoteStructuredData(
   }
   return error;
 }
 
 void GDBRemoteCommunicationClient::OnRunPacketSent(bool first) {
   GDBRemoteClientBase::OnRunPacketSent(first);
   m_curr_tid = LLDB_INVALID_THREAD_ID;
 }
+
+bool GDBRemoteCommunicationClient::UsesNativeSignals() {
+  if (m_uses_native_signals == eLazyBoolCalculate) {
+    GetRemoteQSupported();
+  }
+  return m_uses_native_signals == eLazyBoolYes;
+}

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

std::vector<std::string> GDBRemoteCommunicationServerCommon::HandleFeatures(
   // 128KBytes is a reasonable max packet size--debugger can always use less.
   constexpr uint32_t max_packet_size = 128 * 1024;
 
   // Features common to platform server and llgs.
   return {
       llvm::formatv("PacketSize={0}", max_packet_size),
       "QStartNoAckMode+",
       "qEcho+",
+      "native-signals+",
   };
 }

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

 //===-- ProcessGDBRemote.cpp ----------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Host/Config.h"
 
 #include <cerrno>
 #include <cstdlib>
 #if LLDB_ENABLE_POSIX
 #include <netinet/in.h>
 #include <sys/mman.h>
 #include <sys/socket.h>
 #include <unistd.h>
Status ProcessGDBRemote::DoConnectRemote(llvm::StringRef remote_url) {
 
   LLDB_LOGF(log,
             "ProcessGDBRemote::%s pid %" PRIu64
             ": normalized target architecture triple: %s",
             __FUNCTION__, GetID(),
             GetTarget().GetArchitecture().GetTriple().getTriple().c_str());
 
   if (error.Success()) {
+    if (!m_gdb_comm.UsesNativeSignals()) {
+      Process::SetUnixSignals(std::make_shared<GDBRemoteSignals>());
+    } else {
       PlatformSP platform_sp = GetTarget().GetPlatform();
       if (platform_sp && platform_sp->IsConnected())
         SetUnixSignals(platform_sp->GetUnixSignals());
       else
         SetUnixSignals(UnixSignals::Create(GetTarget().GetArchitecture()));
     }
+  }
 
   return error;
 }
 
 Status ProcessGDBRemote::WillLaunchOrAttach() {
   Status error;
   m_stdio_communication.Clear();
   return error;