Revert "jitlog: use a memfd and glog"

This reverts commit 0aa6ac4e74.

oi/OIDebugger.cpp

@@ -29,7 +29,6 @@
 #include <cerrno>
 #include <chrono>
 #include <cstddef>
-#include <cstdio>
 #include <cstring>
 #include <numeric>
 #include <span>
@@ -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`.
+  // 1. Copy the log file path in out text segment
-  // 2. Duplicate that fd to the debugger using `pidfd_getfd`.
+  // 2. Run the syscall
-  // 3. Store the resulting fds in OIDebugger.
+  // 3. Store the resulting fd in the segmentConfigFile
-  bool ret = true;
+  if (!segConfig.existingConfig) {
+    auto logFilePath =
+        fs::path("/tmp") / ("oid-" + std::to_string(traceePid) + ".jit.log");
 
-  auto traceeFd = remoteSyscall<MemfdCreate>("jit.log", 0);
+    auto logFilePathLen = strlen(logFilePath.c_str()) + 1;
-  if (!traceeFd.has_value()) {
+    if (logFilePathLen > textSegSize) {
-    LOG(ERROR) << "Failed to create memory log file";
+      LOG(ERROR) << "The Log File's path " << logFilePath << " ("
-    return false;
+                 << logFilePathLen << ") is too long for the text segment ("
-  }
+                 << textSegSize << ")";
-  logFds.traceeFd = *traceeFd;
+      return false;
+    }
 
-  auto traceePidFd = syscall(SYS_pidfd_open, traceePid, 0);
+    /*
-  if (traceePidFd == -1) {
+     * Using the text segment to store the path in the remote process' memory.
-    PLOG(ERROR) << "Failed to open child pidfd";
+     * The memory will be re-used anyway and the path will get overwritten.
-    return false;
+     */
-  }
+    if (!writeTargetMemory((void*)logFilePath.c_str(),
-  auto debuggerFd = syscall(SYS_pidfd_getfd, traceePidFd, *traceeFd, 0);
+                           (void*)segConfig.textSegBase,
-  if (close(static_cast<int>(traceePidFd)) != 0) {
+                           logFilePathLen)) {
-    PLOG(ERROR) << "Failed to close pidfd";
+      LOG(ERROR) << "Failed to write Log File's path into target process";
-    ret = false;
+      return false;
-  }
+    }
-  if (debuggerFd == -1) {
+
-    PLOG(ERROR) << "Failed to duplicate child memfd to debugger";
+    /*
-    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<SysOpen>(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<int>(debuggerFd);
+  return true;
-  return ret;
 }
 
 bool OIDebugger::cleanupLogFile(void) {
-  bool ret = true;
+  return remoteSyscall<SysClose>(segConfig.logFile).has_value();
-  if (logFds.traceeFd == -1)
-    return ret;
-
-  if (!remoteSyscall<SysClose>(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;
 }
 
 /* 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<SysFsync>(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) << ")";

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{};
 
   /*

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>;