[Lldb-commits] [lldb] [LLDB][Part 1] Support enabling/disabling InstrumentationRuntime plugins in an debug session (PR #193328)

[Dan Liew via lldb-commits]

delcypher wrote:

@jimingham 

> We use the Dummy Target in lldb to store all the info that would prime a new target (stop-hooks, breakpoints, etc). So you could use the state of the plugin in the Dummy Target to determine is default enablement.
> 

That's interesting I did not know the Dummy Target was a thing. Currently the plugin instances themselves are stored within the Process associated with the Dummy Target. I'm guessing that a Dummy Target doesn't have a Process so we'd probably need to store the enablement of plugins in the Dummy Target itself.

> But I don't think it is required to have a Target domain plugin also exist in the Global domain just so it can have a default enablement.

The other reason I did this is so I could avoid changing too much. The current patch set basically leaves the existing behavior for enabling instrumentation plugins as it is in the `global` domain which is the default behavior of the `plugin enable|disable|list` commands.

> Not sure how much this matters to your overall design.

So I think the end result of my patch set is not ideal

1. `plugin enable --domain debugger instrumentation-runtime.<name>` - Only affects the Targets in the Debugger instances with an alive process. It doesn't change the enablement for targets created after this command is executed. I think this a little unexpected.
2. `plugin enable --domain global instrumentation-runtime.<name>` - Effectively only changes the enablement (i.e. the default) for Targets created after running this command. It doesn't change the enablement in any live processes which is probably a little confusing.

Both of these are direct result of me trying to keep the notion of a global domain around to avoid changing too much in one patch set.

The way I think should probably work is

1. There is no `global` domain for InstrumentationRuntime plugins (we'd have to add a forth domain "auto" which is the default for the `plugin` shell commands and means pick the most specific domain the plugin namespace supports). I think we'd actually want to get rid of the `global` domain **for all** plugins eventually (because its really bad for isolation between Debugger instances) but that seems like a large change that I don't want to do right now.
2. The `InstrumentationRuntime` plugins support only the `debugger` and `target` domains. Trying to use the `global` domain results in an error.
3. `Debugger` instances store the default enablement settings of `InstrumentationRuntime` plugins. These defaults would be used when creating a new Target that belongs to the `Debugger` instance. With this in place it means we could make `plugin enable --domain debugger` also affect the default enablement for targets created later.

The end result of this would be the commands would behave like this:

```
# Uses domain `auto`. If there's a selected target then the domain is `target`,
# otherwise it's `debugger`. This means the same syntax works in `~/.lldb_init` 
# and interactively. If we don't like the idea of context dependent commands 
# we could just make this error and require the user to pick the domain they mean.
plugin enable instrumentation-runtime.<name>

# Changes the enablement in all targets part of this Debugger instance and also sets the default
# for new targets
plugin enable --domain debugger instrumentation-runtime.<name>

# Changes the enablement only in the currently selected target. If there's no target
# or the process is dead this errors.
plugin enable --domain target instrumentation-runtime.<name>
```

If we really wanted to do what I sketched above we basically need to figure how to handle the transition of storing plugin enablement globally to storing it in Debugger instances. I see two ways of doing this:

1. Make a special one-off support in Debugger instances for storing the default enablement of instrumentation runtimes. Not good design but much less disruptive than (2.). We could potentially store the enablement inside the DummyTarget itself as you suggested earlier.
2. Make `PluginManager` non-global and instead make each Debugger instance store its own copy and then re-wire up everything to work with this new model. This sounds like a lot of work. If we did this the `global` domain can be removed completely.

My general thinking around this is I think my patch set has a general enough design that it allows fixing this mess with  default enablement (global vs debugger) instance in a later step. So the way I'd prefer this to go is we land my patch set and then someone else (I don't think I'm qualified) tackles making `PluginManager` non-global.

Does this make sense? Sorry for the long explaination.


https://github.com/llvm/llvm-project/pull/193328