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

[Pavel Labath via lldb-commits]

On 08/11/2019 01:33, via lldb-commits wrote:
> 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?

Ah, yes.. I should've realized that wasn't enough.

I agree (and I've alluded to this in the past) that changing the llvm 
behavior is the best option, though ideally, I'd take this a step 
further, and have llvm avoid installing *any* signal handlers by default.

This kind of race is not just limited to SIGPIPE. Other signals suffer 
from the same issues too, it's just that the manifestation of that is 
more subtle.

lldb and liblldb are still racing in who sets the SIGSEGV (etc.) 
handlers last. Since the main effect of those handlers is to produce a 
backtrace and exit, you won't notice this most of the time. But another 
effect of these handlers is to run the RemoveFileOnSignal actions, and 
so the set of files removed might differ since the two lists of files to 
delete are independent too.

(BTW, the fact that this is two copies of llvm racing with each other 
here is unfortunate but irrelevant for this discussion -- the same kind 
of problem would occur if we had llvm and some other entity both trying 
to handle the same signals.)

I think the right behavior for the llvm *library* should be to not 
install *any* signal handlers by default. Instead it should expose some 
API like `runAbnormalExitActions()`, which it's *users* can invoke from 
a signal handler, if they choose so.

Then, the client programs can opt into installing the signal handlers 
and calling this function. This can be done as a part of InitLLVM, and 
it can be the default behavior of InitLLVM. The lldb driver can then 
pass some flag to InitLLVM to disable this handling, or it can just 
continue to override it after the fact, like it does now.


> 
> 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.

A third option might be to link liblldb and the lldb driver to libllvm 
dynamically. That would solve this problem by not having two copies of 
llvm around, but it would also come with some runtime and binary size 
costs...

pl