Page MenuHomePhabricator

[lldb/Plugins] Use external functions to (de)initialize plugins
ClosedPublic

Authored by JDevlieghere on Fri, Feb 7, 11:15 AM.

Details

Summary

This is a step towards making the initialize and terminate calls be generated by CMake, which in turn is towards making it possible to disable plugins at configuration time.

The other alternative discussed was static initializers, but that requires a non-portable linker flag that will prevent the linker from performing dead code elimination.

For this path I just did the ScriptInterpreter plugins. If everyone is on board with the approach I plan to land this in two patches, one with the macros as shown here and another patch where CMake generates the macro expansion for us.

Diff Detail

Event Timeline

JDevlieghere created this revision.Fri, Feb 7, 11:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptFri, Feb 7, 11:15 AM
labath accepted this revision.Fri, Feb 7, 11:52 AM

This looks fine to me. I have one possible simplification idea in an inline comment you can consider...

lldb/include/lldb/Core/PluginManager.h
34–35

no semicolon here. That the macro invocation will be forced to look more like a function call.

lldb/source/API/SystemInitializerFull.cpp
29–32

Fun fact: This declaration is not really needed as the INITIALIZE macros can forward-declare this themselves.

#define LLDB_PLUGIN_INITIALIZE(x) extern void lldb_initialize_##x(); lldb_initialize_##x()

The gotcha here is that the forward declaration will pick up the namespace of the place where the macros are used.
That would be lldb_private in this case, which I don't think is a bad choice, but we can also easily change that by calling the functions from a different namespace.

This doesn't make a big difference in this patch (in fact the separate declaration is probably be better because it's less magical) , but I think it may simplify the process of autogenerating this stuff, since there will only be one insertion point.

This revision is now accepted and ready to land.Fri, Feb 7, 11:52 AM
xiaobai accepted this revision.Fri, Feb 7, 12:51 PM

I like where this is going!

This revision was automatically updated to reflect the committed changes.
JDevlieghere marked an inline comment as done.