[Lldb-commits] [PATCH] D63363: [Signals] Create a plugin directory just for signals
Pavel Labath via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Mon Jun 17 12:20:24 PDT 2019
labath added a comment.
In D63363#1546504 <https://reviews.llvm.org/D63363#1546504>, @jfb wrote:
> In D63363#1546490 <https://reviews.llvm.org/D63363#1546490>, @xiaobai wrote:
> > In D63363#1546464 <https://reviews.llvm.org/D63363#1546464>, @jfb wrote:
> > > Can you describe what the goal of your plugin architecture is? Maybe you need an RFC by email before moving stuff around.
> > >
> > > I want to understand what you're going for because as they are today the signals mostly work, and aren't really tested (because injecting these conditions isn't trivial). Anything you change is likely to break some subtle invariant which will only repro when your change is deployed at scale.
> > My goal is to remove non-plugin libraries dependencies on plugins. Today, Target depends on the ProcessUtility plugin because of UnixSignals. If Signals were their own plugin that could be accessed through the PluginManager interface, that dependency would go away. As Pavel said, this feels somewhat over engineered and contrived, but it is the simplest path forward. I am willing to drop this patch and go for a different solution if there is a nicer solution agreed upon by the community, so an RFC sounds like a nice idea. :)
> Removing the dependency seems fine, but we have to be careful with how signals work: they're inherently global state, and you want to register / de-register them exactly where they're registered / de-registered right now to keep them working exactly the same. If you change this, we need to really think through why that's a good idea (it might be!).
I think there's some misunderstading here. This code has nothing to do with signal handlers. All these classes do is describe the set of possible signals on a given target (e.g. if remote-debugging a linux mips machine from a darwin x86 host, we need to know what number does linux use to represent SIGSEGV on mips, etc.)...
CHANGES SINCE LAST ACTION
More information about the lldb-commits