GNOME Bugzilla – Bug 522697
Patch: make libIDL work with Microsoft's compilers
Last modified: 2008-05-20 07:54:30 UTC
libIDL currently has a makefile that allows one to compile it with Microsoft's compilers. Problem is, it is not entirely correct, and it also seems to be really old, so it doesn't do a few things that are required for newer compilers. I'll attach a patch that does the following: . properly annotate functions and data being exported in the library so that a working import library is created (no need for a .def file) . fix the makefile so it also invokes lex/yacc, and does manifest embedding as required for newer VC compilers (8 and 9). The function annotations would also work for GCC with visibility support (see http://gcc.gnu.org/wiki/Visibility). This is not enabled by default nor there's an option in configure to enable it, because I'm completely unfamiliar with autoconf/automake. It can be enabled by using: CFLAGS="-fvisibility=hidden -DLIBIDL_NO_AUTO_EXPORT" A better name for the define is also welcome.
Created attachment 107368 [details] [review] Patch for MS VC support. The patch shows me deleting a bunch of stuff from libIDL.def, I'm actuall just "svn rm"ing the file since it's not needed anymore. I tried using the autoconf/automake system on Windows but didn't go very far (I managed to compile things after manually tweaking the generated makefiles, but linking was a completely different story). The Makefile.msc approach works fine though.
Created attachment 107372 [details] [review] Updated patch (also changes tstidl.c)
Created attachment 107397 [details] [review] Another updated patch (clean up macros a bit).
Created attachment 107890 [details] [review] Yet another updated patch I've compiled ORBit with this patch, so things seem to be working. This one uses "cl.exe /E" as the preprocessor by default (set in the makefile), and fixes IDL_parse_filename so that it compiles with MSVC properly (there's no need to symlink the .idl file to .idl.c - cl.exe parses the .idl file just fine with the original extension).
As there is a .def file already, wouldn't it be a good idea to update it if necessary, and let that direct what functions gets exported? (It should then be used when linking the DLL both in Makefile.am and Makefile.msc.in.) Then no decorations would be needed for functions in the headers, which is good. (*If* we really would add decorations to the functions in the headers, we might then as well also add the calling convention attribute.) It is more in line with existing practise in the GTK+ stack not to have any dllimport/export decorations in headers for functions. (For variables the situation is different.) What is the __attribute__((visibility("default"))) thing you add in IDL.h.in? That it not related to MSVC, is it? The patch contains some gratuitous whitespace changes, please remove.
The "visibility" thing is for GCC (see http://gcc.gnu.org/wiki/Visibility). I think it's a good idea to have around, even if disabled by default. It provides very similar functionality to the MS import/export annotations, and keeps things consistent between the two ports (i.e., same functions are exported, so if something links on Linux it's very probably it will link on Windows too). It's how I tested things before I had the Win32 env ready, to make sure I had the right things exported. Before I go ahead cleaning other things, are the decorations really that undesirable? I ask for a few reasons: (i) If keeping the GCC visibility support is desired, I'm not familiar with any way to do it other than using the attributes. (ii) Not really an argument for libIDL, but ORBit has a lot of generated code. Keeping the .def files in sync with generated code is quite a pain. So auto-generating annotations like that seems like a much better approach to me (and is what my unsubmitted patches do). (iii) Regardless of whether one prefers annotations or .def files, annotations are necessary for data exports. I don't remember off of my head if libIDL has any of those, but ORBit, especially the generated code, has tons. So, if you already have to generate annotations for the exported data, why not generate for everything (data + functions), instead of having to deal with two different ways of doing it? Annotations can become messy (especially when using MSVC), but I tend to prefer them a lot over .def files whenever possible. For the calling convention, are you referring to __stdcall?
Yes, but patches for visibility restrictions on ELF (and other?) non-Windows platforms hardly belong in this bug, do they? The decorations for functions are undesirable, in my opinion, for two reasons: 1) They make the headers uglier. OK, so this is obviously something that is in the eye of the beholder. Inserting just one relatively short "word" in front of each function declaration is not that bad. 2) Doing this just in libIDL is inconsistent with the rest of the stack. The main use of libIDL is after all in the bonobo-based GNOME stack. You are quite correct that decorations are necessary for data exports (exported variables) when using MSVC. And indeed ORBit2 and idl-compiler-generated code has tons of those. Such decorations for exported variables are not currently there because mingw (the GNU linker) automatically exports all globals if there are no explicit exports, and it knows how to automatically turn plain references to undefined variables into imports of exported variables at link time (auto-import). This is BTW why I am amazed if it really is possible to compile code generated by the ORBit2 idl-compiler such as it is today with MSVC, even if, or actually especially if, one adds the necessary export/import decorations to the variables. At least with mingw, back when I ported that stuff to Wi32 using mingw, if one used export/import decorations for exported variables this caused "initializer is not a constant" errors already when compiling. This is because the idl-compiler generated code generates initializers for static data that effectively want the address of an imported variable, or something like that. I don't recall the details now. If the compiler knows that a variable is imported (i.e. its declaration had the dllimport decoration), then it doesn't allow its address to be taken in a static context. Try compiling (just compile, -c) the following source file test1.c with gcc and MSVC (various versions): extern __declspec(dllimport) int an_int; extern __declspec(dllimport) int an_int_array[]; static int *p1 = &an_int; static int *p2 = an_int_array; static int *p3 = &an_int_array[1]; int main (void) { printf ("%d %d %d\n", *p1, *p2, *p3); return 0; } Then remove the dllimport decorations and it obviously compiles. If the an_int and an_int_array variables then actually are provided in a DLL from a source file test1dll.c: __declspec(dllexport) int an_int = 4; __declspec(dllexport) int an_int_array[] = { 1, 2, 42 }; and you remove the dllexport decorations and compile this into a DLL test1dll.dll with mingw, and then link the test1.o and the import library for test1dll.dll into test1.exe, it will link fine, thanks to the auto-import feature which is turned on by default. You will get informational messages: Info: resolving _an_int by linking to __imp__an_int (auto-import) Info: resolving _an_int_array by linking to __imp__an_int_array (auto-import) But no matter how you tweak it, with dllimport/export decorations or not, you won't be able to compile and link it with the MS toolchain. I would love to be proved wrong, of course. There is another GNU ld flag, --enable-runtime-pseudo-reloc. This flag is not enabled by default. Back when I ported ORBit2 etc, this flag was essential to get the stuff to link. But for some reason, I now can't reproduce that requirement with small test programs like the above. They link fine even without that switch. I haven't yet checked if ORBit2 etc now links also without that flag. (Maybe that runtime-pseudo-reloc features has simply been made the silent default in the GNU ld I now use?)
(In reply to comment #7) > Yes, but patches for visibility restrictions on ELF (and other?) non-Windows > platforms hardly belong in this bug, do they? Since I took the annotation approach for exporting the symbols on Windows, I thought that adding that feature was worth the two extra lines of code. :-) If the annotations are really undesirable, then we wouldn't be able to support it anyway (maybe with ld scripts, which I'm not very familiar with). > 1) They make the headers uglier. While I don't disagree, I think the advantages it brings outweight the minor inconvenience when reading the headers. > 2) Doing this just in libIDL is inconsistent with the rest of the stack. The > main use of libIDL is after all in the bonobo-based GNOME stack. It would be consistent with ORBit, if the same approach is taken (and I don't really see a way around it if MSVC support is desirable). But libIDL is such a small library that it doesn't make a lot of difference in the end. Changing it to the .def file is probably easy. But be aware that this same discussion will be needed also for ORBit, and there I really don't see a way around the problem. > Try compiling (just compile, -c) the following source file test1.c with gcc and > MSVC (various versions): What doesn't work for you? With VC8, that code compiles, and if I create the DLL with the data, it links and runs as expected. We've been using code like that at work for some time now, even when we were using VC 7.1, with no problems.
Created attachment 111079 [details] Zip archive with test1.c, test1dll.c and doit.bat Unpack somewhere and run the doit.bat file. With MSVC8 I get: c:\devel\src\tml\pe-problem\test1>doit c:\devel\src\tml\pe-problem\test1>cl -W3 -MD -LD -Fetest1dll.dll test1dll.c Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 14.00.50727.762 for 80x86 Copyright (C) Microsoft Corporation. All rights reserved. test1dll.c Microsoft (R) Incremental Linker Version 8.00.50727.762 Copyright (C) Microsoft Corporation. All rights reserved. /dll /implib:test1dll.lib /out:test1dll.dll test1dll.obj Creating library test1dll.lib and object test1dll.exp c:\devel\src\tml\pe-problem\test1>cl -W3 -MD -Fetest1.exe test1.c test1dll.lib Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 14.00.50727.762 for 80x86 Copyright (C) Microsoft Corporation. All rights reserved. test1.c test1.c(6) : error C2099: initializer is not a constant test1.c(7) : error C2099: initializer is not a constant test1.c(8) : error C2099: initializer is not a constant With MSVC9 I get the same errors. What am I missing?
Sorry, my bad, I had the variables local to a function and not static. With static variables I see the error. Let me take a closer look at this and get back to you.
There's an easy, if not very pretty, workaround to the problem - which is similar to what I've seen other code do. Instead of statically initializing the data, initialize it to some dummy value and have a function to do initialization. Then call that function from DllMain() or, on GCC, a function that has __attribute__((constructor)). (Since GCC doesn't have that limitation in the first place, the extra generated code can be optional, maybe with a command line option to the stubber.) As I said it's not pretty, but it works.
Created attachment 111178 [details] [review] My suggested patch What do you think of this? The major difference from your patch is that I don't decorate the functions in IDL.h.in, and I use pkg-config in the Makefile.msc.in. Builds with MSVS7, 8 and 9 for me.
Hi Tor, thanks for doing the cleanup yourself. Looks good, I just have a couple of comments: . in the README.win32 file, do you want to mention that libIDL can now use cl.exe as the preprocessor? Also, there's a typo: "...some older compiler *yuo* will..." . in IDL.h.in, if LIBIDL_COMPILATION is not defined, shouldn't it use "extern" instead of just being empty? (Same thing for the non-Win32 case.) I'll use the same approach with ORBit and tag the generated functions, but keep the non-generated stuff in a separate .def file.
Thanks for noticing those. Fixed and committed: 2008-05-19 Tor Lillqvist <tml@novell.com> Bug 522697 - Patch: make libIDL work with Microsoft's compilers Patch by Marcelo Vanzin, applied with modifications. * configure.in: Define Automake confitional OS_WIN32. * Makefile.am: Use --export-symbols libIDL.def on Windows. * Makefile.msc.in: Significant changes. Build a DLL with the same name as libtool does. Use pkg-config. * util.c: Make it work also in the !HAVE_CPP_STDIN && !HAVE_SYMLINK case. * util.h: Make __IDL_tmp_filename const. * include/libIDL/IDL.h.in: Rework the dllimport logic. Use LIBIDL_VAR to decorate imported variables, similar to GLIB_VAR. Unlike Marcelo's patch, I decided not to decorate functions. Maybe later I can be convinced it would be a good idea, but for now just use the .def file to export them and let them be imported automatically. * README.win32 * libIDL.def: Update.