diff --git a/src/Util.cpp b/src/Util.cpp index 5a1a8375f331..fc4e3a77ab4a 100644 --- a/src/Util.cpp +++ b/src/Util.cpp @@ -9,6 +9,7 @@ #include "Error.h" #include #include +#include #include #include #include @@ -17,10 +18,12 @@ #include #ifdef _MSC_VER +#include #include #include // For _spawnvp #else #include +#include #include #include // For mmap #include @@ -509,6 +512,10 @@ void write_entire_file(const std::string &pathname, const void *source, size_t s } int run_process(std::vector args) { + return run_process(std::move(args), "", ""); +} + +int run_process(std::vector args, const std::string &stdout_path, const std::string &stderr_path) { internal_assert(!args.empty()) << "run_process called with empty args\n"; std::vector argv; @@ -521,12 +528,84 @@ int run_process(std::vector args) { debug(2) << "Running process: " << PrintSpan(args) << "\n"; #ifdef _WIN32 - // Wait for completion; return the child's exit code. + // _spawnvp() has no redirection parameter, so temporarily point the + // calling process's own stdout/stderr at the requested files, spawn + // synchronously, then restore them. This is safe only because the + // call blocks (_P_WAIT) -- there's no window where another thread + // could observe the redirected fds. + int saved_stdout = -1, saved_stderr = -1; + if (!stdout_path.empty()) { + saved_stdout = _dup(_fileno(stdout)); + int fd = _open(stdout_path.c_str(), _O_WRONLY | _O_CREAT | _O_TRUNC | _O_BINARY, _S_IWRITE); + if (fd == -1) { + if (saved_stdout != -1) { + _close(saved_stdout); + } + return -1; + } + _dup2(fd, _fileno(stdout)); + _close(fd); + } + if (!stderr_path.empty()) { + saved_stderr = _dup(_fileno(stderr)); + if (stderr_path == stdout_path) { + // Share the fd stdout was just redirected to, rather than + // opening the same path again: two independent opens would + // give stdout/stderr independent file offsets, so interleaved + // writes would clobber each other instead of concatenating. + _dup2(_fileno(stdout), _fileno(stderr)); + } else { + int fd = _open(stderr_path.c_str(), _O_WRONLY | _O_CREAT | _O_TRUNC | _O_BINARY, _S_IWRITE); + if (fd == -1) { + if (saved_stdout != -1) { + _dup2(saved_stdout, _fileno(stdout)); + _close(saved_stdout); + } + if (saved_stderr != -1) { + _close(saved_stderr); + } + return -1; + } + _dup2(fd, _fileno(stderr)); + _close(fd); + } + } + int rc = _spawnvp(_P_WAIT, argv[0], argv.data()); + + if (saved_stdout != -1) { + _dup2(saved_stdout, _fileno(stdout)); + _close(saved_stdout); + } + if (saved_stderr != -1) { + _dup2(saved_stderr, _fileno(stderr)); + _close(saved_stderr); + } + return (rc >= 0) ? rc : -1; #else + posix_spawn_file_actions_t actions; + posix_spawn_file_actions_init(&actions); + if (!stdout_path.empty()) { + posix_spawn_file_actions_addopen(&actions, STDOUT_FILENO, stdout_path.c_str(), + O_WRONLY | O_CREAT | O_TRUNC, 0644); + } + if (!stderr_path.empty()) { + if (stderr_path == stdout_path) { + // Share stdout's fd (as shell `2>&1` does) instead of opening + // the same path again: two independent opens would give + // stdout/stderr independent file offsets, so interleaved + // writes would clobber each other instead of concatenating. + posix_spawn_file_actions_adddup2(&actions, STDOUT_FILENO, STDERR_FILENO); + } else { + posix_spawn_file_actions_addopen(&actions, STDERR_FILENO, stderr_path.c_str(), + O_WRONLY | O_CREAT | O_TRUNC, 0644); + } + } + pid_t pid = 0; - int status = posix_spawnp(&pid, argv[0], nullptr, nullptr, argv.data(), environ); + int status = posix_spawnp(&pid, argv[0], &actions, nullptr, argv.data(), environ); + posix_spawn_file_actions_destroy(&actions); if (status != 0 || waitpid(pid, &status, 0) == -1) { return -1; } diff --git a/src/Util.h b/src/Util.h index 4de80fb5866a..60d75441c5db 100644 --- a/src/Util.h +++ b/src/Util.h @@ -398,6 +398,12 @@ class TemporaryFile final { * could not be started. */ int run_process(std::vector args); +/** As above, but redirect the child's stdout and/or stderr to the given + * files instead of inheriting the caller's. Pass an empty string for + * either path to leave that stream untouched. Passing the same path for + * both combines them into a single file, as with the shell's `2>&1`. */ +int run_process(std::vector args, const std::string &stdout_path, const std::string &stderr_path); + /** Routines to test if math would overflow for signed integers with * the given number of bits. */ // @{ diff --git a/test/correctness/CMakeLists.txt b/test/correctness/CMakeLists.txt index 48a5c336be9d..ef3c8fa3c67d 100644 --- a/test/correctness/CMakeLists.txt +++ b/test/correctness/CMakeLists.txt @@ -306,6 +306,7 @@ tests( respect_input_constraint_in_bounds_inference.cpp reuse_stack_alloc.cpp round.cpp + run_process.cpp runtime_prefixes.cpp saturating_casts.cpp scatter.cpp diff --git a/test/correctness/run_process.cpp b/test/correctness/run_process.cpp new file mode 100644 index 000000000000..3e5c2534baa7 --- /dev/null +++ b/test/correctness/run_process.cpp @@ -0,0 +1,107 @@ +// Exercises Internal::run_process()'s stdout/stderr redirection overload. +// Each check re-execs this test binary with --child so that the process +// being redirected is a real child process, not just an in-process write. + +#include "Halide.h" + +#include +#include +#include +#include +#include + +#ifdef _MSC_VER +#include +#include +#endif + +using namespace Halide; + +namespace { + +namespace fs = std::filesystem; + +// Return 1 from main() on failure; the test harness treats that as a failure. +// (assert() is compiled out in release builds, so we can't rely on it.) +#define check(cond) \ + do { \ + if (!(cond)) { \ + std::cerr << "FAILED: " #cond " (line " << __LINE__ << ")\n"; \ + return 1; \ + } \ + } while (0) + +constexpr int kChildExitCode = 3; +const std::string kStdoutMarker = "hello from stdout\n"; +const std::string kStderrMarker = "hello from stderr\n"; + +// Child mode: write known markers to stdout and stderr, then exit with a +// known nonzero code. stdout is flushed before writing to stderr (which is +// unbuffered by default) so that when both streams share a single file, the +// resulting order is deterministic. +int run_child() { +#ifdef _MSC_VER + // Markers are compared byte-for-byte against a Unix-style '\n'; without + // this, the CRT's default text-mode translation would write '\r\n' + // instead. + _setmode(_fileno(stdout), _O_BINARY); + _setmode(_fileno(stderr), _O_BINARY); +#endif + std::fputs(kStdoutMarker.c_str(), stdout); + std::fflush(stdout); + std::fputs(kStderrMarker.c_str(), stderr); + return kChildExitCode; +} + +std::string slurp(const std::string &path) { + std::vector data = Internal::read_entire_file(path); + return {data.begin(), data.end()}; +} + +} // namespace + +int main(int argc, char **argv) { + if (argc == 2 && std::string(argv[1]) == "--child") { + return run_child(); + } + + const std::string self = fs::absolute(argv[0]).string(); + + // Separate files for stdout and stderr: each should contain exactly its + // own marker, and the exit code should be preserved. + { + Internal::TemporaryFile out("run_process_out", ".txt"); + Internal::TemporaryFile err("run_process_err", ".txt"); + + int rc = Internal::run_process({self, "--child"}, out.pathname(), err.pathname()); + check(rc == kChildExitCode); + check(slurp(out.pathname()) == kStdoutMarker); + check(slurp(err.pathname()) == kStderrMarker); + } + + // Same file for both streams: the two markers should be concatenated in + // write order, not corrupted or partially overwritten (which is what + // would happen if the same path were opened twice independently instead + // of sharing one fd). + { + Internal::TemporaryFile combined("run_process_combined", ".txt"); + + int rc = Internal::run_process({self, "--child"}, combined.pathname(), combined.pathname()); + check(rc == kChildExitCode); + check(slurp(combined.pathname()) == kStdoutMarker + kStderrMarker); + } + + // Empty paths (including the plain 1-arg overload, which forwards to + // this one) must still work exactly as before: no redirection, just the + // exit code. + { + int rc = Internal::run_process({self, "--child"}, "", ""); + check(rc == kChildExitCode); + + rc = Internal::run_process({self, "--child"}); + check(rc == kChildExitCode); + } + + std::cout << "Success!\n"; + return 0; +}