From f7f4c298f8acac3c467444471a2b359495c83348 Mon Sep 17 00:00:00 2001 From: Simon Farre Date: Thu, 11 Jul 2024 12:08:17 +0200 Subject: [PATCH] Added dynamic `TargetDescription` TargetDescription dynamically generates the "master" target description contents (target.xml), so that we only have to manage the individual concrete CPU features (like foo-sse.xml, for instance). TargetDescription will generate the xml contents that includes the used features. Removed combinatorial target.xml files These are now handled by `TargetDescription` instead. This unblocks the #3776 PR (AVX512 support) from getting pulled in, though that PR will need to take this PR into account. --- CMakeLists.txt | 7 +- src/GdbServer.cc | 1 + src/GdbServerConnection.cc | 43 +++------ src/GdbServerConnection.h | 5 +- src/TargetDescription.cc | 132 ++++++++++++++++++++++++++ src/TargetDescription.h | 35 +++++++ third-party/gdb/amd64-avx-linux.xml | 19 ---- third-party/gdb/amd64-linux.xml | 19 ---- third-party/gdb/amd64-pkeys-linux.xml | 20 ---- third-party/gdb/i386-avx-linux.xml | 18 ---- third-party/gdb/i386-linux.xml | 18 ---- third-party/gdb/i386-pkeys-linux.xml | 19 ---- 12 files changed, 186 insertions(+), 150 deletions(-) create mode 100644 src/TargetDescription.cc create mode 100644 src/TargetDescription.h delete mode 100644 third-party/gdb/amd64-avx-linux.xml delete mode 100644 third-party/gdb/amd64-linux.xml delete mode 100644 third-party/gdb/amd64-pkeys-linux.xml delete mode 100644 third-party/gdb/i386-avx-linux.xml delete mode 100644 third-party/gdb/i386-linux.xml delete mode 100644 third-party/gdb/i386-pkeys-linux.xml diff --git a/CMakeLists.txt b/CMakeLists.txt index 1c3a4780745..124fbf69c29 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -648,6 +648,7 @@ set(RR_SOURCES src/SourcesCommand.cc src/StdioMonitor.cc src/SysCpuMonitor.cc + src/TargetDescription.cc src/Task.cc src/ThreadGroup.cc src/TraceeAttentionSet.cc @@ -797,12 +798,6 @@ set(RR_GDB_RESOURCES 64bit-seg.xml 64bit-sse.xml 64bit-pkeys.xml - amd64-pkeys-linux.xml - amd64-avx-linux.xml - amd64-linux.xml - i386-pkeys-linux.xml - i386-avx-linux.xml - i386-linux.xml aarch64-core.xml aarch64-fpu.xml aarch64-pauth.xml diff --git a/src/GdbServer.cc b/src/GdbServer.cc index 18bbe46cdb8..86ff30e347e 100644 --- a/src/GdbServer.cc +++ b/src/GdbServer.cc @@ -206,6 +206,7 @@ void GdbServer::dispatch_regs_request(const Registers& regs, return; } vector rs; + rs.reserve(end); for (GdbServerRegister r = GdbServerRegister(0); r <= end; r = GdbServerRegister(r + 1)) { rs.push_back(get_reg(regs, extra_regs, r)); } diff --git a/src/GdbServerConnection.cc b/src/GdbServerConnection.cc index d551441f76c..eba06202da7 100644 --- a/src/GdbServerConnection.cc +++ b/src/GdbServerConnection.cc @@ -30,6 +30,7 @@ #include "DebuggerExtensionCommandHandler.h" #include "ReplaySession.h" #include "ScopedFd.h" +#include "TargetDescription.h" #include "core.h" #include "log.h" @@ -67,7 +68,8 @@ GdbServerConnection::GdbServerConnection(ThreadGroupUid tguid, const Features& f multiprocess_supported_(false), hwbreak_supported_(false), swbreak_supported_(false), - list_threads_in_stop_reply_(false) { + list_threads_in_stop_reply_(false), + target_description(nullptr) { #ifndef REVERSE_EXECUTION features_.reverse_execution = false; #endif @@ -111,7 +113,7 @@ unique_ptr GdbServerConnection::await_connection( Task* t, ScopedFd& listen_fd, const GdbServerConnection::Features& features) { auto dbg = unique_ptr( new GdbServerConnection(t->thread_group()->tguid(), features)); - dbg->set_cpu_features(get_cpu_features(t->arch())); + dbg->set_cpu_features(t->arch()); dbg->await_debugger(listen_fd); return dbg; } @@ -503,29 +505,6 @@ static string read_target_desc(const char* file_name) { return ss.str(); } -static const char* target_description_name(uint32_t cpu_features) { - // This doesn't scale, but it's what gdb does... - switch (cpu_features) { - case 0: - return "i386-linux.xml"; - case GdbServerConnection::CPU_X86_64: - return "amd64-linux.xml"; - case GdbServerConnection::CPU_AVX: - return "i386-avx-linux.xml"; - case GdbServerConnection::CPU_X86_64 | GdbServerConnection::CPU_AVX: - return "amd64-avx-linux.xml"; - case GdbServerConnection::CPU_PKU | GdbServerConnection::CPU_AVX: - return "i386-pkeys-linux.xml"; - case GdbServerConnection::CPU_X86_64 | GdbServerConnection::CPU_PKU | GdbServerConnection::CPU_AVX: - return "amd64-pkeys-linux.xml"; - case GdbServerConnection::CPU_AARCH64: - return "aarch64-core.xml"; - default: - FATAL() << "Unknown features"; - return nullptr; - } -} - bool GdbServerConnection::xfer(const char* name, char* args) { const char* mode = args; args = strchr(args, ':'); @@ -609,11 +588,8 @@ bool GdbServerConnection::xfer(const char* name, char* args) { return false; } - string target_desc = - read_target_desc((strcmp(annex, "") && strcmp(annex, "target.xml")) - ? annex - : target_description_name(cpu_features_)); - write_xfer_response(target_desc.c_str(), target_desc.size(), offset, len); + const auto desc = strcmp(annex, "") && strcmp(annex, "target.xml") ? read_target_desc(annex) : target_description->to_xml(); + write_xfer_response(desc.c_str(), desc.size(), offset, len); return false; } @@ -2335,4 +2311,11 @@ bool GdbServerConnection::is_connection_alive() { return connection_alive_; } bool GdbServerConnection::is_pass_signal(int sig) { return pass_signals.find(to_gdb_signum(sig)) != pass_signals.end(); } +void GdbServerConnection::set_cpu_features(SupportedArch arch) { + cpu_features_ = get_cpu_features(arch); + DEBUG_ASSERT(target_description == nullptr && + "Target description already created"); + target_description = std::make_unique(arch, cpu_features_); +} + } // namespace rr diff --git a/src/GdbServerConnection.h b/src/GdbServerConnection.h index a463eac6693..a8a9aa18fc6 100644 --- a/src/GdbServerConnection.h +++ b/src/GdbServerConnection.h @@ -17,6 +17,7 @@ #include "ReplaySession.h" #include "ReplayTimeline.h" #include "ScopedFd.h" +#include "TargetDescription.h" #include "TaskishUid.h" #include "core.h" @@ -750,7 +751,8 @@ class GdbServerConnection { CPU_AARCH64 = 0x4, CPU_PKU = 0x8 }; - void set_cpu_features(uint32_t features) { cpu_features_ = features; } + + void set_cpu_features(SupportedArch arch); uint32_t cpu_features() const { return cpu_features_; } GdbServerConnection(ThreadGroupUid tguid, const Features& features); @@ -877,6 +879,7 @@ class GdbServerConnection { bool hwbreak_supported_; // client supports hwbreak extension bool swbreak_supported_; // client supports swbreak extension bool list_threads_in_stop_reply_; // client requested threads: and thread-pcs: in stop replies + std::unique_ptr target_description; }; } // namespace rr diff --git a/src/TargetDescription.cc b/src/TargetDescription.cc new file mode 100644 index 00000000000..900be1d36de --- /dev/null +++ b/src/TargetDescription.cc @@ -0,0 +1,132 @@ +/* -*- Mode: C++; tab-width: 8; c-basic-offset: 2; indent-tabs-mode: nil; -*- */ + +#include "TargetDescription.h" + +#include + +#include "GdbServerConnection.h" +#include "kernel_abi.h" + +namespace rr { + +class FeatureStream { +public: + string result() { return stream.str(); } + + template + friend FeatureStream& operator<<(FeatureStream& stream, Any any); + +private: + stringstream stream; + const char* arch_prefix; +}; + +template +FeatureStream& operator<<(FeatureStream& stream, Any any) { + stream.stream << any; + return stream; +} + +template <> +FeatureStream& operator<<(FeatureStream& stream, rr::SupportedArch arch) { + stream << ""; + switch (arch) { + case rr::x86: + stream << "i386"; + stream.arch_prefix = "32bit-"; + break; + case rr::x86_64: + stream << "i386:x86-64"; + stream.arch_prefix = "64bit-"; + break; + case rr::aarch64: + stream << "aarch64"; + stream.arch_prefix = "aarch64-"; + break; + } + stream << "\n"; + return stream; +} + +template <> +FeatureStream& operator<<(FeatureStream& stream, TargetFeature feature) { + DEBUG_ASSERT(stream.arch_prefix != nullptr && + "No architecture has been provided to description"); + stream << R"( )" << '\n'; + return stream; +} + +TargetDescription::TargetDescription(rr::SupportedArch arch, + uint32_t cpu_features) + : arch(arch), target_features() { + + // default-assumed registers per arch + switch (arch) { + case rr::x86: + target_features.push_back(TargetFeature::Core); + target_features.push_back(TargetFeature::SSE); + target_features.push_back(TargetFeature::Linux); + break; + case rr::x86_64: + target_features.push_back(TargetFeature::Core); + target_features.push_back(TargetFeature::SSE); + target_features.push_back(TargetFeature::Linux); + target_features.push_back(TargetFeature::Segment); + break; + case rr::aarch64: + target_features.push_back(TargetFeature::Core); + target_features.push_back(TargetFeature::FPU); + break; + } + + if (cpu_features & rr::GdbServerConnection::CPU_AVX) { + DEBUG_ASSERT((arch == rr::x86 || arch == rr::x86_64) && "unexpected arch"); + target_features.push_back(TargetFeature::AVX); + } + + if (cpu_features & rr::GdbServerConnection::CPU_PKU) { + DEBUG_ASSERT((arch == rr::x86 || arch == rr::x86_64) && "unexpected arch"); + target_features.push_back(TargetFeature::PKeys); + } +} + +static const char header[] = R"( + + +)"; + +string TargetDescription::to_xml() const { + FeatureStream fs; + fs << header << arch << "GNU/Linux\n"; + for (const auto feature : target_features) { + fs << feature; + } + fs << ""; + + return fs.result(); +} +} // namespace rr \ No newline at end of file diff --git a/src/TargetDescription.h b/src/TargetDescription.h new file mode 100644 index 00000000000..f6606bcef8c --- /dev/null +++ b/src/TargetDescription.h @@ -0,0 +1,35 @@ +/* -*- Mode: C++; tab-width: 8; c-basic-offset: 2; indent-tabs-mode: nil; -*- */ + +#ifndef RR_TARGET_DESCRIPTION_H_ +#define RR_TARGET_DESCRIPTION_H_ + +#include + +#include "kernel_abi.h" + +using namespace std; + +namespace rr { + +enum class TargetFeature : uint32_t { + Core = 0, + SSE, + Linux, + Segment, + AVX, + PKeys, + FPU, +}; + +class TargetDescription { +public: + explicit TargetDescription(rr::SupportedArch arch, uint32_t cpu_features); + string to_xml() const; + +private: + SupportedArch arch; + vector target_features; +}; +} // namespace rr + +#endif /* RR_TARGET_DESCRIPTION_H_ */ \ No newline at end of file diff --git a/third-party/gdb/amd64-avx-linux.xml b/third-party/gdb/amd64-avx-linux.xml deleted file mode 100644 index d2dc3bc18f7..00000000000 --- a/third-party/gdb/amd64-avx-linux.xml +++ /dev/null @@ -1,19 +0,0 @@ - - - - - - - - i386:x86-64 - GNU/Linux - - - - - - diff --git a/third-party/gdb/amd64-linux.xml b/third-party/gdb/amd64-linux.xml deleted file mode 100644 index aad02a3ac6f..00000000000 --- a/third-party/gdb/amd64-linux.xml +++ /dev/null @@ -1,19 +0,0 @@ - - - - - - - - i386:x86-64 - GNU/Linux - - - - - - diff --git a/third-party/gdb/amd64-pkeys-linux.xml b/third-party/gdb/amd64-pkeys-linux.xml deleted file mode 100644 index 1fa5bde1161..00000000000 --- a/third-party/gdb/amd64-pkeys-linux.xml +++ /dev/null @@ -1,20 +0,0 @@ - - - - - - - - i386:x86-64 - GNU/Linux - - - - - - - diff --git a/third-party/gdb/i386-avx-linux.xml b/third-party/gdb/i386-avx-linux.xml deleted file mode 100644 index c957fabadd7..00000000000 --- a/third-party/gdb/i386-avx-linux.xml +++ /dev/null @@ -1,18 +0,0 @@ - - - - - - - - i386 - GNU/Linux - - - - - diff --git a/third-party/gdb/i386-linux.xml b/third-party/gdb/i386-linux.xml deleted file mode 100644 index 625984e6d83..00000000000 --- a/third-party/gdb/i386-linux.xml +++ /dev/null @@ -1,18 +0,0 @@ - - - - - - - - i386 - GNU/Linux - - - - - diff --git a/third-party/gdb/i386-pkeys-linux.xml b/third-party/gdb/i386-pkeys-linux.xml deleted file mode 100644 index 47f7b2f0093..00000000000 --- a/third-party/gdb/i386-pkeys-linux.xml +++ /dev/null @@ -1,19 +0,0 @@ - - - - - - - - i386 - GNU/Linux - - - - - -