[cfe-dev] Big Clang DLL, plug-in mechanism revision

[Nico Weber]

On Mon, May 9, 2011 at 12:45 PM, Argyrios Kyrtzidis <kyrtzidis at apple.com>wrote:

> I agree with Anton and personally am not too fond of dllimport/dllexport.
> Could we live without the big Clang DLL and just get rid of the current
> static plug-in registration, replacing it with explicit plugin classes with
> virtual functions (whose objects get created in the plugin) ?
>
> Also stepping back a bit, there's a "tooling" effort going on that makes it
> easier to build custom tools without needing plugins, isn't this a better
> way to go ?
>

We're using a plugin that runs in addition to normal codegen on the chrome
buildbots (the -add-plugin flag). It outputs additional warnings (
http://www.chromium.org/developers/coding-style/chromium-style-checker-errorshas
a list if you're curious). Running with the plugin slows down our
build
by a few seconds (a full build is 40 minutes, so it's a very small
percentage). Doing this with the tooling stuff would be both slower and more
annoying. TL;DR: I think the plugin stuff is probably useful even when the
tooling stuff is there.


> Keep in mind that dynamically loading plugins is of reduced usefulness
> since we don't have a stable API.
>

(I'm fine with breaking changes to the plugin api.)


>
> -Argyrios
>
> On May 9, 2011, at 11:57 AM, John Thompson wrote:
>
> My collegue suggested that a clearer statement of the problems to be solved
> might help, since overall it was kind of spread out over a few threads over
> a couple of months.
>
> 1. Plug-in registration.  The current plug-in mechanism doesn't work on
> Windows.  Shared libraries work differently on Windows, such that building a
> plug-in DLL on windows doesn't create a connection to the plug-in registry
> in the libraries used in the Clang executable.  The result is that in
> running a plug-in like the PrintFunctionNames plug-in, there is no output
> becase the plug-in is adding an entry to a registry that exists only in it's
> own data space.
>
> My solution for this was to dumb-down the registry a bit, and to have the
> plug-in export a function to be called that creates and returns the plug-in
> action object.
>
> Even if the big-DLL change in #2 is not acceptable, I strongly need the
> plug-in mechanism to work, as I wrote Lua-bindings plug-in, and I need to
> have it work on Windows.  So it's very important to me to either have this
> fix accepted, or get feedback on a more acceptable solution, or have someone
> else fix it.
>
> 2. Big Clang DLL. If you build a plug-in or other shared library on
> Windows, the DLL is much like a position-independent executable in that it
> has it's own code and data sections, with absolute addresses fixed up at
> runtime.  When the DLL runs, it typically will only call versions of code
> and static data that live in the DLL itself.  An exception is if you pass in
> an object with virtual function that is created in a separate executable.
> By means of the vtable, the virtual functions called will be those from the
> executable where the constructor lives.
>
> If it is expected that the DLL will read or make changes to static data not
> explicitly or indirectly passed to it, this will not happen, as the DLL is
> only operating on it's own data space, or the data pointers explicitly
> passed to it.
>
> Fortunately, because both the Clang executable and the plug-ins are created
> using the DLL version of the VC++ runtime, plug-ins that don't rely on
> static data from the main executable can still work (as my Lua bindings
> plug-in does).  This means that values returned from calls like malloc can
> be freed from either the main executable or the DLL.  But I fear that we
> might encounter other plug-ins that won't work properly, without true
> sharing of the common code and data.
>
> Again, I need at least #1, and hope it can go in soon (leaving in the old
> registry for a while until all plug-ins use the new mechanism).  I don't
> personally need #2, but I think it should be done, and I'm willing to do the
> work.
>
> Thanks.
>
> -John
>
> On Wed, May 4, 2011 at 6:06 PM, John Thompson <
> john.thompson.jtsoftware at gmail.com> wrote:
>
>> Hi,
>>
>> May I solicit some feedback on the work I've done for these two issues?
>>
>> Basically I'm attacking the problem from two separate angles, doing the
>> following changes in two separate trees.
>>
>> In one, I revised the plug-in mechanism, dumbing it down a bit, to avoid
>> the problems encountered on Windows where the plug-in registry referenced in
>> the plug-in DLL doesn't connect with the plug-in registry in the clang
>> executable.  I say dumbing it down a bit because it uses a more basic vector
>> of structures for storing the plugin name and a callback function for
>> creating the action node.  The original patch was kind of old, so I updated
>> it, using one new file from the other tree for the __cdeclspec(dllexport)
>> wrapper.  Note that I left in the original plug-in registry stuff
>> side-by-side.  I tested it the earlier version on both Windows and Linux.
>> It still has the potential problem of the plug-in not having access to
>> everything in the clang executable, which can only be resolved by using a
>> big Clang DLL.  Here's the patches for this change:
>>
>> http://www.jtlanguage.com/xfer/plugin_llvm_patch.txt
>> http://www.jtlanguage.com/xfer/plugin_clang_patch.txt
>>
>> In the other, I did the ground work for creating a big Clang DLL from all
>> the LLVM and Clang sources used in the original Clang executable.  It
>> currently only exports a small subset of the functions, mostly only those
>> needed to get the Clang and other executables to link correctly.  I'll recap
>> the scheme later in this email, revising and pasting info from my original
>> messages, with some updates.  The clang.exe executable links and runs,
>> though there is some dependency problem I don't understand yet when doing
>> the "ALL_BUILD".  In running the tests, some of the Index tests fail for
>> unknown reasons.  Otherwise, without the cmake "LLVM_BUILD_DLL" option, the
>> normal build and test results are unchanged.  Yesterday I tested the patch
>> on Linux and fixed a couple of compile problems.  Basically, this shows that
>> the basic big DLL mechanism works, and that all that is needed is to add the
>> rest of the "*_LINKAGE" tags and fix the dependency and Index test
>> problems.  Here's the patches for this change:
>>
>> http://www.jtlanguage.com/xfer/dll_llvm_patch.txt
>> http://www.jtlanguage.com/xfer/dll_clang_patch.txt
>>
>> Note that the big Clang DLL does not automatically fix the original
>> problem of the plug-in registry, as the needed internal registry symbols are
>> not exported.  So the full solution of fixing the plug-in registry mechanism
>> probably requires both of these changes, the first for the registering
>> mechanism, and the second for allowing the plug-in to full access of the
>> Clang code and data.
>>
>> Basically I'm looking for feedback on these two schemes, or a go-ahead
>> with checking in what I have so far, for a staged adoption.  Because the DLL
>> scheme still needs a lot of work in adding the "*_LINKAGE" tags to so many
>> declarations, I want to find out if the owners of the respective systems
>> approve of these changes, rather than continuing with all the work and
>> finding that it will not be used.  I know it's a pretty big change, and
>> Windows-specific, but I think it's important enough to try.
>>
>> I'm thinking I could check in what I have at present (with any changes
>> required), which has the build changes and the new module-common headers,
>> and the subset of "*_LINKAGE" tag additions that let the link succeed, and
>> then check in the remaining changes in smaller batches, as I add the
>> remaining "*_LINKAGE" tags.
>>
>> I'm not sure how the big Clang DLL fits into the long-term scheme of
>> things, whether it remains a special build option on Windows, or becomes the
>> mainstream version.  For now just leave it as a special build option.  I'm
>> also not sure if any other LLVM modules should be in the DLL, and whether
>> there should also be a separate LLVM DLL.  I started doing a little work on
>> this, but didn't include it here.
>>
>> So here's stuff from the original email for the DLL stuff revisions, as I
>> missed a description of something I did on the LLVM side in the cmake files,
>> and I hadn't gotten the unit test mechanism to work at all.  Sorry for the
>> verbosity; I'm just trying to be complete:
>>
>> The scheme is meant to satisfy the following design goals/constraints:
>> 1. Create a single Windows DLL including the main Clang and LLVM modules
>> needed for a compiler and other tools, such that it's the only module you
>> need to link against (from the Clang/LLVM project).
>> 2. Preserve the current module organization and build, leaving the
>> static-library and other platform's shared library versions as they are.
>> (This leaves open the option of creating per-module Windows DLLS in the
>> future.)
>>  3. Enable Clang plug-ins to share the code and data space with the Clang
>> or other tool executables.
>> 4. Make the "big" DLL version optional by means of cmake arguments given
>> upon running cmake. (I'm not sure of the criteria for deciding whether to
>> make it the default for Windows or not.)
>> These are the build changes, with respect to the cmake build tool:
>> 1. A "BUILD_LLVM_DLL" configuration flag is added to the cmake build, such
>> that specifying "-DBUILD_LLVM_DLL=ON" in the cmake command line will enable
>> the big-DLL build. By default it is off.
>>  2. Factor out the common portions of the "add_clang_library" into
>> "add_clang_library_common".
>>
>> 3. Modify the cmake macro "add_clang_library" to collect the source file
>> full path strings in a new target property.
>>
>> 4. Add a new "add_clang_dll" cmake macro for creating the big DLL. This
>> macro operates like "add_clang_library", but always builds a DLL target, and
>> doesn't collect the source paths. It also passes "LLVM_USE_DLL" and
>> "LLVM_EXPORTS" defines to the compiler, to enable the symbol exports to be
>> described later.
>>  5. Create a new "ClangDLL" project under tools/clang/lib for creating
>> the big DLL, which uses the new "add_clang_dll" macro.
>> 6. Add new "set_clang_components" and "set_clang_llvm_components" cmake
>> macros for use in conjunction with "add_clang_executable" to collect the
>> Clang and LLVM components, as opposed to setting "LLVM_USED_LIBS" and
>> "LLVM_LINK_COMPONENTS" directory. (These macros will substitute the big DLL
>> for the component libraries.)
>>  7. Modify "add_clang_executable" to pass an "LLVM_USE_DLL" define to the
>> compiler if the big-DLL version is enabled. This define enables the symbol
>> imports/exports to be described later.
>>
>> 8. (new) Do similar changes to the LLVM-side cmake macros, such as chaning
>> add_llvm_library to collect the sources.
>> 9. Update the clang executable, interpreter, and PrintFunctionNames
>> projects to use the new and revised cmake macros.
>> The source changes mainly consist of adding
>> "(moduleNameUppercase)_LINKAGE" macro instances to the declarations of the
>> classes, functions, and other symbols to be exported/imported, and new
>> header files that define these macros. These changes can be summarized as
>> follows:
>>  1. A new "(moduleName)Common.h" file is added to each LLVM and Clang
>> module include directory. This file defines a
>> "(moduleNameUppercase)_LINKAGE" macro for that component module. This macro
>> expands to nothing for the non-big-DLL version ("LLVM_USE_DLL" is not
>> defined to the compiler), or to the LLVM_EXPORT macro when building the DLL
>> ("Clang_EXPORTS" or "LLVM_EXPORTS" or "(moduleName)_EXPORTS" is defined), or
>> to the LLVM_IMPORT macro when building against the DLL ("LLVM_USE_DLL" is
>> defined). This include file will be included by any headers in the component
>> needing the "(moduleNameUppercase)_LINKAGE" macro.
>>  2. A new "LLVMCommon.h" file is added to the include/llvm directory
>> under the root to define "LLVM_EXPORT" and "LLVM_IMPORT" macros that wrap
>> the Visual C++ "__declspec(dllexport)" and "__declspec(dllimport)"
>> directives, as well as to define the "LLVMCORE_LINKAGE" macro. This file
>> will be included by all the "(moduleName)Common.h" files for the LLVM
>> components.
>>  3. A new "ClangCommon.h" file is added to the tools/clang/include/clang
>> directory. It just includes the "LLVMCommon.h" file, and will be included by
>> the "(moduleName)Common.h" files for the Clang components.
>>  In my experimental version, I added most of the "(moduleName)Common.h"
>> files, but I only added the "(moduleNameUppercase)_LINKAGE" macro instance
>> to some of the symbols, at least the ones needed by the Clang driver and
>> interpreter to link.
>>  Caveats:
>> 1. The ALL_BUILD project build breaks on some missing .inc files from the
>> X86 target project. I think this has to do with an inherent dependency
>> problem in the cmake build. Even though the "(target)CodeGenTable_gen"
>> project dependency is added to the DLL, the .inc files are not created
>> before the DLL link. I will look into this. A rebuild of the "clang" project
>> does work, as that was the focus of my experiment. The interpreter might
>> build too, as it did earlier.
>>  2. Several of the "Index" tests don't pass.  I haven't figured out why
>> yet.
>> 3. As I mentioned, I only exported some of the symbols, those needed to
>> get the clang executables to build. A big edit (manual or automated) will be
>> needed to add the "(moduleNameUppercase)_LINKAGE" macros to the rest of the
>> symbols, and any needed "(moduleName)Common.h" headers not yet created.
>>  4. I only enabled the X86 target in LLVM (the default for the Windows
>> build in general), for the purpose of this experiment. The other targets
>> will need to be done as well.
>> 5. I tried running the PrintFunctionNames plugin, but there's still a
>> disconnect between the registry referenced by the plugin and the one in the
>> big DLL. I didn't look into it deep enough yet, but I'm guessing it has
>> something to do with the template mechanism used not being exported. I'm not
>> even sure how to export it. But I think the better solution is to switch to
>> the simplified plugin mechanism I provided a patch for in a previous post
>> (and which I'm still waiting for a review fornudge-nudge).
>>  6. In building on Windows with the big DLL mechanism enabled, there are
>> a lot of "class 'someTemplateClass<_Ty>' needs to have dll-interface to be
>> used by clients of class 'someClass'" warnings. These just need to have some
>> extra template specialization declarations with the "*_LINKAGE" tag.
>>
>> You can search on "CMakeLists.txt" and ".cmake" to see the cmake file
>> changes, or "Common.h" to see the new headers. Most of the rest of the
>> changes are just the adding of the "(moduleNameUppercase)_LINKAGE" macros to
>> the class and function definitions.
>>
>> Thanks!
>>
>> -John
>>
>> --
>> John Thompson
>> John.Thompson.JTSoftware at gmail.com
>>
>>
>
>
> --
> John Thompson
> John.Thompson.JTSoftware at gmail.com
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110509/55b45c6c/attachment.html>