From a571187f94b4bab39666fc3f7085e86268b52849 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 | 42 +++------ src/GdbServerConnection.h | 5 +- src/TargetDescription.cc | 128 ++++++++++++++++++++++++++ src/TargetDescription.h | 27 ++++++ 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, 174 insertions(+), 149 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 0327d605641..6f3170142e8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -629,6 +629,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 @@ -768,12 +769,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 f13046da943..66e1c471abf 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 ca26e932e09..6cf9271b030 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" @@ -111,7 +112,8 @@ 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())); + const auto arch = t->arch(); + dbg->set_cpu_features(arch, get_cpu_features(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_decription->to_xml()); + write_xfer_response(desc.c_str(), desc.size(), offset, len); return false; } @@ -2341,4 +2317,12 @@ 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, + uint32_t features) { + cpu_features_ = features; + DEBUG_ASSERT(target_decription == nullptr && + "Target description already created"); + target_decription = std::make_unique(arch, cpu_features_); +} + } // namespace rr diff --git a/src/GdbServerConnection.h b/src/GdbServerConnection.h index 94a05f9a1c9..3a3add11ed3 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 features); 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_decription{nullptr}; }; } // namespace rr diff --git a/src/TargetDescription.cc b/src/TargetDescription.cc new file mode 100644 index 00000000000..ed01c4e5d7b --- /dev/null +++ b/src/TargetDescription.cc @@ -0,0 +1,128 @@ +#include "TargetDescription.h" +#include "GdbServerConnection.h" +#include "kernel_abi.h" +#include +namespace rr { + +class FeatureStream { + std::stringstream stream; + const char* arch_prefix = nullptr; + +public: + FeatureStream() : stream() {} + // NOTE: This implementation relies on the default copy ctor, + // assignment, etc. + + std::string result() noexcept { return stream.str(); } + + template + friend FeatureStream& operator<<(FeatureStream& stream, Any any) noexcept; +}; + +template +FeatureStream& operator<<(FeatureStream& stream, Any any) noexcept { + stream.stream << any; + return stream; +} + +template <> +FeatureStream& operator<<(FeatureStream& stream, + rr::SupportedArch arch) noexcept { + 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) noexcept { + DEBUG_ASSERT(stream.arch_prefix != nullptr && + "No architecture has been provided to description"); + stream << R"( )" << '\n'; + return stream; +} + +TargetDescription::TargetDescription(rr::SupportedArch arch, + u32 cpu_features) noexcept + : 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); + 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 constexpr auto Header = R"( + + +)"; + +std::string TargetDescription::to_xml() const noexcept { + 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..18c2d753b27 --- /dev/null +++ b/src/TargetDescription.h @@ -0,0 +1,27 @@ +#pragma once +#include "kernel_abi.h" +#include + +namespace rr { + +struct GdbServerRegisterValue; + +using u32 = std::uint32_t; + +enum class TargetFeature : u32 { + Core = 0, + SSE, + Linux, + Segment, + AVX, + PKeys +}; + +class TargetDescription { + SupportedArch arch; + std::vector target_features; +public: + explicit TargetDescription(rr::SupportedArch arch, u32 cpu_features) noexcept; + std::string to_xml() const noexcept; +}; +} \ 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 - - - - - -