[PATCH] plugin problem on Windows

With reference to the Bugzilla 9078 issue I posted, it appears the plug-in mechanism doesn’t work on Windows.

To recap, in loading a Windows plug-in DLL, the DLL will have it’s own copy of the clang library code and data, such that the plug-in registry node list ends up in the DLL and not the Clang executable’s data space, hence as-is, the plug-ins node doesn’t end up in the registry instance in Clang.

One way to fix it is to have the plug-ins provide a function hook for retrieving the node. Since the address look-up function in Clang looks in all plug-ins loaded, that means it has to be a uniquely-named function. Therefore, perhaps basing it on the plug-in name is a way to go,

I’ve enclosed a patch that modifies things to do this. It expects the plug-in to export a function with a name like "Get_(plug-in name)Node that returns the node instance. See the modified PrintFunctionNames for an example of this. Since the name for the PrintFunctionNames plug-in is “print-fns”, I have the function lookup code replace ‘-’ with '’ to get a legal C name.

I’ve also modified the example.CMakeLists.txt file to build the example projects as part of the main build.

I also needed some changes in the Registry class to add an AddNode function and expose some privates so the plug-in can get at its node.

I’ve enclosed two patch files, one for the Clang side and one for the LLVM side (the Registry hack).

Permission to check it in? Otherwise, please suggest an alternate fix.

-John

cpluginfix.patch (3.22 KB)

lpluginfix.patch (1.19 KB)

Can’t you weak-link the clang libs to your plugin? That’s how it’s done on OS X I think.

Nico

(phone)

Can’t you weak-link the clang libs to your plugin? That’s how it’s done on OS X I think.

Sorry, I don’t know what that means in the context of Windows. It appears that on Windows, the Clang libraries are all statically linked. Perhaps if the Clang libraries were DLLs, it might work, assuming the DLLs would share the data space as well as the code. I tried re-building with cmake using -DBUILD_SHARED_LIBS=ON, but go lots of errors, and the cmake.html page indicated it didn’t work.

I just realized this would need some more work, because I left in the original code, such that on *nix boxes, the node would be double registered. Anyway, I wanted to open it up for discussion.

Making a big Clang DLL could be an option, but individual subsystem DLLs probably would be a pain, because of all the import/export mixing.

John Thompson
<john.thompson.jtsoftware@gmail.com>
writes:

With reference to the Bugzilla 9078 issue I posted, it appears the plug-in
mechanism doesn't work on Windows.

To recap, in loading a Windows plug-in DLL, the DLL will have it's own copy
of the clang library code and data, such that the plug-in registry node list
ends up in the DLL and not the Clang executable's data space, hence as-is,
the plug-ins node doesn't end up in the registry instance in Clang.

A Windows DLL has a single copy of its code and data for a given
process. But I guess that the problem is that Clang is statically linked
into every plugin. A proper solution for that is to build Clang as a
DLL. You can manually do that on MinGW, but not on VC++.

I've also modified the example.CMakeLists.txt file to build the example
projects as part of the main build.

The `examples' directory is optionally added from the toplevel
CMakeLists.txt file. No need to add it again.

[snip]

Yeah, I tried building a DLL version of Clang using:

cmake -G “Visual Studio 9 2008” -D SHARED_LIBRARY=true .

but Visual Studio 2008 would just hang.

I’m guessing it probably has to do with circular dependencies, which can be problematic on Windows with DLLs. But even if it isn’t, having Clang be a bunch of DLLs probably is not a good idea anyway. Rather than try to figure it out, maybe the cmake build should be redone to directly build a single big DLL. Since I don’t know cmake (yet), I’ll leave this for your comments.

Since I was able to get a plug-in working with Clang not in a DLL, but not without a change to the basic mechanism, would this be a reasonable solution?

Basically, rather than have the plug-in initiate a call to clang, the plug-in exports a function that Clang calls to get the needed registration information.

However, this implies some restrictions on the plug-ins, which I don’t know are acceptible. One is that the plug-in can’t directly access global functions or data in the Clang side; it all has to be through the object tree passed to the plug-in. This also means that virtual functions in the object will run in the context of the Clang side, and non-virtual functions will come from the plug-in side. Another restriction is that you can’t mix memory allocations, i.e. allocate a block from one runtime that is freed by the other. I didn’t seem to run into any of these in the Lua bindings plug-in I did, since it’s purely an output mechanism.

Therefore, which should we do, fixing Clang as a single DLL, or the non-DLL scheme I used?

-John

John Thompson <john.thompson.jtsoftware@gmail.com> writes:

Yeah, I tried building a DLL version of Clang using:

cmake -G "Visual Studio 9 2008" -D SHARED_LIBRARY=true .
but Visual Studio 2008 would just hang.

It is -DBUILD_SHARED_LIBS=ON, not -D SHARED_LIBRARY=true.

Anyways that's pointless because Clang (and LLVM) lacks the necessary
directives (exports file or declspec(dllimport|dllexport) on
declarations.) MinGW can do up to some point without those, because
binutils supports marking everything as exported, but VC++ requires
explicit directives.

I'm guessing it probably has to do with circular dependencies, which can be
problematic on Windows with DLLs. But even if it isn't, having Clang be a
bunch of DLLs probably is not a good idea anyway. Rather than try to figure
it out, maybe the cmake build should be redone to directly build a single
big DLL. Since I don't know cmake (yet), I'll leave this for your comments.

Such "big DLL" would be useless because it exports nothing. Actually,
clang.dll is created by the CMake build for VC++, but only a few
functions are exported. See clang/tools/libclang/libclang.exports.

[snip]

John Thompson <john.thompson.jtsoftware@gmail.com> writes:

Yeah, I tried building a DLL version of Clang using:

cmake -G "Visual Studio 9 2008" -D SHARED_LIBRARY=true .
but Visual Studio 2008 would just hang.

It is -DBUILD_SHARED_LIBS=ON, not -D SHARED_LIBRARY=true.

Anyways that's pointless because Clang (and LLVM) lacks the necessary
directives (exports file or declspec(dllimport|dllexport) on
declarations.) MinGW can do up to some point without those, because
binutils supports marking everything as exported, but VC++ requires
explicit directives.

It's actually possible to do this without explicit directives by
making use of link -dump to dump the symbol table to an export file
and relink all the static libs into a giant DLL that exports
everything. At one point I was working on this, but got pulled into
other things.

- Michael Spencer

Michael Spencer <bigcheesegs@gmail.com> writes:

Anyways that's pointless because Clang (and LLVM) lacks the necessary
directives (exports file or declspec(dllimport|dllexport) on
declarations.) MinGW can do up to some point without those, because
binutils supports marking everything as exported, but VC++ requires
explicit directives.

It's actually possible to do this without explicit directives by
making use of link -dump to dump the symbol table to an export file
and relink all the static libs into a giant DLL that exports
everything. At one point I was working on this, but got pulled into
other things.

There is a limit of 2^16 for the number of symbols you can list on a
export file. I know because you told me that.

I tried to export one library at a time instead of creating a single big
one, but some libraries exceeds the 2^16 limit.

I was actually wrong. There is a 2^16 limit when using indexed exports
(which is much faster at load time). If you don't use them, then the
limit has an upper bound of (2^32) - 1.

- Michael Spencer

Michael Spencer <bigcheesegs@gmail.com> writes:

It's actually possible to do this without explicit directives by
making use of link -dump to dump the symbol table to an export file
and relink all the static libs into a giant DLL that exports
everything. At one point I was working on this, but got pulled into
other things.

There is a limit of 2^16 for the number of symbols you can list on a
export file. I know because you told me that.

I tried to export one library at a time instead of creating a single big
one, but some libraries exceeds the 2^16 limit.

I was actually wrong. There is a 2^16 limit when using indexed exports
(which is much faster at load time). If you don't use them, then the
limit has an upper bound of (2^32) - 1.

What's the difference among indexed and non-indexed exports? The exports
file I created was just a list of symbols and the linker failed
complaining about the 2^16 limit. Then I read some documents on msdn and
the 2^16 limit was mentioned, but nothing about 2^32.

With reference to the Bugzilla 9078 issue I posted, it appears the plug-in mechanism doesn’t work on Windows.

To recap, in loading a Windows plug-in DLL, the DLL will have it’s own copy of the clang library code and data, such that the plug-in registry node list ends up in the DLL and not the Clang executable’s data space, hence as-is, the plug-ins node doesn’t end up in the registry instance in Clang.

One way to fix it is to have the plug-ins provide a function hook for retrieving the node. Since the address look-up function in Clang looks in all plug-ins loaded, that means it has to be a uniquely-named function. Therefore, perhaps basing it on the plug-in name is a way to go,

I’ve enclosed a patch that modifies things to do this. It expects the plug-in to export a function with a name like "Get_(plug-in name)Node that returns the node instance. See the modified PrintFunctionNames for an example of this. Since the name for the PrintFunctionNames plug-in is “print-fns”, I have the function lookup code replace ‘-’ with '’ to get a legal C name.

I’ve also modified the example.CMakeLists.txt file to build the example projects as part of the main build.

I also needed some changes in the Registry class to add an AddNode function and expose some privates so the plug-in can get at its node.

I’ve enclosed two patch files, one for the Clang side and one for the LLVM side (the Registry hack).

Permission to check it in? Otherwise, please suggest an alternate fix.

I think we should move away from the llvm::Registry mechanism. How about the exported function returns a PluginASTAction*, we store all PluginASTAction* pointers that the plugins return and iterate over them when invoking plugin actions.

-Argyrios

Óscar,

It is -DBUILD_SHARED_LIBS=ON, not -D SHARED_LIBRARY=true.

Oh sorry, I was typing from memory. But that was one of the iterations I tried, and just now retried, with the same hang. So you are not seeing the hang when trying to build any of the generated projects?

BTW, we could really use some extension to the documentation for the options available in the cmake stuff.

Michael,

It’s actually possible to do this without explicit directives by
making use of link -dump to dump the symbol table to an export file
and relink all the static libs into a giant DLL that exports
everything. At one point I was working on this, but got pulled into
other things.

I don’t see a -dump option for the Visual Studio 2008 linker in the docs. Did you mean the /MAP option?

Do you have a way to automate the exporting? I’d like to find out more.

Óscar,

What’s the difference among indexed and non-indexed exports? The exports
file I created was just a list of symbols and the linker failed
complaining about the 2^16 limit. Then I read some documents on msdn and
the 2^16 limit was mentioned, but nothing about 2^32.

By “indexed” I assume you mean “ordinal”.

My understanding is that the exported symbols can be referenced by ordinal or name, the ordinal being a carry-over from the old days for saving space. Looking at the .def file docs, I see that under the “EXPORTS” section you can do either:

ExportNameA @1
ExportNameB

where the first line is for an ordinal export, and the second is a named export. I’m wondering if the same limit applies to the named exports. I would gues not.

In the past, I’ve always just used the __declspec(export) and __declspec(import) directives in the headers for DLLs. Basically you put something like the following in a main header for the library, for example, for MyLibrary:

#if defined(DLL)
#if defined(MYLIBRARY_EXPORTS) || defined(BIG_DLL_EXPORTS)
#define MYLIBRARY_LINKAGE __declspec(dllexport)
#else // client sees this
#define MYLIBRARY_LINKAGE __declspec(dllimport)
#endif
#else // assume static library linkage
#define MYLIBRARY_LINKAGE
#endif

You define MYLIBRARY_EXPORTS in the project file for the library (or BIG_DLL_EXPORTS for a single big DLL), and then you use the linkage symbol in the class or function declarations that should be exported, i.e.:

class MYLIBRARY_LINKAGE MyClass
{
// All class functions will be exported.
};

MYLIBRARY_LINKAGE void MyFunction();

I’d be willing to take on this task, if this is the way we want to handle it. If we can’t automate the .def file creation, this would probably be the better option.

But I would like to hear of other ways too.

It seems trying to get Clang as individual DLLs probably wouldn’t work. Creating a big CMake file that builds all the sources into a single DLL would probably be a pain to maintain. Perhaps a CMake file that turns on the __declspec directives in the static libraries but still using he current CMake files, and then builds the DLL from the static libraries is the way to go.

Or, instead of using __declspec, have the individual static libary builds output a .map file, and use a custom tool that pulls out the exports and creates a .def file. (Was that what you we working on, Michael?) It seems like there ought to be an option for just exporting all globals.

Argyrios,

I think we should move away from the llvm::Registry mechanism. How about the exported function returns a PluginASTAction*, we store all PluginASTAction* pointers that the plugins return and iterate over them when invoking plugin actions.

Sounds good to me. But if not, at least fix the Registry mechanism by adding the needed accessors, which curiously it doesn’t have, which is what I tried to do in my patch.

-John

John Thompson <john.thompson.jtsoftware@gmail.com> writes:

It is -DBUILD_SHARED_LIBS=ON, not -D SHARED_LIBRARY=true.

Oh sorry, I was typing from memory. But that was one of the iterations I
tried, and just now retried, with the same hang. So you are not seeing the
hang when trying to build any of the generated projects?

BUILD_SHARED_LIBS is not supported on Windows, so there is no point on
trying it.

BTW, we could really use some extension to the documentation for the options
available in the cmake stuff.

Like this?

http://www.llvm.org/docs/CMake.html#freccmake

What's the difference among indexed and non-indexed exports? The exports
file I created was just a list of symbols and the linker failed
complaining about the 2^16 limit. Then I read some documents on msdn and
the 2^16 limit was mentioned, but nothing about 2^32.

By "indexed" I assume you mean "ordinal".

That was my guess, but it failed without the ordinal too.

[snip]

In the past, I've always just used the __declspec(export) and
__declspec(import) directives in the headers for DLLs. Basically you put
something like the following in a main header for the library, for example,
for MyLibrary:

[snip]

I'd be willing to take on this task, if this is the way we want to handle
it. If we can't automate the .def file creation, this would probably be the
better option.

My advice is to describe your intent on the llvm-commits mailing list,
showing an example, and ask if it acceptable.

[snip]

Argyrios,

I think we should move away from the llvm::Registry mechanism. How about the exported function returns a PluginASTAction*, we store all PluginASTAction* pointers that the plugins return and iterate over them when invoking plugin actions.

Sounds good to me. But if not, at least fix the Registry mechanism by adding the needed accessors, which curiously it doesn’t have, which is what I tried to do in my patch.

Well your patch pokes at and exposes, IMO, implementation details of Registry; it doesn’t seem the right way to go about it…

Maybe it's worth to expose this option to be available e.g. for configuration via
ccmake?

Óscar,

Like this?

http://www.llvm.org/docs/CMake.html#freccmake

My apologies. Seeing it, I now remember to have seen it before. I added a link to it in the Clang Getting Started page also.

That was my guess, but it failed without the ordinal too.

Then that’s probably a vote for the __declspec mechanism.

My advice is to describe your intent on the llvm-commits mailing list,
showing an example, and ask if it acceptable.

Okay, I’ll do that. But I think I’ll experiment a little with it first, i.e. building a big DLL with a few classes and functions exported, to make sure it works.

Argyrios,

Well your patch pokes at and exposes, IMO, implementation details of Registry; it doesn’t seem the right way to go about it…

Okay. Would you like me to have a go at it, or were you thinking to do it?

Thanks.

-John

Argyrios,

Well your patch pokes at and exposes, IMO, implementation details of Registry; it doesn’t seem the right way to go about it…

Okay. Would you like me to have a go at it, or were you thinking to do it?

Please have a go, you’ll be able to make sure that it works on Windows, unlike if I try.

Hi Argyrios,

I’ve attached a couple of patches for a dumbed-down plugin registration mechanism, for review.

The cpluginreg patch has most of it for the Clang side.

The lpluginreg patch, for the LLVM side, just adds a couple of preprocessor symbols for wrapping the DLL imports/exports for Windows. I wasn’t really sure where to put them, as there doesn’t seem to be a common header always included. Because they might be involved in the DLL work I plan to do next, it seemed to make sense to put them in the LLVM side for now. Let me know if there is a better place.

I tested this on both Windows and Linux with the print-fns plugin.

-John

lpluginreg.patch (851 Bytes)

cpluginreg.patch (5.49 KB)