Avoid throwing exceptions for -fno-exceptions builds

Author: elsteveogrande

libcxx/src/stacktrace/tools/tools.cpp

@@ -48,70 +48,118 @@ _LIBCPP_HIDE_FROM_ABI base::str u64_string(base& base, uintptr_t __val) {
 #  define STRINGIFY0(x) #x
 #  define STRINGIFY(x) STRINGIFY0(x)
 
-void try_tools(base& base, function<bool(tool const&)> cb) {
+bool try_tools(base& base, function<bool(tool const&)> cb) {
   char const* prog_name;
 
   if ((prog_name = getenv("LIBCXX_STACKTRACE_FORCE_LLVM_SYMBOLIZER_PATH"))) {
     if (cb(llvm_symbolizer{base, prog_name})) {
-      return;
+      return true;
     }
   } else {
 #  if defined(LIBCXX_STACKTRACE_FORCE_LLVM_SYMBOLIZER_PATH)
     if (cb(llvm_symbolizer{base, STRINGIFY(LIBCXX_STACKTRACE_FORCE_LLVM_SYMBOLIZER_PATH)})) {
-      return;
+      return true;
     }
 #  else
     if (cb(llvm_symbolizer{base})) {
-      return;
+      return true;
     }
 #  endif
   }
 
   if ((prog_name = getenv("LIBCXX_STACKTRACE_FORCE_GNU_ADDR2LINE_PATH"))) {
     if (cb(addr2line{base, prog_name})) {
-      return;
+      return true;
     }
   } else {
 #  if defined(LIBCXX_STACKTRACE_FORCE_GNU_ADDR2LINE_PATH)
     if (cb(addr2line{base, STRINGIFY(LIBCXX_STACKTRACE_FORCE_GNU_ADDR2LINE_PATH)})) {
-      return;
+      return true;
     }
 #  else
     if (cb(addr2line{base})) {
-      return;
+      return true;
     }
 #  endif
   }
 
   if ((prog_name = getenv("LIBCXX_STACKTRACE_FORCE_APPLE_ATOS_PATH"))) {
     if (cb(atos{base, prog_name})) {
-      return;
+      return true;
     }
   } else {
 #  if defined(LIBCXX_STACKTRACE_FORCE_APPLE_ATOS_PATH)
     if (cb(atos{base, STRINGIFY(LIBCXX_STACKTRACE_FORCE_APPLE_ATOS_PATH)})) {
-      return;
+      return true;
     }
 #  else
     if (cb(atos{base})) {
-      return;
+      return true;
     }
 #  endif
   }
+
+  return false; // nothing succeeded
 }
 
 } // namespace
 
-void spawner::resolve_lines() {
-  try_tools(base_, [&](tool const& prog) {
+bool file_actions::initFileActions() {
+  if (!fa_initialized_) {
+    if (posix_spawn_file_actions_init(&fa_)) {
+      return false;
+    }
+    fa_initialized_ = true;
+  }
+  return true;
+}
+
+file_actions::~file_actions() { posix_spawn_file_actions_destroy(&fa_); }
+
+bool file_actions::addClose(int fd) { return initFileActions() && (posix_spawn_file_actions_addclose(&fa_, fd) == 0); }
+
+bool file_actions::addDup2(int fd, int std_fd) {
+  return initFileActions() && (posix_spawn_file_actions_adddup2(&fa_, fd, std_fd) == 0);
+}
+
+fd file_actions::redirectOutFD() {
+  int fds[2];
+  if (::pipe(fds)) {
+    return {}; // return invalid FD
+  }
+  addClose(fds[0]);
+  addDup2(fds[1], 1);
+  return {fds[0]};
+}
+
+pspawn::~pspawn() {
+  if (pid_) {
+    kill(pid_, SIGTERM);
+    wait();
+  }
+}
+
+bool pspawn::spawn(base::list<base::str> const& argStrings) {
+  base::vec<char const*> argv = tool_.base_.make_vec<char const*>();
+  argv.reserve(argStrings.size() + 1);
+  for (auto const& str : argStrings) {
+    argv.push_back(str.data());
+  }
+  argv.push_back(nullptr);
+  return posix_spawnp(&pid_, argv[0], &fa_.fa_, nullptr, const_cast<char**>(argv.data()), nullptr) == 0;
+}
+
+int pspawn::wait() {
+  int status;
+  waitpid(pid_, &status, 0);
+  return status;
+}
+
+bool spawner::resolve_lines() {
+  return try_tools(base_, [&](tool const& prog) {
     char buf[512];
     pspawn_tool proc(prog, base_, buf, sizeof(buf));
-    try {
-      proc.run();
-      return true;
-    } catch (failed const& failed) {
-    }
-    return false;
+    return proc.run();
   });
 }
 
@@ -170,7 +218,7 @@ Note that this includes an extra empty line as a terminator.
       line.pop_back();
     }
     if (line.empty()) {
-      return;
+      return; // done
     }
     if (!line.starts_with("  ")) {
       // The symbol has no leading whitespace, while the other

libcxx/src/stacktrace/tools/tools.h

@@ -17,7 +17,6 @@
 #include <cstddef>
 #include <cstdlib>
 #include <spawn.h>
-#include <string>
 #include <sys/fcntl.h>
 #include <sys/types.h>
 #include <sys/wait.h>
@@ -27,7 +26,6 @@
 #include <__stacktrace/basic.h>
 #include <__stacktrace/entry.h>
 
-#include "stacktrace/utils/failed.h"
 #include "stacktrace/utils/fd.h"
 
 _LIBCPP_BEGIN_NAMESPACE_STD
@@ -74,73 +72,29 @@ struct atos : tool {
 
 struct file_actions {
   posix_spawn_file_actions_t fa_;
+  bool fa_initialized_{false};
 
-  file_actions() {
-    if (posix_spawn_file_actions_init(&fa_)) {
-      throw failed("posix_spawn_file_actions_init", errno);
-    }
-  }
+  ~file_actions();
 
-  ~file_actions() { posix_spawn_file_actions_destroy(&fa_); }
+  bool initFileActions();
+  bool addClose(int fd);
+  bool addDup2(int fd, int std_fd);
 
-  void addClose(int fd) {
-    if (posix_spawn_file_actions_addclose(&fa_, fd)) {
-      throw failed("posix_spawn_file_actions_addclose", errno);
-    }
-  }
-  void addDup2(int fd, int stdfd) {
-    if (posix_spawn_file_actions_adddup2(&fa_, fd, stdfd)) {
-      throw failed("posix_spawn_file_actions_adddup2", errno);
-    }
-  }
-
-  fd redirectOutFD() {
-    int fds[2];
-    if (::pipe(fds)) {
-      throw failed("pipe", errno);
-    }
-    addClose(fds[0]);
-    addDup2(fds[1], 1);
-    return {fds[0]};
-  }
-
-  void redirectInNull() { addDup2(fd::null_fd(), 0); }
-  void redirectOutNull() { addDup2(fd::null_fd(), 1); }
-  void redirectErrNull() { addDup2(fd::null_fd(), 2); }
+  fd redirectOutFD();
+  bool redirectInNull() { return addDup2(fd::null_fd(), 0); }
+  bool redirectOutNull() { return addDup2(fd::null_fd(), 1); }
+  bool redirectErrNull() { return addDup2(fd::null_fd(), 2); }
 };
 
 struct pspawn {
   tool const& tool_;
   pid_t pid_{0};
   file_actions fa_{};
 
-  // TODO(stacktrace23): ignore SIGCHLD for spawned subprocess
+  ~pspawn();
 
-  ~pspawn() {
-    if (pid_) {
-      kill(pid_, SIGTERM);
-      wait();
-    }
-  }
-
-  void spawn(base::list<base::str> const& argStrings) {
-    base::vec<char const*> argv = tool_.base_.make_vec<char const*>();
-    argv.reserve(argStrings.size() + 1);
-    for (auto const& str : argStrings) {
-      argv.push_back(str.data());
-    }
-    argv.push_back(nullptr);
-    int err;
-    if ((err = posix_spawnp(&pid_, argv[0], &fa_.fa_, nullptr, const_cast<char**>(argv.data()), nullptr))) {
-      throw failed("posix_spawnp", err);
-    }
-  }
-
-  int wait() {
-    int status;
-    waitpid(pid_, &status, 0);
-    return status;
-  }
+  bool spawn(base::list<base::str> const& argStrings);
+  int wait();
 };
 
 struct pspawn_tool : pspawn {
@@ -155,29 +109,36 @@ struct pspawn_tool : pspawn {
     fa_.redirectInNull();
   }
 
-  void run() {
-    // Cannot run "addr2line" or similar without addresses, since we
-    // provide them in argv, and if there are none passed in argv, the
-    // tool will try to read from stdin and hang.
+  bool run() {
+    // Cannot run "addr2line" or similar without addresses, since we provide them in argv,
+    // and if there are none passed in argv, the tool will try to read from stdin and hang.
+    // Nothing to do, so return true for "success".
     if (base_.__entries_.empty()) {
-      return;
+      return true;
+    }
+
+    if (!fd_) {
+      return false;
     }
 
     auto argStrings = tool_.buildArgs(base_);
-    spawn(argStrings);
+    if (!spawn(argStrings)) {
+      return false;
+    }
 
     auto end = base_.__entries_.end();
     auto it  = base_.__entries_.begin();
     while (it != end) {
       auto& entry = (entry_base&)(*it++);
       tool_.parseOutput(base_, entry, stream_);
     }
+    return true;
   }
 };
 
 struct spawner {
   base& base_;
-  void resolve_lines();
+  bool resolve_lines();
 };
 
 } // namespace __stacktrace

libcxx/src/stacktrace/utils/failed.h

@@ -19,19 +19,16 @@
 #include <unistd.h>
 #include <utility>
 
-#include "stacktrace/utils/failed.h"
-
 _LIBCPP_BEGIN_NAMESPACE_STD
 namespace __stacktrace {
 
 /** Encapsulates a plain old file descriptor `int`.  Avoids copies in order to
 force some component to "own" this, although it's freely convertible back to
 integer form.  Default-constructed, closed, and moved-out-of instances will have
 the invalid fd `-1`. */
-class _LIBCPP_HIDE_FROM_ABI fd {
+struct _LIBCPP_HIDE_FROM_ABI fd {
   int fd_{-1};
 
-public:
   fd() : fd(-1) {}
   fd(int fdint) : fd_(fdint) {}
 
@@ -73,20 +70,19 @@ class _LIBCPP_HIDE_FROM_ABI fd {
 };
 
 /** Wraps a readable fd using the `streambuf` interface.  I/O errors arising
-from reading the provided fd will result in a `Failed` being thrown. */
+from reading the provided fd will result in EOF. */
 struct _LIBCPP_HIDE_FROM_ABI fd_streambuf final : std::streambuf {
   fd& fd_;
   char* buf_;
   size_t size_;
+
   _LIBCPP_HIDE_FROM_ABI fd_streambuf(fd& fd, char* buf, size_t size) : fd_(fd), buf_(buf), size_(size) {}
   _LIBCPP_HIDE_FROM_ABI virtual ~fd_streambuf() = default;
 
   _LIBCPP_HIDE_FROM_ABI int underflow() override {
     int bytesRead = ::read(fd_, buf_, size_);
-    if (bytesRead < 0) {
-      throw ::std::__stacktrace::failed("I/O error reading from child process", errno);
-    }
-    if (bytesRead == 0) {
+    if (bytesRead <= 0) {
+      // error or EOF: return eof to stop
       return traits_type::eof();
     }
     setg(buf_, buf_, buf_ + bytesRead);