eugene updated this revision to Diff 90407.
eugene added a comment.
Addressing code review commends, and moving a signal filtering method to the
base Process class.
https://reviews.llvm.org/D30520
Files:
include/lldb/Target/Process.h
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/Process.cpp
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
@@ -314,3 +314,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:02;03;05;07;0b;0d;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,5 @@
std::advance(it, index);
return it->first;
}
+
+uint64_t UnixSignals::GetVersion() const { return m_version; }
Index: source/Target/Process.cpp
===================================================================
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -1621,6 +1621,13 @@
return PrivateResume();
}
+Error Process::WillResume() {
+ UpdateAutomaticSignalFiltering();
+ return Error();
+}
+
+void Process::DidLaunch() { UpdateAutomaticSignalFiltering(); }
+
Error Process::ResumeSynchronous(Stream *stream) {
Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_STATE |
LIBLLDB_LOG_PROCESS));
@@ -6217,3 +6224,9 @@
find_it->second->HandleArrivalOfStructuredData(*this, type_name, object_sp);
return true;
}
+
+Error Process::UpdateAutomaticSignalFiltering() {
+ // Default implementation does nothign.
+ // No automatic signal filtering to speak of.
+ return Error();
+}
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===================================================================
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -397,12 +397,15 @@
lldb::addr_t base_addr,
bool value_is_offset);
+ Error UpdateAutomaticSignalFiltering() override;
+
private:
//------------------------------------------------------------------
// For ProcessGDBRemote only
//------------------------------------------------------------------
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,
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;
@@ -1165,6 +1166,7 @@
}
void ProcessGDBRemote::DidLaunch() {
+ Process::DidLaunch();
ArchSpec process_arch;
DidLaunchOrAttach(process_arch);
}
@@ -1248,6 +1250,7 @@
}
Error ProcessGDBRemote::WillResume() {
+ Process::WillResume();
m_continue_c_tids.clear();
m_continue_C_tids.clear();
m_continue_s_tids.clear();
@@ -3739,6 +3742,54 @@
return false;
}
+Error ProcessGDBRemote::UpdateAutomaticSignalFiltering() {
+ 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) {
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
@@ -87,6 +87,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 +151,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 +427,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 +
@@ -3569,6 +3582,26 @@
: nullptr;
}
+Error GDBRemoteCommunicationClient::SendSignalsToIgnore(
+ llvm::ArrayRef<int32_t> signals) {
+ // Format packet:
+ // QPassSignals:<hex_sig1>;<hex_sig2>...;<hex_sigN>
+ auto range = llvm::make_range(signals.begin(), signals.end());
+ std::string packet = formatv("QPassSignals:{0:$[;]@(x-2)}", range).str();
+
+ StringExtractorGDBRemote response;
+ auto send_status = SendPacketAndWaitForResponse(packet, 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);
Index: include/lldb/Target/Process.h
===================================================================
--- include/lldb/Target/Process.h
+++ include/lldb/Target/Process.h
@@ -1236,7 +1236,7 @@
/// Allow Process plug-ins to execute some code after launching
/// a process.
//------------------------------------------------------------------
- virtual void DidLaunch() {}
+ virtual void DidLaunch();
//------------------------------------------------------------------
/// Called before resuming to a process.
@@ -1247,7 +1247,7 @@
/// @return
/// Returns an error object.
//------------------------------------------------------------------
- virtual Error WillResume() { return Error(); }
+ virtual Error WillResume();
//------------------------------------------------------------------
/// Resumes all of a process's threads as configured using the
@@ -2607,7 +2607,7 @@
bool RunPreResumeActions();
void ClearPreResumeActions();
-
+
void ClearPreResumeAction(PreResumeActionCallback callback, void *baton);
ProcessRunLock &GetRunLock();
@@ -3145,6 +3145,8 @@
Error StopForDestroyOrDetach(lldb::EventSP &exit_event_sp);
+ virtual Error UpdateAutomaticSignalFiltering();
+
bool StateChangedIsExternallyHijacked();
void LoadOperatingSystemPlugin(bool flush);
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits