From 8f71efc2d0a3b436a9b137a2f7b1b520d71f73a0 Mon Sep 17 00:00:00 2001 From: Jon Haslam Date: Mon, 20 Nov 2023 18:35:10 +0000 Subject: [PATCH] Revert "jitlog: use a memfd and glog" This reverts commit 0aa6ac4e74b12d00e7eceee54cdb7a2a05141f7f. --- oi/OIDebugger.cpp | 145 +++++++++++++++++++++------------------------- oi/OIDebugger.h | 6 +- oi/Syscall.h | 2 - 3 files changed, 66 insertions(+), 87 deletions(-) diff --git a/oi/OIDebugger.cpp b/oi/OIDebugger.cpp index ef40ab6..07faec6 100644 --- a/oi/OIDebugger.cpp +++ b/oi/OIDebugger.cpp @@ -29,7 +29,6 @@ #include #include #include -#include #include #include #include @@ -196,91 +195,61 @@ bool OIDebugger::singleStepFunc(pid_t pid, uint64_t real_end) { } bool OIDebugger::setupLogFile(void) { - // 1. Open an anonymous memfd in the target with `memfd_create`. - // 2. Duplicate that fd to the debugger using `pidfd_getfd`. - // 3. Store the resulting fds in OIDebugger. - bool ret = true; + // 1. Copy the log file path in out text segment + // 2. Run the syscall + // 3. Store the resulting fd in the segmentConfigFile + if (!segConfig.existingConfig) { + auto logFilePath = + fs::path("/tmp") / ("oid-" + std::to_string(traceePid) + ".jit.log"); - auto traceeFd = remoteSyscall("jit.log", 0); - if (!traceeFd.has_value()) { - LOG(ERROR) << "Failed to create memory log file"; - return false; - } - logFds.traceeFd = *traceeFd; + auto logFilePathLen = strlen(logFilePath.c_str()) + 1; + if (logFilePathLen > textSegSize) { + LOG(ERROR) << "The Log File's path " << logFilePath << " (" + << logFilePathLen << ") is too long for the text segment (" + << textSegSize << ")"; + return false; + } - auto traceePidFd = syscall(SYS_pidfd_open, traceePid, 0); - if (traceePidFd == -1) { - PLOG(ERROR) << "Failed to open child pidfd"; - return false; - } - auto debuggerFd = syscall(SYS_pidfd_getfd, traceePidFd, *traceeFd, 0); - if (close(static_cast(traceePidFd)) != 0) { - PLOG(ERROR) << "Failed to close pidfd"; - ret = false; - } - if (debuggerFd == -1) { - PLOG(ERROR) << "Failed to duplicate child memfd to debugger"; - return false; + /* + * Using the text segment to store the path in the remote process' memory. + * The memory will be re-used anyway and the path will get overwritten. + */ + if (!writeTargetMemory((void*)logFilePath.c_str(), + (void*)segConfig.textSegBase, + logFilePathLen)) { + LOG(ERROR) << "Failed to write Log File's path into target process"; + return false; + } + + /* + * Execute the `open(2)` syscall on the remote process. + * We use the O_SYNC flags to ensure each write we do make it on the disk. + * Another option would have been O_DSYNC, but since the logs get always + * appended to the end of the file, the file size always changes and + * we have to update the metadata everytime anyways. + * So O_SYNC won't incure a performance penalty, compared to O_DSYNC, + * and ensure we can use the metadata for future automation, etc. + */ + auto fd = remoteSyscall(segConfig.textSegBase, // path + O_CREAT | O_APPEND | O_WRONLY, // flags + S_IRUSR | S_IWUSR | S_IRGRP); // mode + if (!fd.has_value()) { + LOG(ERROR) << "Failed to open Log File " << logFilePath; + return false; + } + + segConfig.logFile = *fd; } - logFds.debuggerFd = static_cast(debuggerFd); - return ret; + return true; } bool OIDebugger::cleanupLogFile(void) { - bool ret = true; - if (logFds.traceeFd == -1) - return ret; - - if (!remoteSyscall(logFds.traceeFd).has_value()) { - LOG(ERROR) << "Remote close failed"; - ret = false; - } - - if (logFds.debuggerFd == -1) - return ret; - - FILE* logs = fdopen(logFds.debuggerFd, "r"); - if (logs == NULL) { - PLOG(ERROR) << "Failed to fdopen jitlog"; - return false; - } - if (fseek(logs, 0, SEEK_SET) != 0) { - PLOG(ERROR) << "Failed to fseek jitlog"; - return false; - } - - char* line = nullptr; - size_t read = 0; - VLOG(1) << "Outputting JIT logs:"; - errno = 0; - while ((read = getline(&line, &read, logs)) != (size_t)-1) { - VLOG(1) << "JITLOG: " << line; - } - if (errno) { - PLOG(ERROR) << "getline"; - return false; - } - VLOG(1) << "Finished outputting JIT logs."; - - free(line); - if (fclose(logs) == -1) { - PLOG(ERROR) << "fclose"; - return false; - } - - return ret; + return remoteSyscall(segConfig.logFile).has_value(); } /* Set up traced process results and text segments */ bool OIDebugger::segmentInit(void) { - if (generatorConfig.features[Feature::JitLogging]) { - if (!setupLogFile()) { - LOG(ERROR) << "setUpLogFile failed!!!"; - return false; - } - } - /* * TODO: change this. If setup_results_segment() fails we have to remove * the text segment. @@ -291,6 +260,11 @@ bool OIDebugger::segmentInit(void) { LOG(ERROR) << "setUpSegment failed!!!"; return false; } + + if (!setupLogFile()) { + LOG(ERROR) << "setUpLogFile failed!!!"; + return false; + } } else { if (!unmapSegment(SegType::data)) { LOG(ERROR) << "Failed to unmmap data segment"; @@ -371,7 +345,8 @@ void OIDebugger::createSegmentConfigFile(void) { << " dataSegBase: " << segConfig.dataSegBase << " dataSegSize: " << segConfig.dataSegSize << " replayInstBase: " << segConfig.replayInstBase - << " cookie: " << segConfig.cookie; + << " cookie: " << segConfig.cookie + << " logFile: " << segConfig.logFile; assert(segConfig.existingConfig); } @@ -1792,6 +1767,11 @@ bool OIDebugger::unmapSegments(bool deleteSegConfFile) { ret = false; } + if (ret && !cleanupLogFile()) { + LOG(ERROR) << "Problem closing target process log file"; + ret = false; + } + deleteSegmentConfig(deleteSegConfFile); return ret; @@ -1850,6 +1830,13 @@ bool OIDebugger::removeTraps(pid_t pid) { it = activeTraps.erase(it); } + if (generatorConfig.features[Feature::JitLogging]) { + /* Flush the JIT log, so it's always written on disk at least once */ + if (!remoteSyscall(segConfig.logFile).has_value()) { + LOG(ERROR) << "Failed to flush the JIT Log"; + } + } + /* Resume the main thread now, so it doesn't have to wait on restoreState */ if (!contTargetThread(targetPid)) { return false; @@ -2354,7 +2341,7 @@ bool OIDebugger::compileCode() { } int logFile = - generatorConfig.features[Feature::JitLogging] ? logFds.traceeFd : 0; + generatorConfig.features[Feature::JitLogging] ? segConfig.logFile : 0; if (!writeTargetMemory( &logFile, (void*)syntheticSymbols["logFile"], sizeof(logFile))) { LOG(ERROR) << "Failed to write logFile in probe's cookieValue"; @@ -2424,6 +2411,7 @@ void OIDebugger::restoreState(void) { VLOG(1) << "Couldn't interrupt target pid " << p << " (Reason: " << strerror(errno) << ")"; } + VLOG(1) << "Waiting to stop PID : " << p; if (waitpid(p, 0, WSTOPPED) != p) { @@ -2570,9 +2558,6 @@ void OIDebugger::restoreState(void) { } } - if (!cleanupLogFile()) - LOG(ERROR) << "failed to cleanup log file!"; - if (ptrace(PTRACE_DETACH, p, 0L, 0L) < 0) { LOG(ERROR) << "restoreState Couldn't detach target pid " << p << " (Reason: " << strerror(errno) << ")"; diff --git a/oi/OIDebugger.h b/oi/OIDebugger.h index 793fab1..3997616 100644 --- a/oi/OIDebugger.h +++ b/oi/OIDebugger.h @@ -249,11 +249,6 @@ class OIDebugger { std::filesystem::path segConfigFilePath; std::filesystem::path customCodeFile; - struct { - int traceeFd = -1; - int debuggerFd = -1; - } logFds; - struct c { uintptr_t textSegBase{}; size_t textSegSize{}; @@ -264,6 +259,7 @@ class OIDebugger { uintptr_t dataSegBase{}; size_t dataSegSize{}; uintptr_t cookie{}; + int logFile{}; } segConfig{}; /* diff --git a/oi/Syscall.h b/oi/Syscall.h index 59ce775..521e5ec 100644 --- a/oi/Syscall.h +++ b/oi/Syscall.h @@ -71,8 +71,6 @@ struct Syscall { using SysOpen = Syscall<"open", SYS_open, int, const char*, int, mode_t>; using SysClose = Syscall<"close", SYS_close, int, int>; using SysFsync = Syscall<"fsync", SYS_fsync, int, int>; -using MemfdCreate = - Syscall<"memfd_create", SYS_memfd_create, int, const char*, unsigned int>; using SysMmap = Syscall<"mmap", SYS_mmap, void*, void*, size_t, int, int, int, off_t>;