improving debuggee memory I/O by using direct calls instead of the QF…

…ile 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.

plugins/DebuggerCore/unix/linux/FeatureDetect.cpp

@@ -17,12 +17,14 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
 #include "FeatureDetect.h"
+#include "PlatformFile.h"
 #include "edb.h"
 
 #include <cstdint>
 #include <fcntl.h>
 #include <iomanip>
 #include <iostream>
+#include <linux/limits.h>
 #include <string>
 #include <sys/ptrace.h>
 #include <sys/types.h>
@@ -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<uintptr_t>(&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;
 	}

plugins/DebuggerCore/unix/linux/PlatformFile.h

@@ -0,0 +1,58 @@
+/*
+Copyright (C) 2024 - 2024 Evan Teran
+						  [email protected]
+
+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 <http://www.gnu.org/licenses/>.
+*/
+
+#ifndef PLATFORM_FILE_H_20150517_
+#define PLATFORM_FILE_H_20150517_
+
+#include <fcntl.h>
+#include <string>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+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

plugins/DebuggerCore/unix/linux/PlatformProcess.cpp

@@ -34,17 +34,14 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include "libELF/elf_model.h"
 #include "linker.h"
 #include "util/Container.h"
-
 #include <QByteArray>
 #include <QDateTime>
 #include <QDebug>
 #include <QFile>
 #include <QFileInfo>
 #include <QTextStream>
-
-#include <fstream>
-
 #include <elf.h>
+#include <fstream>
 #include <linux/limits.h>
 #include <pwd.h>
 #include <sys/mman.h>
@@ -222,27 +219,6 @@ QList<Module> 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<QFile>(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<PlatformFile>(path);
+			readOnlyMemFile_  = memory_file;
+			readWriteMemFile_ = memory_file;
+		} else {
+			auto memory_file = std::make_shared<PlatformFile>(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<const char *>(buf), len);
+			written = readWriteMemFile_->writeAt(reinterpret_cast<const char *>(buf), len, address);
 			if (written == 0 || written == quint64(-1)) {
 				return 0;
 			}

plugins/DebuggerCore/unix/linux/PlatformProcess.h

@@ -20,10 +20,11 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #define PLATFORM_PROCESS_H_20150517_
 
 #include "IProcess.h"
+#include "PlatformFile.h"
 #include "Status.h"
 
 #include <QCoreApplication>
-#include <QFile>
+
 
 namespace DebuggerCorePlugin {
 
@@ -86,8 +87,8 @@ class PlatformProcess final : public IProcess {
 private:
 	DebuggerCore *core_ = nullptr;
 	edb::pid_t pid_;
-	std::shared_ptr<QFile> readOnlyMemFile_;
-	std::shared_ptr<QFile> readWriteMemFile_;
+	std::shared_ptr<PlatformFile> readOnlyMemFile_;
+	std::shared_ptr<PlatformFile> readWriteMemFile_;
 	QMap<edb::address_t, Patch> patches_;
 	QString input_;
 	QString output_;