vsk marked an inline comment as done. vsk added a comment. I admit I hadn't paid enough attention to the order in which signal handlers were registered in lldb.
================ Comment at: lldb/tools/driver/Driver.cpp:850 #if !defined(_MSC_VER) signal(SIGPIPE, SIG_IGN); signal(SIGWINCH, sigwinch_handler); ---------------- labath wrote: > vsk wrote: > > labath wrote: > > > I don't think this line does anything anymore. > > I don't think that's true. LLVM still does `registerHandler(SIGPIPE, > > SignalKind::IsKill)` in `RegisterHandlers`. At least, removing the > > `signal(SIGPIPE, SIG_IGN)` calls causes the unit test to "fail" (i.e. the > > test RUNs but doesn't reach OK). > Ah, right, I see now. This does have a very subtle change in behavior -- it > changes the signal handler that llvm restores *after* the signal has been > received the first time (Signals.inc:355). Normally, it would put SIG_DFL > there (which would kill the process). But with this, it puts SIG_IGN there. > So this changes what we do when we receive SIGPIPE for the *second* time. > > This means that the first SIGPIPE signal is ignored by the virtue of us > calling `SetPipeSignalFunction(nullptr)`, but all subsequent SIGPIPEs are > ignored thanks to this SIG_IGN. It also means this whole talk of "being able > to survive multiple SIGPIPEs" is moot, as llvm does not allow that. In fact > it looks like the very first occurence of SIGPIPE will cause _all_ signal > handlers to be uninstalled. I think this is a very confusing situation to be > in. > > If we don't want to make llvm support handling/ignoring multiple SIGPIPEs > correctly, then I think that a much better solution would be to just work > around this in lldb by making sure the llvm handler is never invoked for > SIGPIPE (this can be arranged by making sure `signal(SIGPIPE, SIG_IGN)` is > called *after* llvm installs its handlers. Point taken. I agree that when a SIGPIPE is received, it doesn't make sense to reset the signal handler state as it was prior to llvm's handler registration. Do you think it'd be reasonable to: - In lldb, force llvm to register its handlers, then immediately ignore SIGPIPE. - Revert the `SetPipeSignalFunction` changes, as they would no longer be required. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69148/new/ https://reviews.llvm.org/D69148 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
