[Lldb-commits] [lldb] 0877dd1 - [Driver] Force llvm to install its handlers before lldb's

[via lldb-commits]

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 at 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 at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits