eugene created this revision.

If QPassSignals packaet is supported by lldb-server, lldb-client will utilize 
it and ask the server to ignore signals that don't require stops or 
notifications.
Such signals will be immediately re-injected into inferior to continue normal 
execution.


https://reviews.llvm.org/D30520

Files:
  include/lldb/Target/UnixSignals.h
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  source/Target/UnixSignals.cpp
  unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Index: unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===================================================================
--- unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -211,10 +211,11 @@
       std::async(std::launch::async,
                  [&] { return client.GetModulesInfo(file_specs, triple); });
   HandlePacket(
-      server, "jModulesInfo:["
-              R"({"file":"/foo/bar.so","triple":"i386-pc-linux"},)"
-              R"({"file":"/foo/baz.so","triple":"i386-pc-linux"},)"
-              R"({"file":"/foo/baw.so","triple":"i386-pc-linux"}])",
+      server,
+      "jModulesInfo:["
+      R"({"file":"/foo/bar.so","triple":"i386-pc-linux"},)"
+      R"({"file":"/foo/baz.so","triple":"i386-pc-linux"},)"
+      R"({"file":"/foo/baw.so","triple":"i386-pc-linux"}])",
       R"([{"uuid":"404142434445464748494a4b4c4d4e4f","triple":"i386-pc-linux",)"
       R"("file_path":"/foo/bar.so","file_offset":0,"file_size":1234}]])");
 
@@ -314,3 +315,27 @@
       << ss.GetString();
   ASSERT_EQ(10, num_packets);
 }
+
+TEST_F(GDBRemoteCommunicationClientTest, SendSignalsToIgnore) {
+  TestClient client;
+  MockServer server;
+  Connect(client, server);
+  if (HasFailure())
+    return;
+
+  const lldb::tid_t tid = 0x47;
+  const uint32_t reg_num = 4;
+  std::future<Error> result = std::async(std::launch::async, [&] {
+    return client.SendSignalsToIgnore({2, 3, 5, 7, 0xB, 0xD, 0x11});
+  });
+
+  HandlePacket(server, "QPassSignals:2;3;5;7;B;D;11", "OK");
+  EXPECT_TRUE(result.get().Success());
+
+  result = std::async(std::launch::async, [&] {
+    return client.SendSignalsToIgnore(std::vector<int32_t>());
+  });
+
+  HandlePacket(server, "QPassSignals:", "OK");
+  EXPECT_TRUE(result.get().Success());
+}
Index: source/Target/UnixSignals.cpp
===================================================================
--- source/Target/UnixSignals.cpp
+++ source/Target/UnixSignals.cpp
@@ -123,12 +123,14 @@
   Signal new_signal(name, default_suppress, default_stop, default_notify,
                     description, alias);
   m_signals.insert(std::make_pair(signo, new_signal));
+  ++m_version;
 }
 
 void UnixSignals::RemoveSignal(int signo) {
   collection::iterator pos = m_signals.find(signo);
   if (pos != m_signals.end())
     m_signals.erase(pos);
+  ++m_version;
 }
 
 const char *UnixSignals::GetSignalAsCString(int signo) const {
@@ -217,6 +219,7 @@
   collection::iterator pos = m_signals.find(signo);
   if (pos != m_signals.end()) {
     pos->second.m_suppress = value;
+    ++m_version;
     return true;
   }
   return false;
@@ -240,6 +243,7 @@
   collection::iterator pos = m_signals.find(signo);
   if (pos != m_signals.end()) {
     pos->second.m_stop = value;
+    ++m_version;
     return true;
   }
   return false;
@@ -263,6 +267,7 @@
   collection::iterator pos = m_signals.find(signo);
   if (pos != m_signals.end()) {
     pos->second.m_notify = value;
+    ++m_version;
     return true;
   }
   return false;
@@ -284,3 +289,7 @@
   std::advance(it, index);
   return it->first;
 }
+
+uint64_t UnixSignals::GetVersion() const {
+  return m_version;
+}
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===================================================================
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -403,12 +403,15 @@
   //------------------------------------------------------------------
   std::string m_partial_profile_data;
   std::map<uint64_t, uint32_t> m_thread_id_to_used_usec_map;
+  uint64_t m_last_signals_version = 0;
 
   static bool NewThreadNotifyBreakpointHit(void *baton,
                                            StoppointCallbackContext *context,
                                            lldb::user_id_t break_id,
                                            lldb::user_id_t break_loc_id);
 
+  Error SendSignalsToIgnoreIfNeeded();
+
   //------------------------------------------------------------------
   // ContinueDelegate interface
   //------------------------------------------------------------------
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -79,9 +79,10 @@
 #include "Utility/StringExtractorGDBRemote.h"
 #include "lldb/Host/Host.h"
 
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringSwitch.h"
-#include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/Threading.h"
+#include "llvm/Support/raw_ostream.h"
 
 #define DEBUGSERVER_BASENAME "debugserver"
 using namespace lldb;
@@ -949,6 +950,7 @@
               lldb_utility::PseudoTerminal::invalid_fd)
             SetSTDIOFileDescriptor(pty.ReleaseMasterFileDescriptor());
         }
+        SendSignalsToIgnoreIfNeeded();
       }
     } else {
       if (log)
@@ -1064,20 +1066,22 @@
       if (log)
         log->Printf("ProcessGDBRemote::%s gdb-remote had process architecture, "
                     "using %s %s",
-                    __FUNCTION__, process_arch.GetArchitectureName()
-                                      ? process_arch.GetArchitectureName()
-                                      : "<null>",
+                    __FUNCTION__,
+                    process_arch.GetArchitectureName()
+                        ? process_arch.GetArchitectureName()
+                        : "<null>",
                     process_arch.GetTriple().getTriple().c_str()
                         ? process_arch.GetTriple().getTriple().c_str()
                         : "<null>");
     } else {
       process_arch = m_gdb_comm.GetHostArchitecture();
       if (log)
         log->Printf("ProcessGDBRemote::%s gdb-remote did not have process "
                     "architecture, using gdb-remote host architecture %s %s",
-                    __FUNCTION__, process_arch.GetArchitectureName()
-                                      ? process_arch.GetArchitectureName()
-                                      : "<null>",
+                    __FUNCTION__,
+                    process_arch.GetArchitectureName()
+                        ? process_arch.GetArchitectureName()
+                        : "<null>",
                     process_arch.GetTriple().getTriple().c_str()
                         ? process_arch.GetTriple().getTriple().c_str()
                         : "<null>");
@@ -1089,9 +1093,10 @@
         if (log)
           log->Printf(
               "ProcessGDBRemote::%s analyzing target arch, currently %s %s",
-              __FUNCTION__, target_arch.GetArchitectureName()
-                                ? target_arch.GetArchitectureName()
-                                : "<null>",
+              __FUNCTION__,
+              target_arch.GetArchitectureName()
+                  ? target_arch.GetArchitectureName()
+                  : "<null>",
               target_arch.GetTriple().getTriple().c_str()
                   ? target_arch.GetTriple().getTriple().c_str()
                   : "<null>");
@@ -1112,9 +1117,10 @@
           if (log)
             log->Printf("ProcessGDBRemote::%s remote process is ARM/Apple, "
                         "setting target arch to %s %s",
-                        __FUNCTION__, process_arch.GetArchitectureName()
-                                          ? process_arch.GetArchitectureName()
-                                          : "<null>",
+                        __FUNCTION__,
+                        process_arch.GetArchitectureName()
+                            ? process_arch.GetArchitectureName()
+                            : "<null>",
                         process_arch.GetTriple().getTriple().c_str()
                             ? process_arch.GetTriple().getTriple().c_str()
                             : "<null>");
@@ -1142,9 +1148,10 @@
         if (log)
           log->Printf("ProcessGDBRemote::%s final target arch after "
                       "adjustments for remote architecture: %s %s",
-                      __FUNCTION__, target_arch.GetArchitectureName()
-                                        ? target_arch.GetArchitectureName()
-                                        : "<null>",
+                      __FUNCTION__,
+                      target_arch.GetArchitectureName()
+                          ? target_arch.GetArchitectureName()
+                          : "<null>",
                       target_arch.GetTriple().getTriple().c_str()
                           ? target_arch.GetTriple().getTriple().c_str()
                           : "<null>");
@@ -1265,6 +1272,7 @@
 
   ListenerSP listener_sp(
       Listener::MakeListener("gdb-remote.resume-packet-sent"));
+  SendSignalsToIgnoreIfNeeded();
   if (listener_sp->StartListeningForEvents(
           &m_gdb_comm, GDBRemoteCommunication::eBroadcastBitRunPacketSent)) {
     listener_sp->StartListeningForEvents(
@@ -3739,6 +3747,55 @@
   return false;
 }
 
+Error ProcessGDBRemote::SendSignalsToIgnoreIfNeeded() {
+  Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
+  LLDB_LOG(log, "Check if need to update ignored signals");
+
+  // QPassSignals package is not supported by the server,
+  // there is way we can ignore any signals on server side.
+  if (!m_gdb_comm.GetQPassSignalsSupported())
+    return Error();
+
+  // No signals, nothing to send.
+  if (m_unix_signals_sp == nullptr)
+    return Error();
+
+  // Signals' version hasn't changed, no need to send anything.
+  uint64_t new_signals_version = m_unix_signals_sp->GetVersion();
+  if (new_signals_version == m_last_signals_version) {
+    LLDB_LOG(log, "Signals' version hasn't changed. version={0}",
+             m_last_signals_version);
+    return Error();
+  }
+
+  llvm::SmallVector<int32_t, 40> signals_to_ignore;
+  int32_t signo = m_unix_signals_sp->GetFirstSignalNumber();
+  while (signo != LLDB_INVALID_SIGNAL_NUMBER) {
+    bool should_ignore = !m_unix_signals_sp->GetShouldStop(signo) &&
+                         !m_unix_signals_sp->GetShouldNotify(signo) &&
+                         !m_unix_signals_sp->GetShouldSuppress(signo);
+
+    if (should_ignore)
+      signals_to_ignore.push_back(signo);
+    signo = m_unix_signals_sp->GetNextSignalNumber(signo);
+  }
+
+  LLDB_LOG(log,
+           "Signals' version changed. old version={0}, new version={1}, "
+           "signals ignored={2}",
+           m_last_signals_version, new_signals_version,
+           signals_to_ignore.size());
+
+  Error error = m_gdb_comm.SendSignalsToIgnore(signals_to_ignore);
+
+  if (error.Success())
+    m_last_signals_version = new_signals_version;
+  else {
+    LLDB_LOG(log, "Error: {0}", error);
+  }
+  return error;
+}
+
 bool ProcessGDBRemote::StartNoticingNewThreads() {
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
   if (m_thread_create_bp_sp) {
@@ -4112,8 +4169,9 @@
     return false;
 
   feature_node.ForEachChildElementWithName(
-      "reg", [&target_info, &dyn_reg_info, &cur_reg_num, &reg_offset,
-              &abi_sp](const XMLNode &reg_node) -> bool {
+      "reg",
+      [&target_info, &dyn_reg_info, &cur_reg_num, &reg_offset,
+       &abi_sp](const XMLNode &reg_node) -> bool {
         std::string gdb_group;
         std::string gdb_type;
         ConstString reg_name;
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===================================================================
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -347,6 +347,8 @@
 
   bool GetEchoSupported();
 
+  bool GetQPassSignalsSupported();
+
   bool GetAugmentedLibrariesSVR4ReadSupported();
 
   bool GetQXferFeaturesReadSupported();
@@ -450,6 +452,9 @@
 
   void ServeSymbolLookups(lldb_private::Process *process);
 
+  // Sends QPassSignals packet to the server with given signals to ignore.
+  Error SendSignalsToIgnore(llvm::ArrayRef<int32_t> signals);
+
   //------------------------------------------------------------------
   /// Return the feature set supported by the gdb-remote server.
   ///
@@ -527,6 +532,7 @@
   LazyBool m_supports_jThreadExtendedInfo;
   LazyBool m_supports_jLoadedDynamicLibrariesInfos;
   LazyBool m_supports_jGetSharedCacheInfo;
+  LazyBool m_supports_QPassSignals;
 
   bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1,
       m_supports_qUserName : 1, m_supports_qGroupName : 1,
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===================================================================
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -40,6 +40,7 @@
 #include "Utility/StringExtractorGDBRemote.h"
 #include "lldb/Host/Config.h"
 
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 
 #if defined(HAVE_LIBCOMPRESSION)
@@ -87,6 +88,7 @@
       m_supports_jThreadExtendedInfo(eLazyBoolCalculate),
       m_supports_jLoadedDynamicLibrariesInfos(eLazyBoolCalculate),
       m_supports_jGetSharedCacheInfo(eLazyBoolCalculate),
+      m_supports_QPassSignals(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),
@@ -150,6 +152,13 @@
   return m_supports_qEcho == eLazyBoolYes;
 }
 
+bool GDBRemoteCommunicationClient::GetQPassSignalsSupported() {
+  if (m_supports_QPassSignals == eLazyBoolCalculate) {
+    GetRemoteQSupported();
+  }
+  return m_supports_QPassSignals == eLazyBoolYes;
+}
+
 bool GDBRemoteCommunicationClient::GetAugmentedLibrariesSVR4ReadSupported() {
   if (m_supports_augmented_libraries_svr4_read == eLazyBoolCalculate) {
     GetRemoteQSupported();
@@ -419,6 +428,11 @@
     else
       m_supports_qEcho = eLazyBoolNo;
 
+    if (::strstr(response_cstr, "QPassSignals+"))
+      m_supports_QPassSignals = eLazyBoolYes;
+    else
+      m_supports_QPassSignals = eLazyBoolNo;
+
     const char *packet_size_str = ::strstr(response_cstr, "PacketSize=");
     if (packet_size_str) {
       StringExtractorGDBRemote packet_response(packet_size_str +
@@ -589,9 +603,9 @@
   if (m_supports_jLoadedDynamicLibrariesInfos == eLazyBoolCalculate) {
     StringExtractorGDBRemote response;
     m_supports_jLoadedDynamicLibrariesInfos = eLazyBoolNo;
-    if (SendPacketAndWaitForResponse("jGetLoadedDynamicLibrariesInfos:",
-                                     response,
-                                     false) == PacketResult::Success) {
+    if (SendPacketAndWaitForResponse(
+            "jGetLoadedDynamicLibrariesInfos:", response, false) ==
+        PacketResult::Success) {
       if (response.IsOKResponse()) {
         m_supports_jLoadedDynamicLibrariesInfos = eLazyBoolYes;
       }
@@ -1276,9 +1290,10 @@
           if (log)
             log->Printf("GDBRemoteCommunicationClient::%s parsed host "
                         "architecture as %s, triple as %s from triple text %s",
-                        __FUNCTION__, m_host_arch.GetArchitectureName()
-                                          ? m_host_arch.GetArchitectureName()
-                                          : "<null-arch-name>",
+                        __FUNCTION__,
+                        m_host_arch.GetArchitectureName()
+                            ? m_host_arch.GetArchitectureName()
+                            : "<null-arch-name>",
                         m_host_arch.GetTriple().getTriple().c_str(),
                         triple.c_str());
         }
@@ -2417,7 +2432,7 @@
      * The reply from '?' packet could be as simple as 'S05'. There is no packet
      * which can
      * give us pid and/or tid. Assume pid=tid=1 in such cases.
-    */
+     */
     if (response.IsUnsupportedResponse() && IsConnected()) {
       m_curr_tid = 1;
       return true;
@@ -2453,7 +2468,7 @@
      * The reply from '?' packet could be as simple as 'S05'. There is no packet
      * which can
      * give us pid and/or tid. Assume pid=tid=1 in such cases.
-    */
+     */
     if (response.IsUnsupportedResponse() && IsConnected()) {
       m_curr_tid_run = 1;
       return true;
@@ -2591,7 +2606,7 @@
      * be as simple as 'S05'. There is no packet which can give us pid and/or
      * tid.
      * Assume pid=tid=1 in such cases.
-    */
+     */
     if (response.IsUnsupportedResponse() && thread_ids.size() == 0 &&
         IsConnected()) {
       thread_ids.push_back(1);
@@ -3569,6 +3584,33 @@
              : nullptr;
 }
 
+Error GDBRemoteCommunicationClient::SendSignalsToIgnore(
+    llvm::ArrayRef<int32_t> signals) {
+  // Format package:
+  // QPassSignals:<hex_sig1>;<hex_sig2>...;<hex_sigN>
+  StreamString packet;
+  packet.PutCString("QPassSignals:");
+  for (size_t i = 0; i < signals.size(); ++i) {
+    if (i != 0)
+      packet.PutCString(";");
+
+    packet << llvm::utohexstr(static_cast<uint32_t>(signals[i]));
+  }
+
+  StringExtractorGDBRemote response;
+  auto send_status =
+      SendPacketAndWaitForResponse(packet.GetString(), response, false);
+
+  if (send_status != GDBRemoteCommunication::PacketResult::Success)
+    return Error("Sending QPassSignals packet failed");
+
+  if (response.IsOKResponse()) {
+    return Error();
+  } else {
+    return Error("Unknown error happened during sending QPassSignals packet.");
+  }
+}
+
 Error GDBRemoteCommunicationClient::ConfigureRemoteStructuredData(
     const ConstString &type_name, const StructuredData::ObjectSP &config_sp) {
   Error error;
Index: include/lldb/Target/UnixSignals.h
===================================================================
--- include/lldb/Target/UnixSignals.h
+++ include/lldb/Target/UnixSignals.h
@@ -88,6 +88,10 @@
 
   void RemoveSignal(int signo);
 
+  // Returns a current version of the data stored in this class.
+  // Version gets incremented each time Set... method is called.
+  uint64_t GetVersion() const;
+
 protected:
   //------------------------------------------------------------------
   // Classes that inherit from UnixSignals can see and modify these
@@ -110,6 +114,7 @@
   typedef std::map<int32_t, Signal> collection;
 
   collection m_signals;
+  uint64_t m_version = 0;
 
   // GDBRemote signals need to be copyable.
   UnixSignals(const UnixSignals &rhs);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to