Implement QPassSignals GDB package in lldb-server

[eugene]

Summary

QPassSignals package allows lldb client to tell lldb-server to ignore certain types of signals and re-inject them back to inferior without stopping execution.

[labath]

Good stuff. We just need a couple of changes to make this conform better with llvm guidelines.

include/lldb/Host/common/NativeProcessProtocol.h

73

llvm::ArrayRef<int> signals

321

use of unordered_set is discouraged in llvm http://llvm.org/docs/ProgrammersManual.html#set-like-containers-std-set-smallset-setvector-etc. It sounds like this would be a good use-case for a llvm::DenseSet or potentially SmallSet.

packages/Python/lldbsuite/test/tools/lldb-server/signal-filtering/Makefile

4

I am pretty sure the CFLAGS_EXTRAS line is not necessary here.

packages/Python/lldbsuite/test/tools/lldb-server/signal-filtering/TestGdbRemote_QPassSignals.py

74

I really like how these tests are written. May suggest one more improvement:

• have the inferior count how many signals it received and then return that number from main()
• in the test you can then check the exit code matches the number of signals you are expecting

This should verify that we actually pass the signals and not just swallow them silently.

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

3097

This is not a big deal, but in LLVM we generally don't put braces around blocks that fit on a single line.

[eugene]

Addressing code review comments.

[labath]

Looks great. Thank you.

include/lldb/Host/common/NativeProcessProtocol.h

 
 #include <mutex>
 #include <vector>
 
 #include "lldb/Host/MainLoop.h"
 #include "lldb/Utility/Error.h"
 #include "lldb/lldb-private-forward.h"
 #include "lldb/lldb-types.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/StringRef.h"
 
 #include "NativeBreakpointList.h"
 #include "NativeWatchpointList.h"
 
 namespace lldb_private {
 class MemoryRegionInfo;
 class ResumeActionList;
public:
   ///
   /// @return
   ///     Returns an error object.
   //------------------------------------------------------------------
   virtual Error Interrupt();
 
   virtual Error Kill() = 0;
 
+  //------------------------------------------------------------------
+  // Tells a process not to stop the inferior on given signals
+  // and just reinject them back.
+  //------------------------------------------------------------------
+  virtual Error IgnoreSignals(llvm::ArrayRef<int> signals);
+
   //----------------------------------------------------------------------
   // Memory and memory region functions
   //----------------------------------------------------------------------
 
   virtual Error GetMemoryRegionInfo(lldb::addr_t load_addr,
                                     MemoryRegionInfo &range_info);
 
   virtual Error ReadMemory(lldb::addr_t addr, void *buf, size_t size,
protected:
   std::string m_exit_description;
   std::recursive_mutex m_delegates_mutex;
   std::vector<NativeDelegate *> m_delegates;
   NativeBreakpointList m_breakpoint_list;
   NativeWatchpointList m_watchpoint_list;
   int m_terminal_fd;
   uint32_t m_stop_id;
 
+  // Set of signal numbers that LLDB directly injects back to inferior
+  // without stopping it.
+  llvm::DenseSet<int> m_signals_to_ignore;
+
   // lldb_private::Host calls should be used to launch a process for debugging,
   // and
   // then the process should be attached to. When attaching to a process
   // lldb_private::Host calls should be used to locate the process to attach to,
   // and then this function should be called.
   NativeProcessProtocol(lldb::pid_t pid);
 
   // -----------------------------------------------------------

packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py

_KNOWN_QSUPPORTED_STUB_FEATURES = [
         "PacketSize",
         "QStartNoAckMode",
         "QThreadSuffixSupported",
         "QListThreadsInStopReply",
         "qXfer:auxv:read",
         "qXfer:libraries:read",
         "qXfer:libraries-svr4:read",
         "qXfer:features:read",
-        "qEcho"
+        "qEcho",
+        "QPassSignals"
     ]
 
     def parse_qSupported_response(self, context):
         self.assertIsNotNone(context)
 
         raw_response = context.get("qSupported_response")
         self.assertIsNotNone(raw_response)
 

packages/Python/lldbsuite/test/tools/lldb-server/signal-filtering/Makefile

+LEVEL = ../../../make
+
+CXX_SOURCES := main.cpp
+
+include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/tools/lldb-server/signal-filtering/TestGdbRemote_QPassSignals.py

+from __future__ import print_function
+
+import gdbremote_testcase
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestGdbRemote_QPassSignals(gdbremote_testcase.GdbRemoteTestCaseBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def expect_signal(self, expected_signo):
+        self.test_sequence.add_log_lines(["read packet: $vCont;c#a8",
+                                          {"direction": "send",
+                                           "regex": r"^\$T([0-9a-fA-F]{2}).*#[0-9a-fA-F]{2}$",
+                                           "capture": {1: "hex_exit_code"}},
+                                          ],
+                                         True)
+
+        context = self.expect_gdbremote_sequence()
+        self.assertIsNotNone(context)
+
+        hex_exit_code = context.get("hex_exit_code")
+        self.assertIsNotNone(hex_exit_code)
+        self.assertEqual(int(hex_exit_code, 16), expected_signo)
+
+    def expect_exit_code(self, exit_code):
+        self.test_sequence.add_log_lines(
+            ["read packet: $vCont;c#a8",
+             "send packet: $W{0:02x}#00".format(exit_code)],
+            True)
+        self.expect_gdbremote_sequence()
+
+
+    def ignore_signals(self, signals):
+        def signal_name_to_hex(signame):
+            return format(lldbutil.get_signal_number(signame), 'x')
+        signals_str = ";".join(map(signal_name_to_hex, signals))
+
+        self.test_sequence.add_log_lines(["read packet: $QPassSignals:"
+                                          + signals_str + " #00",
+                                          "send packet: $OK#00"],
+                                         True)
+
+        context = self.expect_gdbremote_sequence()
+        self.assertIsNotNone(context)
+
+    @llgs_test
+    @skipUnlessPlatform(["linux", "android"])
+    def test_q_pass_signals(self):
+        self.init_llgs_test()
+        self.build()
+        self.set_inferior_startup_launch()
+        procs = self.prep_debug_monitor_and_inferior()
+        expected_signals = ["SIGSEGV",
+            "SIGALRM", "SIGFPE", "SIGBUS", "SIGINT", "SIGHUP"]
+        signals_to_ignore = ["SIGUSR1", "SIGUSR2"]
+        self.ignore_signals(signals_to_ignore)
+        for signal_name in expected_signals:
+            signo = lldbutil.get_signal_number(signal_name)
+            self.expect_signal(signo)
+        self.expect_exit_code(len(signals_to_ignore))
+
+    @llgs_test
+    @skipUnlessPlatform(["linux", "android"])
+    def test_change_signals_at_runtime(self):
+        self.init_llgs_test()
+        self.build()
+        self.set_inferior_startup_launch()
+        procs = self.prep_debug_monitor_and_inferior()
+        expected_signals = ["SIGSEGV", "SIGUSR1", "SIGUSR2",
+            "SIGALRM", "SIGHUP"]
+        signals_to_ignore = ["SIGFPE", "SIGBUS", "SIGINT"]
+
+        for signal_name in expected_signals:
+            signo = lldbutil.get_signal_number(signal_name)
+            self.expect_signal(signo)
+            if signal_name == "SIGALRM":
+                self.ignore_signals(signals_to_ignore)
+        self.expect_exit_code(len(signals_to_ignore))
+
+    @llgs_test
+    def test_default_signals_behavior(self):
+        self.init_llgs_test()
+        self.build()
+        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)
+
+
+    @llgs_test
+    @skipUnlessPlatform(["linux", "android"])
+    def test_support_q_pass_signals(self):
+        self.init_llgs_test()
+        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"], "+")

packages/Python/lldbsuite/test/tools/lldb-server/signal-filtering/main.cpp

+//===-- main.cpp ------------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include <signal.h>
+#include <stdio.h>
+#include <vector>
+
+static int signal_counter = 0;
+
+static void count_signal(int signo) {
+  ++signal_counter;
+  printf("Signal %d\n", signo);
+}
+
+static void raise_signals() {
+  std::vector<int> signals(
+      {SIGSEGV, SIGUSR1, SIGUSR2, SIGALRM, SIGFPE, SIGBUS, SIGINT, SIGHUP});
+
+  for (int signal_num : signals) {
+    signal(signal_num, count_signal);
+  }
+
+  for (int signal_num : signals) {
+    raise(signal_num);
+  }
+}
+
+int main() {
+  raise_signals();
+  return signal_counter;
+}

source/Host/common/NativeProcessProtocol.cpp

 #if !defined(SIGSTOP)
   error.SetErrorString("local host does not support signaling");
   return error;
 #else
   return Signal(SIGSTOP);
 #endif
 }
 
+Error NativeProcessProtocol::IgnoreSignals(llvm::ArrayRef<int> signals) {
+  m_signals_to_ignore.clear();
+  m_signals_to_ignore.insert(signals.begin(), signals.end());
+  return Error();
+}
+
 lldb_private::Error
 NativeProcessProtocol::GetMemoryRegionInfo(lldb::addr_t load_addr,
                                            MemoryRegionInfo &range_info) {
   // Default: not implemented.
   return Error("not implemented");
 }
 
 bool NativeProcessProtocol::GetExitStatus(ExitType *exit_type, int *status,

source/Plugins/Process/Linux/NativeProcessLinux.cpp

if (!StateIsStoppedState(thread_state, false)) {
                GetID(), thread.GetID(), thread_state);
       SignalIfAllThreadsStopped();
     }
 
     // Done handling.
     return;
   }
 
+  // Check if debugger should stop at this signal or just ignore it
+  // and resume the inferior.
+  if (m_signals_to_ignore.find(signo) != m_signals_to_ignore.end()) {
+     ResumeThread(thread, thread.GetState(), signo);
+     return;
+  }
+
   // This thread is stopped.
   LLDB_LOG(log, "received signal {0}", Host::GetSignalAsCString(signo));
   thread.SetStoppedBySignal(signo, &info);
 
   // Send a stop to the debugger after we get all other threads to stop.
   StopRunningThreads(thread.GetID());
 }
 

source/Plugins/Process/POSIX/CrashReason.cpp

case SEGV_ACCERR:
     return CrashReason::ePrivilegedAddress;
 #ifndef SEGV_BNDERR
 #define SEGV_BNDERR 3
 #endif
   case SEGV_BNDERR:
     return CrashReason::eBoundViolation;
   }
 
-  assert(false && "unexpected si_code for SIGSEGV");
   return CrashReason::eInvalidCrashReason;
 }
 
 CrashReason GetCrashReasonForSIGILL(const siginfo_t &info) {
   assert(info.si_signo == SIGILL);
 
   switch (info.si_code) {
   case ILL_ILLOPC:
CrashReason GetCrashReasonForSIGILL(const siginfo_t &info) {
   case ILL_PRVREG:
     return CrashReason::ePrivilegedRegister;
   case ILL_COPROC:
     return CrashReason::eCoprocessorError;
   case ILL_BADSTK:
     return CrashReason::eInternalStackError;
   }
 
-  assert(false && "unexpected si_code for SIGILL");
   return CrashReason::eInvalidCrashReason;
 }
 
 CrashReason GetCrashReasonForSIGFPE(const siginfo_t &info) {
   assert(info.si_signo == SIGFPE);
 
   switch (info.si_code) {
   case FPE_INTDIV:
CrashReason GetCrashReasonForSIGFPE(const siginfo_t &info) {
   case FPE_FLTRES:
     return CrashReason::eFloatInexactResult;
   case FPE_FLTINV:
     return CrashReason::eFloatInvalidOperation;
   case FPE_FLTSUB:
     return CrashReason::eFloatSubscriptRange;
   }
 
-  assert(false && "unexpected si_code for SIGFPE");
   return CrashReason::eInvalidCrashReason;
 }
 
 CrashReason GetCrashReasonForSIGBUS(const siginfo_t &info) {
   assert(info.si_signo == SIGBUS);
 
   switch (info.si_code) {
   case BUS_ADRALN:
     return CrashReason::eIllegalAlignment;
   case BUS_ADRERR:
     return CrashReason::eIllegalAddress;
   case BUS_OBJERR:
     return CrashReason::eHardwareError;
   }
 
-  assert(false && "unexpected si_code for SIGBUS");
   return CrashReason::eInvalidCrashReason;
 }
 }
 
 std::string GetCrashReasonString(CrashReason reason, const siginfo_t &info) {
   std::string str;
 
 // make sure that siginfo_t has the bound fields available.
return GetCrashReasonString(reason,
                               reinterpret_cast<lldb::addr_t>(info.si_addr));
 }
 
 std::string GetCrashReasonString(CrashReason reason, lldb::addr_t fault_addr) {
   std::string str;
 
   switch (reason) {
   default:
-    assert(false && "invalid CrashReason");
+    str = "unknown crash reason";
     break;
 
   case CrashReason::eInvalidAddress:
     str = "signal SIGSEGV: invalid address";
     AppendFaultAddr(str, fault_addr);
     break;
   case CrashReason::ePrivilegedAddress:
     str = "signal SIGSEGV: address access protected";

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

uint32_t max_packet_size = 128 * 1024; // 128KBytes is a reasonable max packet
                                          // size--debugger can always use less
   response.Printf("PacketSize=%x", max_packet_size);
 
   response.PutCString(";QStartNoAckMode+");
   response.PutCString(";QThreadSuffixSupported+");
   response.PutCString(";QListThreadsInStopReply+");
   response.PutCString(";qEcho+");
 #if defined(__linux__)
+  response.PutCString(";QPassSignals+");
   response.PutCString(";qXfer:auxv:read+");
 #endif
 
   return SendPacketNoLock(response.GetString());
 }
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_QThreadSuffixSupported(

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

protected:
   PacketResult Handle_qThreadStopInfo(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_jThreadsInfo(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qWatchpointSupportInfo(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qFileLoadAddress(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_QPassSignals(StringExtractorGDBRemote &packet);
+
   void SetCurrentThreadID(lldb::tid_t tid);
 
   lldb::tid_t GetCurrentThreadID() const;
 
   void SetContinueThreadID(lldb::tid_t tid);
 
   lldb::tid_t GetContinueThreadID() const { return m_continue_tid; }
 

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

RegisterMemberFunctionHandler(
       &GDBRemoteCommunicationServerLLGS::Handle_vCont_actions);
   RegisterMemberFunctionHandler(
       StringExtractorGDBRemote::eServerPacketType_x,
       &GDBRemoteCommunicationServerLLGS::Handle_memory_read);
   RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_Z,
                                 &GDBRemoteCommunicationServerLLGS::Handle_Z);
   RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_z,
                                 &GDBRemoteCommunicationServerLLGS::Handle_z);
+  RegisterMemberFunctionHandler(
+      StringExtractorGDBRemote::eServerPacketType_QPassSignals,
+      &GDBRemoteCommunicationServerLLGS::Handle_QPassSignals);
 
   RegisterPacketHandler(StringExtractorGDBRemote::eServerPacketType_k,
                         [this](StringExtractorGDBRemote packet, Error &error,
                                bool &interrupt, bool &quit) {
                           quit = true;
                           return this->Handle_k(packet);
                         });
 }
GDBRemoteCommunicationServerLLGS::Handle_qFileLoadAddress(
   if (file_load_address == LLDB_INVALID_ADDRESS)
     return SendErrorResponse(1); // File not loaded
 
   StreamGDBRemote response;
   response.PutHex64(file_load_address);
   return SendPacketNoLock(response.GetString());
 }
 
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_QPassSignals(
+    StringExtractorGDBRemote &packet) {
+  std::vector<int> signals;
+  packet.SetFilePos(strlen("QPassSignals:"));
+
+  // Read sequence of hex signal numbers divided by a semicolon and
+  // optionally spaces.
+  while (packet.GetBytesLeft() > 0) {
+    int signal = packet.GetS32(-1, 16);
+    if (signal < 0)
+      return SendIllFormedResponse(packet, "Failed to parse signal number.");
+    signals.push_back(signal);
+
+    packet.SkipSpaces();
+    char separator = packet.GetChar();
+    if (separator == '\0')
+      break; // End of string
+    if (separator != ';')
+      return SendIllFormedResponse(packet, "Invalid separator,"
+                                            " expected semicolon.");
+  }
+
+  // Fail if we don't have a current process.
+  if (!m_debugged_process_sp)
+    return SendErrorResponse(68);
+
+  Error error = m_debugged_process_sp->IgnoreSignals(signals);
+  if (error.Fail())
+    return SendErrorResponse(69);
+
+  return SendOKResponse();
+}
+
 void GDBRemoteCommunicationServerLLGS::MaybeCloseInferiorTerminalConnection() {
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
 
   // Tell the stdio connection to shut down.
   if (m_stdio_communication.IsConnected()) {
     auto connection = m_stdio_communication.GetConnection();
     if (connection) {
       Error error;

source/Utility/StringExtractorGDBRemote.h

enum ServerPacketType {
     eServerPacketType_vFile_exists,
     eServerPacketType_vFile_md5,
     eServerPacketType_vFile_stat,
     eServerPacketType_vFile_symlink,
     eServerPacketType_vFile_unlink,
     // debug server packages
     eServerPacketType_QEnvironmentHexEncoded,
     eServerPacketType_QListThreadsInStopReply,
+    eServerPacketType_QPassSignals,
     eServerPacketType_QRestoreRegisterState,
     eServerPacketType_QSaveRegisterState,
     eServerPacketType_QSetLogging,
     eServerPacketType_QSetMaxPacketSize,
     eServerPacketType_QSetMaxPayloadSize,
     eServerPacketType_QSetEnableAsyncProfiling,
     eServerPacketType_QSyncThreadState,
     eServerPacketType_QThreadSuffixSupported,

source/Utility/StringExtractorGDBRemote.cpp

case 'Q':
     switch (packet_cstr[1]) {
     case 'E':
       if (PACKET_STARTS_WITH("QEnvironment:"))
         return eServerPacketType_QEnvironment;
       if (PACKET_STARTS_WITH("QEnvironmentHexEncoded:"))
         return eServerPacketType_QEnvironmentHexEncoded;
       break;
 
+    case 'P':
+      if (PACKET_STARTS_WITH("QPassSignals:"))
+        return eServerPacketType_QPassSignals;
+
     case 'S':
       if (PACKET_MATCHES("QStartNoAckMode"))
         return eServerPacketType_QStartNoAckMode;
       if (PACKET_STARTS_WITH("QSaveRegisterState"))
         return eServerPacketType_QSaveRegisterState;
       if (PACKET_STARTS_WITH("QSetDisableASLR:"))
         return eServerPacketType_QSetDisableASLR;
       if (PACKET_STARTS_WITH("QSetDetachOnError:"))