From f83611ba8b1e886b40f6823ed91369fb9b40a574 Mon Sep 17 00:00:00 2001 From: Evan Teran Date: Mon, 18 Mar 2024 23:45:13 -0400 Subject: [PATCH] improving debuggee memory I/O by using direct calls instead of the QFile abstraction new code is simpler and more efficient since it doens't need to seek before reading/writing instead it just uses pread/pwrite which have existed for a long time now. --- .../DebuggerCore/unix/linux/FeatureDetect.cpp | 84 ++++--------------- .../DebuggerCore/unix/linux/PlatformFile.h | 58 +++++++++++++ .../unix/linux/PlatformProcess.cpp | 52 +++--------- .../DebuggerCore/unix/linux/PlatformProcess.h | 7 +- 4 files changed, 89 insertions(+), 112 deletions(-) create mode 100644 plugins/DebuggerCore/unix/linux/PlatformFile.h diff --git a/plugins/DebuggerCore/unix/linux/FeatureDetect.cpp b/plugins/DebuggerCore/unix/linux/FeatureDetect.cpp index 56bef61fd..eb1334115 100644 --- a/plugins/DebuggerCore/unix/linux/FeatureDetect.cpp +++ b/plugins/DebuggerCore/unix/linux/FeatureDetect.cpp @@ -17,12 +17,14 @@ along with this program. If not, see . */ #include "FeatureDetect.h" +#include "PlatformFile.h" #include "edb.h" #include #include #include #include +#include #include #include #include @@ -33,46 +35,6 @@ namespace DebuggerCorePlugin { namespace feature { namespace { -// Custom class to work with files, since various wrappers -// appear to be unreliable to check whether writes succeeded -class File { -public: - explicit File(const std::string &filename) { - fd_ = ::open(filename.c_str(), O_RDWR); - success_ = fd_ != -1; - } - - ssize_t write(const void *buf, size_t count) { - const ssize_t result = ::write(fd_, buf, count); - success_ = result != -1; - return result; - } - - ssize_t read(void *buf, size_t count) { - const ssize_t result = ::read(fd_, buf, count); - success_ = result != -1; - return result; - } - - off_t seekp(size_t offset) { - const off_t result = ::lseek(fd_, offset, SEEK_SET); - success_ = result != -1; - return result; - } - - ~File() { - close(fd_); - } - - explicit operator bool() { - return success_; - } - -private: - int fd_ = -1; - bool success_ = false; -}; - /** * @brief kill_child * @param pid @@ -128,7 +90,10 @@ bool detect_proc_access(bool *read_broken, bool *write_broken) { return false; } - File file("/proc/" + std::to_string(pid) + "/mem"); + char path[PATH_MAX]; + snprintf(path, sizeof(path), "/proc/%d/mem", pid); + + PlatformFile file(path); if (!file) { perror("failed to open memory file"); kill_child(pid); @@ -137,41 +102,24 @@ bool detect_proc_access(bool *read_broken, bool *write_broken) { const auto pageAlignMask = ~(sysconf(_SC_PAGESIZE) - 1); const auto addr = reinterpret_cast(&edb::v1::debugger_ui) & pageAlignMask; - file.seekp(addr); - if (!file) { - perror("failed to seek to address to read"); - kill_child(pid); - return false; - } int buf = 0x12345678; - { - file.read(&buf, sizeof(buf)); - if (!file) { - *read_broken = true; - *write_broken = true; - kill_child(pid); - return false; - } - } - file.seekp(addr); - if (!file) { - perror("failed to seek to address to write"); + if (file.readAt(&buf, sizeof(buf), addr) == -1) { + *read_broken = true; + *write_broken = true; kill_child(pid); return false; } - { - file.write(&buf, sizeof(buf)); - if (!file) { - *read_broken = false; - *write_broken = true; - } else { - *read_broken = false; - *write_broken = false; - } + if (file.writeAt(&buf, sizeof(buf), addr) == -1) { + *read_broken = false; + *write_broken = true; + } else { + *read_broken = false; + *write_broken = false; } + kill_child(pid); return true; } diff --git a/plugins/DebuggerCore/unix/linux/PlatformFile.h b/plugins/DebuggerCore/unix/linux/PlatformFile.h new file mode 100644 index 000000000..3aad3b921 --- /dev/null +++ b/plugins/DebuggerCore/unix/linux/PlatformFile.h @@ -0,0 +1,58 @@ +/* +Copyright (C) 2024 - 2024 Evan Teran + evan.teran@gmail.com + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation, either version 2 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program. If not, see . +*/ + +#ifndef PLATFORM_FILE_H_20150517_ +#define PLATFORM_FILE_H_20150517_ + +#include +#include +#include +#include +#include + +namespace DebuggerCorePlugin { + +class PlatformFile { +public: + explicit PlatformFile(const char *filename, int flags = O_RDWR) { + fd_ = ::open(filename, flags); + } + + ~PlatformFile() { + ::close(fd_); + } + + ssize_t writeAt(const void *buf, size_t count, off_t offset) { + return ::pwrite(fd_, buf, count, offset); + } + + ssize_t readAt(void *buf, size_t count, off_t offset) { + return ::pread(fd_, buf, count, offset); + } + + explicit operator bool() { + return fd_ != -1; + } + +private: + int fd_ = -1; +}; + +} + +#endif diff --git a/plugins/DebuggerCore/unix/linux/PlatformProcess.cpp b/plugins/DebuggerCore/unix/linux/PlatformProcess.cpp index e1ace05fc..15b968c08 100644 --- a/plugins/DebuggerCore/unix/linux/PlatformProcess.cpp +++ b/plugins/DebuggerCore/unix/linux/PlatformProcess.cpp @@ -34,17 +34,14 @@ along with this program. If not, see . #include "libELF/elf_model.h" #include "linker.h" #include "util/Container.h" - #include #include #include #include #include #include - -#include - #include +#include #include #include #include @@ -222,27 +219,6 @@ QList get_loaded_modules(const IProcess *process) { return ret; } -/** - * seeks memory file to given address, taking possible negativity of the - * address into account - * - * @brief seek_addr - * @param file - * @param address - */ -void seek_addr(QFile &file, edb::address_t address) { - if (address <= UINT64_MAX / 2) { - file.seek(address); - } else { - const int fd = file.handle(); - // Seek in two parts to avoid specifying negative offset: off64_t is a signed type - const off64_t halfAddressTruncated = address >> 1; - lseek64(fd, halfAddressTruncated, SEEK_SET); - const off64_t secondHalfAddress = address - halfAddressTruncated; - lseek64(fd, secondHalfAddress, SEEK_CUR); - } -} - } /** @@ -254,18 +230,15 @@ PlatformProcess::PlatformProcess(DebuggerCore *core, edb::pid_t pid) : core_(core), pid_(pid) { if (!core_->procMemReadBroken_) { - auto memory_file = std::make_shared(QString("/proc/%1/mem").arg(pid_)); - - QIODevice::OpenMode flags = QIODevice::ReadOnly | QIODevice::Unbuffered; + char path[PATH_MAX]; + snprintf(path, sizeof(path), "/proc/%d/mem", pid_); if (!core_->procMemWriteBroken_) { - flags |= QIODevice::WriteOnly; - } - - if (memory_file->open(flags)) { + auto memory_file = std::make_shared(path); + readOnlyMemFile_ = memory_file; + readWriteMemFile_ = memory_file; + } else { + auto memory_file = std::make_shared(path, O_RDONLY); readOnlyMemFile_ = memory_file; - if (!core_->procMemWriteBroken_) { - readWriteMemFile_ = memory_file; - } } } } @@ -303,8 +276,7 @@ std::size_t PlatformProcess::readBytes(edb::address_t address, void *buf, std::s } if (readOnlyMemFile_) { - seek_addr(*readOnlyMemFile_, address); - read = readOnlyMemFile_->read(ptr, 1); + read = readOnlyMemFile_->readAt(ptr, 1, address); if (read == 1) { return 1; } @@ -321,8 +293,7 @@ std::size_t PlatformProcess::readBytes(edb::address_t address, void *buf, std::s } if (readOnlyMemFile_) { - seek_addr(*readOnlyMemFile_, address); - read = readOnlyMemFile_->read(ptr, len); + read = readOnlyMemFile_->readAt(ptr, len, address); if (read == 0 || read == quint64(-1)) { return 0; } @@ -413,8 +384,7 @@ std::size_t PlatformProcess::writeBytes(edb::address_t address, const void *buf, if (len != 0) { if (readWriteMemFile_) { - seek_addr(*readWriteMemFile_, address); - written = readWriteMemFile_->write(reinterpret_cast(buf), len); + written = readWriteMemFile_->writeAt(reinterpret_cast(buf), len, address); if (written == 0 || written == quint64(-1)) { return 0; } diff --git a/plugins/DebuggerCore/unix/linux/PlatformProcess.h b/plugins/DebuggerCore/unix/linux/PlatformProcess.h index 0ec2236b3..19ed8c31f 100644 --- a/plugins/DebuggerCore/unix/linux/PlatformProcess.h +++ b/plugins/DebuggerCore/unix/linux/PlatformProcess.h @@ -20,10 +20,11 @@ along with this program. If not, see . #define PLATFORM_PROCESS_H_20150517_ #include "IProcess.h" +#include "PlatformFile.h" #include "Status.h" #include -#include + namespace DebuggerCorePlugin { @@ -86,8 +87,8 @@ class PlatformProcess final : public IProcess { private: DebuggerCore *core_ = nullptr; edb::pid_t pid_; - std::shared_ptr readOnlyMemFile_; - std::shared_ptr readWriteMemFile_; + std::shared_ptr readOnlyMemFile_; + std::shared_ptr readWriteMemFile_; QMap patches_; QString input_; QString output_;