Hey JF, Pavel, We're still seeing crashes due to SIGPIPE on some lldb bots. This workaround in the lldb driver is insufficient, because liblldb.dylib may install its own a fresh set of llvm signal handlers (the `NumRegisteredSignals` global is not shared by all images loaded in a process). Here is a trace that illustrates the issue: https://gist.github.com/vedantk/2d0cc1df9bea9f0fa74ee101d240b82c.
I think we should fix this by changing llvm's default behavior: let's have it ignore SIGPIPE. Driver programs (like clang, dwarfdump, etc.) can then opt-in to exiting when SIGPIPE is received. Wdyt? Some alternatives include: - Add a static initializer to liblldb.dylib that copies the workaround in the driver. This should work fine, but it's a duct tape on top of a bandaid. - Have the dynamic linker coalesce all copies of `NumRegisteredSignals` in a process. This would incur an app launch time hit on iOS (where libllvm is hot code), and it doesn't seem very portable. vedant > On Oct 25, 2019, at 11:19 AM, Vedant Kumar via lldb-commits > <lldb-commits@lists.llvm.org> wrote: > > > Author: Vedant Kumar > Date: 2019-10-25T11:19:10-07:00 > New Revision: 0877dd14e4e85550f8e267b5ceeff1d3409c41ba > > URL: > https://github.com/llvm/llvm-project/commit/0877dd14e4e85550f8e267b5ceeff1d3409c41ba > DIFF: > https://github.com/llvm/llvm-project/commit/0877dd14e4e85550f8e267b5ceeff1d3409c41ba.diff > > LOG: [Driver] Force llvm to install its handlers before lldb's > > Install llvm's signal handlers up front to prevent lldb's handlers from being > ignored. This is (hopefully) a stopgap workaround. > > When lldb invokes an llvm API that installs signal handlers (e.g. > llvm::sys::RemoveFileOnSignal, possibly via a compiler embedded within lldb), > lldb's signal handlers are overriden if llvm is installing its handlers for > the > first time. > > To work around llvm's behavior, force it to install its handlers up front, and > *then* install lldb's handlers. In practice this is used to prevent lldb test > processes from exiting due to IO_ERR when SIGPIPE is received. > > Note that when llvm installs its handlers, it 1) records the old handlers it > replaces and 2) re-installs the old handlers when its new handler is invoked. > That means that a signal not explicitly handled by lldb can fall back to being > handled by llvm's handler the first time it is received, and then by the > default handler the second time it is received. > > Differential Revision: https://reviews.llvm.org/D69403 > > Added: > > > Modified: > lldb/tools/driver/Driver.cpp > > Removed: > > > > ################################################################################ > diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp > index 4a403a7ffb46..8140e2a04c6e 100644 > --- a/lldb/tools/driver/Driver.cpp > +++ b/lldb/tools/driver/Driver.cpp > @@ -845,6 +845,25 @@ int main(int argc, char const *argv[]) > } > SBHostOS::ThreadCreated("<lldb.driver.main-thread>"); > > + // Install llvm's signal handlers up front to prevent lldb's handlers from > + // being ignored. This is (hopefully) a stopgap workaround. > + // > + // When lldb invokes an llvm API that installs signal handlers (e.g. > + // llvm::sys::RemoveFileOnSignal, possibly via a compiler embedded within > + // lldb), lldb's signal handlers are overriden if llvm is installing its > + // handlers for the first time. > + // > + // To work around llvm's behavior, force it to install its handlers up > front, > + // and *then* install lldb's handlers. In practice this is used to prevent > + // lldb test processes from exiting due to IO_ERR when SIGPIPE is received. > + // > + // Note that when llvm installs its handlers, it 1) records the old > handlers > + // it replaces and 2) re-installs the old handlers when its new handler is > + // invoked. That means that a signal not explicitly handled by lldb can > fall > + // back to being handled by llvm's handler the first time it is received, > + // and then by the default handler the second time it is received. > + llvm::sys::AddSignalHandler([](void *) -> void {}, nullptr); > + > signal(SIGINT, sigint_handler); > #if !defined(_MSC_VER) > signal(SIGPIPE, SIG_IGN); > > > > _______________________________________________ > lldb-commits mailing list > lldb-commits@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
