[Lldb-commits] [lldb] 2f377c5 - [lldb][NFCI] Remove use of ConstString from UnixSignals
Alex Langford via lldb-commits
lldb-commits at lists.llvm.org
Thu Sep 14 10:21:31 PDT 2023
Author: Alex Langford
Date: 2023-09-14T10:19:53-07:00
New Revision: 2f377c5bd713e9ee72faa6a99088bb81358059e3
URL: https://github.com/llvm/llvm-project/commit/2f377c5bd713e9ee72faa6a99088bb81358059e3
DIFF: https://github.com/llvm/llvm-project/commit/2f377c5bd713e9ee72faa6a99088bb81358059e3.diff
LOG: [lldb][NFCI] Remove use of ConstString from UnixSignals
The majority of UnixSignals strings are static in the sense that they do
not change. The overwhelming majority of these strings are string
literals. Using ConstString to manage their lifetime does not make
sense. The only exception to this is one of the subclasses of
UnixSignals, for which I have created a StringSet local to that file
which will guarantee the lifetimes of these StringRefs.
As for the other benefits of ConstString, string uniqueness is not a
concern (as many of them are already string literals) and comparing
signal names and aliases should not be a hot path.
Differential Revision: https://reviews.llvm.org/D159011
Added:
Modified:
lldb/include/lldb/Target/UnixSignals.h
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/source/Target/UnixSignals.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Target/UnixSignals.h b/lldb/include/lldb/Target/UnixSignals.h
index b227b67e289229f..b3605ccefddbea4 100644
--- a/lldb/include/lldb/Target/UnixSignals.h
+++ b/lldb/include/lldb/Target/UnixSignals.h
@@ -14,7 +14,6 @@
#include <string>
#include <vector>
-#include "lldb/Utility/ConstString.h"
#include "lldb/lldb-private.h"
#include "llvm/Support/JSON.h"
@@ -101,9 +100,10 @@ class UnixSignals {
// your subclass of UnixSignals or in your Process Plugin's GetUnixSignals
// method before you return the UnixSignal object.
- void AddSignal(int signo, const char *name, bool default_suppress,
+ void AddSignal(int signo, llvm::StringRef name, bool default_suppress,
bool default_stop, bool default_notify,
- const char *description, const char *alias = nullptr);
+ llvm::StringRef description,
+ llvm::StringRef alias = llvm::StringRef());
enum SignalCodePrintOption { None, Address, Bounds };
@@ -147,17 +147,20 @@ class UnixSignals {
const SignalCodePrintOption m_print_option;
};
+ // The StringRefs in Signal are either backed by string literals or reside in
+ // persistent storage (e.g. a StringSet).
struct Signal {
- ConstString m_name;
- ConstString m_alias;
- std::string m_description;
+ llvm::StringRef m_name;
+ llvm::StringRef m_alias;
+ llvm::StringRef m_description;
std::map<int32_t, SignalCode> m_codes;
uint32_t m_hit_count = 0;
bool m_suppress : 1, m_stop : 1, m_notify : 1;
bool m_default_suppress : 1, m_default_stop : 1, m_default_notify : 1;
- Signal(const char *name, bool default_suppress, bool default_stop,
- bool default_notify, const char *description, const char *alias);
+ Signal(llvm::StringRef name, bool default_suppress, bool default_stop,
+ bool default_notify, llvm::StringRef description,
+ llvm::StringRef alias);
~Signal() = default;
void Reset(bool reset_stop, bool reset_notify, bool reset_suppress);
diff --git a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
index 8734a3666a6d8b5..88f1ad15b6b485d 100644
--- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -28,10 +28,12 @@
#include "lldb/Utility/Status.h"
#include "lldb/Utility/StreamString.h"
#include "lldb/Utility/UriParser.h"
+#include "llvm/ADT/StringSet.h"
#include "llvm/Support/FormatAdapters.h"
#include "Plugins/Process/Utility/GDBRemoteSignals.h"
#include "Plugins/Process/gdb-remote/ProcessGDBRemote.h"
+#include <mutex>
#include <optional>
using namespace lldb;
@@ -41,6 +43,11 @@ using namespace lldb_private::platform_gdb_server;
LLDB_PLUGIN_DEFINE_ADV(PlatformRemoteGDBServer, PlatformGDB)
static bool g_initialized = false;
+// UnixSignals does not store the signal names or descriptions itself.
+// It holds onto StringRefs. Becaue we may get signal information dynamically
+// from the remote, these strings need persistent storage client-side.
+static std::mutex g_signal_string_mutex;
+static llvm::StringSet<> g_signal_string_storage;
void PlatformRemoteGDBServer::Initialize() {
Platform::Initialize();
@@ -749,8 +756,18 @@ const UnixSignalsSP &PlatformRemoteGDBServer::GetRemoteUnixSignals() {
if (object_sp && object_sp->IsValid())
description = std::string(object_sp->GetStringValue());
- remote_signals_sp->AddSignal(signo, name.str().c_str(), suppress, stop,
- notify, description.c_str());
+ llvm::StringRef name_backed, description_backed;
+ {
+ std::lock_guard<std::mutex> guard(g_signal_string_mutex);
+ name_backed =
+ g_signal_string_storage.insert(name).first->getKeyData();
+ if (!description.empty())
+ description_backed =
+ g_signal_string_storage.insert(description).first->getKeyData();
+ }
+
+ remote_signals_sp->AddSignal(signo, name_backed, suppress, stop, notify,
+ description_backed);
return true;
});
diff --git a/lldb/source/Target/UnixSignals.cpp b/lldb/source/Target/UnixSignals.cpp
index b8e2a7675f7b244..e3c7a83ece07305 100644
--- a/lldb/source/Target/UnixSignals.cpp
+++ b/lldb/source/Target/UnixSignals.cpp
@@ -18,17 +18,13 @@
using namespace lldb_private;
using namespace llvm;
-UnixSignals::Signal::Signal(const char *name, bool default_suppress,
+UnixSignals::Signal::Signal(llvm::StringRef name, bool default_suppress,
bool default_stop, bool default_notify,
- const char *description, const char *alias)
- : m_name(name), m_alias(alias), m_description(),
+ llvm::StringRef description, llvm::StringRef alias)
+ : m_name(name), m_alias(alias), m_description(description),
m_suppress(default_suppress), m_stop(default_stop),
- m_notify(default_notify),
- m_default_suppress(default_suppress), m_default_stop(default_stop),
- m_default_notify(default_notify) {
- if (description)
- m_description.assign(description);
-}
+ m_notify(default_notify), m_default_suppress(default_suppress),
+ m_default_stop(default_stop), m_default_notify(default_notify) {}
lldb::UnixSignalsSP UnixSignals::Create(const ArchSpec &arch) {
const auto &triple = arch.GetTriple();
@@ -104,9 +100,10 @@ void UnixSignals::Reset() {
// clang-format on
}
-void UnixSignals::AddSignal(int signo, const char *name, bool default_suppress,
- bool default_stop, bool default_notify,
- const char *description, const char *alias) {
+void UnixSignals::AddSignal(int signo, llvm::StringRef name,
+ bool default_suppress, bool default_stop,
+ bool default_notify, llvm::StringRef description,
+ llvm::StringRef alias) {
Signal new_signal(name, default_suppress, default_stop, default_notify,
description, alias);
m_signals.insert(std::make_pair(signo, new_signal));
@@ -135,7 +132,7 @@ llvm::StringRef UnixSignals::GetSignalAsStringRef(int32_t signo) const {
const auto pos = m_signals.find(signo);
if (pos == m_signals.end())
return {};
- return pos->second.m_name.GetStringRef();
+ return pos->second.m_name;
}
std::string
@@ -147,7 +144,7 @@ UnixSignals::GetSignalDescription(int32_t signo, std::optional<int32_t> code,
collection::const_iterator pos = m_signals.find(signo);
if (pos != m_signals.end()) {
- str = pos->second.m_name.GetCString();
+ str = pos->second.m_name.str();
if (code) {
std::map<int32_t, SignalCode>::const_iterator cpos =
@@ -199,14 +196,13 @@ llvm::StringRef UnixSignals::GetShortName(llvm::StringRef name) const {
}
int32_t UnixSignals::GetSignalNumberFromName(const char *name) const {
- ConstString const_name(name);
+ llvm::StringRef name_ref(name);
collection::const_iterator pos, end = m_signals.end();
for (pos = m_signals.begin(); pos != end; pos++) {
- if ((const_name == pos->second.m_name) ||
- (const_name == pos->second.m_alias) ||
- (const_name == GetShortName(pos->second.m_name)) ||
- (const_name == GetShortName(pos->second.m_alias)))
+ if ((name_ref == pos->second.m_name) || (name_ref == pos->second.m_alias) ||
+ (name_ref == GetShortName(pos->second.m_name)) ||
+ (name_ref == GetShortName(pos->second.m_alias)))
return pos->first;
}
@@ -373,11 +369,10 @@ void UnixSignals::IncrementSignalHitCount(int signo) {
json::Value UnixSignals::GetHitCountStatistics() const {
json::Array json_signals;
- for (const auto &pair: m_signals) {
+ for (const auto &pair : m_signals) {
if (pair.second.m_hit_count > 0)
- json_signals.emplace_back(json::Object{
- { pair.second.m_name.GetCString(), pair.second.m_hit_count }
- });
+ json_signals.emplace_back(
+ json::Object{{pair.second.m_name, pair.second.m_hit_count}});
}
return std::move(json_signals);
}
More information about the lldb-commits
mailing list