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