oop redirection in interop + win fix

Author: kr-2003

include/CppInterOp/CppInterOp.h

@@ -668,8 +668,7 @@ CPPINTEROP_API void GetOperator(TCppScope_t scope, Operator op,
 ///\returns nullptr on failure.
 CPPINTEROP_API TInterp_t
 CreateInterpreter(const std::vector<const char*>& Args = {},
-                  const std::vector<const char*>& GpuArgs = {},
-                  int stdin_fd = 0, int stdout_fd = 1, int stderr_fd = 2);
+                  const std::vector<const char*>& GpuArgs = {});
 
 /// Deletes an instance of an interpreter.
 ///\param[in] I - the interpreter to be deleted, if nullptr, deletes the last.
@@ -905,9 +904,11 @@ CPPINTEROP_API void CodeComplete(std::vector<std::string>& Results,
 ///\returns 0 on success, non-zero on failure.
 CPPINTEROP_API int Undo(unsigned N = 1);
 
+#ifndef _WIN32
 /// Returns the process ID of the executor process.
 /// \returns the PID of the executor process.
 CPPINTEROP_API pid_t GetExecutorPID();
+#endif
 
 } // end namespace Cpp
 

lib/CppInterOp/Compatibility.h

@@ -438,9 +438,11 @@ inline void codeComplete(std::vector<std::string>& Results,
 #endif
 }
 
+#ifndef _WIN32
 #ifdef CPPINTEROP_VERSION_PATCH
 inline pid_t getExecutorPID() { return /*llvm*/ getLastLaunchedExecutorPID(); }
 #endif
+#endif
 
 } // namespace compat
 

lib/CppInterOp/CppInterOp.cpp

@@ -63,20 +63,23 @@
 #include <sstream>
 #include <stack>
 #include <string>
+#ifndef _WIN32
+#include <unistd.h>
+#endif
 
 // Stream redirect.
 #ifdef _WIN32
 #include <io.h>
 #ifndef STDOUT_FILENO
 #define STDOUT_FILENO 1
+#define STDERR_FILENO 2
 // For exec().
 #include <stdio.h>
 #define popen(x, y) (_popen(x, y))
 #define pclose (_pclose)
 #endif
 #else
 #include <dlfcn.h>
-#include <unistd.h>
 #endif // WIN32
 
 namespace Cpp {
@@ -3060,9 +3063,7 @@ static std::string MakeResourcesPath() {
 } // namespace
 
 TInterp_t CreateInterpreter(const std::vector<const char*>& Args /*={}*/,
-                            const std::vector<const char*>& GpuArgs /*={}*/,
-                            int stdin_fd /*=0*/, int stdout_fd /*=1*/,
-                            int stderr_fd /*=2*/) {
+                            const std::vector<const char*>& GpuArgs /*={}*/) {
   std::string MainExecutableName = sys::fs::getMainExecutable(nullptr, nullptr);
   std::string ResourceDir = MakeResourcesPath();
   std::vector<const char*> ClingArgv = {"-resource-dir", ResourceDir.c_str(),
@@ -3107,7 +3108,7 @@ TInterp_t CreateInterpreter(const std::vector<const char*>& Args /*={}*/,
 #else
   auto Interp = compat::Interpreter::create(
       static_cast<int>(ClingArgv.size()), ClingArgv.data(), nullptr, {},
-      nullptr, true, stdin_fd, stdout_fd, stderr_fd);
+      nullptr, true);
   if (!Interp)
     return nullptr;
   auto* I = Interp.release();
@@ -3831,12 +3832,12 @@ void Destruct(TCppObject_t This, TCppScope_t scope, bool withFree /*=true*/,
 }
 
 class StreamCaptureInfo {
-  struct file_deleter {
-    void operator()(FILE* fp) { pclose(fp); }
-  };
-  std::unique_ptr<FILE, file_deleter> m_TempFile;
+
+  FILE* m_TempFile;
   int m_FD = -1;
   int m_DupFD = -1;
+  int mode = -1;
+  bool m_OwnsFile = false;
 
 public:
 #ifdef _MSC_VER
@@ -3851,28 +3852,54 @@ class StreamCaptureInfo {
         }()},
         m_FD(FD) {
 #else
-  StreamCaptureInfo(int FD) : m_TempFile{tmpfile()}, m_FD(FD) {
+  StreamCaptureInfo(int FD): mode(FD) {
 #endif
+    auto& I = getInterp();
+    if(I.isOutOfProcess() && FD == STDOUT_FILENO) {
+      m_TempFile = I.getTempFileForOOP(FD);
+      ::fflush(m_TempFile);
+      m_FD = fileno(m_TempFile);
+      m_OwnsFile = false;
+    } else {
+      m_TempFile = tmpfile();
+      m_FD = FD;
+      m_OwnsFile = true;
+    }
     if (!m_TempFile) {
       perror("StreamCaptureInfo: Unable to create temp file");
       return;
     }
 
-    m_DupFD = dup(FD);
+    m_DupFD = dup(m_FD);
 
     // Flush now or can drop the buffer when dup2 is called with Fd later.
     // This seems only necessary when piping stdout or stderr, but do it
     // for ttys to avoid over complicated code for minimal benefit.
-    ::fflush(FD == STDOUT_FILENO ? stdout : stderr);
-    if (dup2(fileno(m_TempFile.get()), FD) < 0)
+    if(m_FD == STDOUT_FILENO) {
+      ::fflush(stdout);
+    } else if(m_FD == STDERR_FILENO) {
+      ::fflush(stderr);
+    } else {
+#ifndef _WIN32
+      fsync(m_FD);
+#endif
+    }
+    // ::fflush(FD == STDOUT_FILENO ? stdout : stderr);
+    if (dup2(fileno(m_TempFile), m_FD) < 0)
       perror("StreamCaptureInfo:");
+    
   }
   StreamCaptureInfo(const StreamCaptureInfo&) = delete;
   StreamCaptureInfo& operator=(const StreamCaptureInfo&) = delete;
   StreamCaptureInfo(StreamCaptureInfo&&) = delete;
   StreamCaptureInfo& operator=(StreamCaptureInfo&&) = delete;
 
-  ~StreamCaptureInfo() { assert(m_DupFD == -1 && "Captured output not used?"); }
+  ~StreamCaptureInfo() { 
+    assert(m_DupFD == -1 && "Captured output not used?"); 
+    if(m_TempFile && m_OwnsFile) {
+      fclose(m_TempFile);
+    }
+  }
 
   std::string GetCapturedString() {
     assert(m_DupFD != -1 && "Multiple calls to GetCapturedString");
@@ -3881,32 +3908,39 @@ class StreamCaptureInfo {
     if (dup2(m_DupFD, m_FD) < 0)
       perror("StreamCaptureInfo:");
     // Go to the end of the file.
-    if (fseek(m_TempFile.get(), 0L, SEEK_END) != 0)
+    if (fseek(m_TempFile, 0L, SEEK_END) != 0)
       perror("StreamCaptureInfo:");
 
     // Get the size of the file.
-    long bufsize = ftell(m_TempFile.get());
+    long bufsize = ftell(m_TempFile);
     if (bufsize == -1)
       perror("StreamCaptureInfo:");
 
     // Allocate our buffer to that size.
     std::unique_ptr<char[]> content(new char[bufsize + 1]);
 
     // Go back to the start of the file.
-    if (fseek(m_TempFile.get(), 0L, SEEK_SET) != 0)
+    if (fseek(m_TempFile, 0L, SEEK_SET) != 0)
       perror("StreamCaptureInfo:");
 
     // Read the entire file into memory.
     size_t newLen =
-        fread(content.get(), sizeof(char), bufsize, m_TempFile.get());
-    if (ferror(m_TempFile.get()) != 0)
+        fread(content.get(), sizeof(char), bufsize, m_TempFile);
+    if (ferror(m_TempFile) != 0)
       fputs("Error reading file", stderr);
     else
       content[newLen++] = '\0'; // Just to be safe.
 
     std::string result = content.get();
     close(m_DupFD);
     m_DupFD = -1;
+    auto& I = getInterp();
+    if (I.isOutOfProcess() && mode != STDERR_FILENO) {
+      if (ftruncate(m_FD, 0) != 0)
+        perror("ftruncate");
+      if (lseek(m_FD, 0, SEEK_SET) == -1)
+        perror("lseek");
+    }
     return result;
   }
 };
@@ -3946,12 +3980,14 @@ int Undo(unsigned N) {
 #endif
 }
 
+#ifndef _WIN32
 pid_t GetExecutorPID() { 
 #ifdef CPPINTEROP_VERSION_PATCH
   return compat::getExecutorPID();
 #endif
   return -1;
 }
+#endif
 
 
 } // end namespace Cpp

lib/CppInterOp/CppInterOpInterpreter.h

@@ -40,9 +40,13 @@
 #include "llvm/TargetParser/Triple.h"
 
 #include <fcntl.h>
+#ifndef _WIN32
 #include <unistd.h>
+#endif
 #include <utility>
 #include <vector>
+#include <stdio.h>
+#include <iostream>
 
 namespace clang {
 class CompilerInstance;
@@ -147,20 +151,80 @@ class Interpreter {
 
   Interpreter(std::unique_ptr<clang::Interpreter> CI) : inner(std::move(CI)) {}
 
+public:
+  struct FileDeleter {
+      void operator()(FILE* f) {
+          if (f) fclose(f);
+      }
+  };
+
+private:
+  static std::unique_ptr<FILE, FileDeleter>& getStdinFile() {
+    static std::unique_ptr<FILE, FileDeleter> stdin_file = nullptr;
+    return stdin_file;
+  }
+  
+  static std::unique_ptr<FILE, FileDeleter>& getStdoutFile() {
+    static std::unique_ptr<FILE, FileDeleter> stdout_file = nullptr;
+    return stdout_file;
+  }
+  
+  static std::unique_ptr<FILE, FileDeleter>& getStderrFile() {
+    static std::unique_ptr<FILE, FileDeleter> stderr_file = nullptr;
+    return stderr_file;
+  }
+  
+  static bool& getOutOfProcess() {
+    static bool outOfProcess = false;
+    return outOfProcess;
+  }
+
+  static bool initializeTempFiles() {
+    getStdinFile().reset(tmpfile());
+    getStdoutFile().reset(tmpfile());
+    getStderrFile().reset(tmpfile());
+    
+    return getStdinFile() && getStdoutFile() && getStderrFile();
+  }
+
+
 public:
   static std::unique_ptr<Interpreter>
   create(int argc, const char* const* argv, const char* llvmdir = nullptr,
          const std::vector<std::shared_ptr<clang::ModuleFileExtension>>&
              moduleExtensions = {},
-         void* extraLibHandle = nullptr, bool noRuntime = true,
-         int stdin_fd = 0, int stdout_fd = 1, int stderr_fd = 2) {
+         void* extraLibHandle = nullptr, bool noRuntime = true) {
     // Initialize all targets (required for device offloading)
     llvm::InitializeAllTargetInfos();
     llvm::InitializeAllTargets();
     llvm::InitializeAllTargetMCs();
     llvm::InitializeAllAsmPrinters();
 
     std::vector<const char*> vargs(argv + 1, argv + argc);
+
+#if defined(_WIN32)
+      getOutOfProcess() = false;
+#else
+      getOutOfProcess() =
+          std::any_of(vargs.begin(), vargs.end(), [](const char* arg) {
+            return llvm::StringRef(arg).trim() == "--use-oop-jit";
+          });
+      int stdin_fd = 0;
+      int stdout_fd = 1;
+      int stderr_fd = 2;
+      if(getOutOfProcess()) {
+        bool init = initializeTempFiles();
+        if(!init) {
+          llvm::errs() << "Can't start out-of-process JIT execution. Continuing with in-process JIT execution.\n";
+          getOutOfProcess() = false;
+        } else {
+          stdin_fd = fileno(getStdinFile().get());
+          stdout_fd = fileno(getStdoutFile().get());
+          stderr_fd = fileno(getStderrFile().get());
+        }
+      }
+#endif
+
     auto CI =
         compat::createClangInterpreter(vargs, stdin_fd, stdout_fd, stderr_fd);
     if (!CI) {
@@ -176,6 +240,24 @@ class Interpreter {
   operator const clang::Interpreter&() const { return *inner; }
   operator clang::Interpreter&() { return *inner; }
 
+  static bool isOutOfProcess() {
+    return getOutOfProcess();
+  }
+
+  FILE* getTempFileForOOP(int FD) {
+    switch(FD) {
+      case(STDIN_FILENO):
+        return getStdinFile().get();
+      case(STDOUT_FILENO):
+        return getStdoutFile().get();
+      case(STDERR_FILENO):
+        return getStderrFile().get();
+      default:
+        llvm::errs() << "No temp file for the FD\n";
+        return nullptr;
+    }
+  }
+
   ///\brief Describes the return result of the different routines that do the
   /// incremental compilation.
   ///

unittests/CppInterOp/JitTest.cpp

@@ -20,6 +20,9 @@ TEST(JitTest, InsertOrReplaceJitSymbol) {
 #ifdef _WIN32
   GTEST_SKIP() << "Disabled on Windows. Needs fixing.";
 #endif
+  if (TestUtils::g_use_oop_jit) {
+    GTEST_SKIP() << "Test fails for OOP JIT builds";
+  }
   std::vector<Decl*> Decls;
   std::string code = R"(
     extern "C" int printf(const char*,...);
@@ -42,6 +45,9 @@ TEST(JitTest, InsertOrReplaceJitSymbol) {
 TEST(Streams, StreamRedirect) {
   // printf and etc are fine here.
   // NOLINTBEGIN(cppcoreguidelines-pro-type-vararg)
+  if (TestUtils::g_use_oop_jit) {
+    GTEST_SKIP() << "Test fails for OOP JIT builds";
+  }
   Cpp::BeginStdStreamCapture(Cpp::kStdOut);
   Cpp::BeginStdStreamCapture(Cpp::kStdErr);
   printf("StdOut is captured\n");
@@ -70,3 +76,26 @@ TEST(Streams, StreamRedirect) {
   EXPECT_STREQ(cerrs.c_str(), "Err\nStdErr\n");
   // NOLINTEND(cppcoreguidelines-pro-type-vararg)
 }
+
+TEST(Streams, StreamRedirectJIT) {
+#ifdef EMSCRIPTEN
+  GTEST_SKIP() << "Test fails for Emscipten builds";
+#endif
+  if (llvm::sys::RunningOnValgrind())
+    GTEST_SKIP() << "XFAIL due to Valgrind report";
+#ifdef _WIN32
+  GTEST_SKIP() << "Disabled on Windows. Needs fixing.";
+#endif
+  TestUtils::CreateInterpreter();
+
+  Cpp::BeginStdStreamCapture(Cpp::kStdOut);
+  Cpp::BeginStdStreamCapture(Cpp::kStdErr);
+  Interp->process(R"(
+    #include <iostream>
+    std::cout << "Hello World" << std::endl;
+    )");
+  std::string CapturedStringErr = Cpp::EndStdStreamCapture();
+  std::string CapturedStringOut = Cpp::EndStdStreamCapture();
+
+  EXPECT_STREQ(CapturedStringOut.c_str(), "Hello World\n");
+}