Re: [PATCH] tests: import variables for MSVC.

[Peter Rosin]

Den 2010-09-25 07:28 skrev Charles Wilson:
> On 9/24/2010 2:46 PM, Peter Rosin wrote:
>> Now I'm also confused.
> 
> That's not good.
> 
>> /me double checks (see below)
>>
>> WHAT? It doesn't work as I stated!?!
>>
>> *ponders that for a bit*
>> *scratches head*
>>
>> Ahh, you said "libtool does this by default IIRC". If that's actually the
>> case than that is what has have me fooled for years.
> 
> Pay close attention to how libtool compiles (the single, only) main.o.
> Does it get the picflag (-DDLL_EXPORT) or not?
> 
> As I've always understood it, *without* auto-import magic, you MUST have
> the following:
> 
> 1) shared:
>   a) a dll, compiled with declspec(dllexport) [or, create the DLL with
> an explicit .def file listing the exported symbols]
>   b) the client *must* be compiled with declspec(dllimport) decorators
> on all symbols the client wants to use from the DLL
> 
> 2) static
>   a) a lib. Nothing should be compiled with declspec(dllexport), and
> obviously there is no .def file involved
>   b) the client must NOT be compiled with declspec(dllimport) -- because
> then you get the missing __imp__foo error.
> 
> So, your test case below doesn't surprise me.  What DOES surprise me, is
> that it ever worked at all with MSVC (or gcc + -disable-auto-import),
> since it appears that Ralf is correct and the *_PROGRAM objects are
> built in only one "mode".  Whether that is "picflag" (-DDLL_EXPORT) or
> not, one or the other linking modality should fail (static linking if
> main.o is compiled with -DDLL_EXPORT; dynamic linking if main.o is NOT
> compiled with -DLL_EXPORT).
> 
> So, yeah, I'm a little confused as well.  I think you need to do some
> archeology on the *_PROGRAM objects (nm -B or whatever the equivalent is
> in msvc land), and figure out what symbols are undefined.  I'd take a
> hard look at demo/.

I have done some more digging and am no longer confused.  As it happens,
you could say that we are both right.  The little sample I quoted was
a little bit too small, so it behaved according to your mental image.
If you "flesh out" the example so that the program also uses a function
from the library, it magically starts to find the variable too, making
it behave according to my mental image.  Now, libraries that only
export data items are /rare/, and clients that only use data items are
also /rare/.  So, in theory you're correct and in practice I'm correct.

So, this works:

$ cat << EOF > dllfoo.c
extern __declspec(dllexport) int variable;
int variable = 2;
int foo(void) { return 0; }
EOF
$ cl -nologo -Fefoo.dll -MD dllfoo.c -link -dll -export:variable,DATA 
-export:foo -implib:foo.dll.lib
dllfoo.c
   Creating library foo.dll.lib and object foo.dll.exp
$ cat << EOF > bar.c
extern __declspec(dllimport) int variable;
int foo(void);
int main(void) { foo(); return variable; }
EOF
$ cl -nologo -Febardll.exe -MD bar.c foo.dll.lib
bar.c
$ ./bardll
$ echo $?
2
$ cat << EOF > libfoo.c
extern int variable;
int variable = 2;
int foo(void) { return 0; }
EOF
$ cl -nologo -c -MD libfoo.c
libfoo.c
$ lib -nologo -out:foo.lib libfoo.obj
$ cl -nologo -Febarlib.exe -MD bar.c foo.lib
bar.c
bar.obj : warning LNK4217: locally defined symbol _variable imported in 
function _main
$ ./barlib
$ echo $?
2

Note, I only added the -export options when building the dll as libtool
adds those automatically, and I wanted to eliminate as many differences
as possible from what libtool is doing.

GCC does not have the ability to find "locally defined symbols" when
it is looking for imports.

> NOTE: Bruno Haible's method worked around this by ALWAYS defining
> symbols in a library as declspec(dllimport), when building the library
> shared, building the library static, OR building a client.
> 
> BUT...to make linking the DLL itself work (with internal references to
> these "dllimported" symbols!), he uses some nm and asm magic to (a)
> manually define the __imp_* redirection symbol values and set them to
> point to the address of the actual symbol, and (b) explicitly export
> each "exported" symbol using an asm .drective, even tho it was marked
> dllimport.
> 
> It's really rather clever, but I've never really figured out how to
> merge it into the typical auto*/libtool process (Bruno's libiconv and
> gettext do it, but with a lot of Makefile.am hackery).  Second, I don't
> know if it would work with MSVC easily; certainly the asm magic would
> need modification.  Third, it almost *requires* to build *everything*
> with --disable-auto-import.  Which would NOT go over well with the
> larger community.  So, I've never pursued it, and obviously Bruno hasn't
> pushed the issue.

Note that since libtool does add the -export options when linking the
dll, it will work to unconditionally declare (for MSVC) any variables
with __declspec(dllimport), and then rely on the -export option when
linking to "win" and make it an export.  But, I suspect that any
references to the variable will go through an indirection, as I suspect
will be the case when the library consumer is using __declspec(dllimport)
even when linking statically.

That indirection (if indeed present) is a small price in my book.  Others
might have a different opinion of course.

>> *deep sigh*
>>
>> Thanks for setting me straight.
>>
>> What now?  Is the patch still good?  (with a reworded changelog of course)
> 
> Well, I think so.  It's possible we might need a follow-on patch for
> "strict correctness" -- but this patch appears to be correct as far as
> it goes.
> 
>> But now I'm really confused.  What made the original patch work?  It had
>> "#define EXTERN extern __declspec(dllimport)" unconditionally for MSVC.
>> And that patch also had two SKIPs and one FAIL (libfoo.a vs. foo.lib).
>> I.e. the exact same result, which means I can't be completely wrong.  Or
>> is the testsuite not doing any static builds?  But that seems highly
>> unlikely indeed.  WTF?
> 
> Look really closely at how hell_static.exe is built in demo/.
> 
> But, to sum up, I see no problems with THIS patch, as far as it goes.
> 
> 
> 
> With regards to Ralf's question re: _MSC_VER.  Well, technically you'd
> probably be "more" correct to do:
> 
> #if (defined(_WIN32) || defined(_WIN32_WCE)) && !defined(__GNUC__)
> ...
> 
> rather than _MSC_VER; that formula would indicate "any win32 or wince
> platform, using any compiler EXCEPT gcc" -- because only gcc has
> "auto-import" on win32.

I agree, it should be more stable, so I'm going with your #if.  Pushing
with that squashed in (but I'm still waiting on the testsuite, so if
anybody objects there's still some time).

> But nobody does that; pretty much all source code with pretensions of
> both cross-platform use, AND windows support, uses _MSC_VER (*badly*
> ported code uses _WIN32 to mean "MSVC" which causes no end of cygwin and
> mingw headaches!).
> 
> Because of that, IIUC most users of "other" compilers for win32 (Intel
> C/C++, Watcom, Borland, etc) either (a) routinely
> s/_MSC_VER/__WATCOMC__/g, or (b) add -D_MSC_VER anyway.

Little reason for us to follow that pattern.

Cheers,
Peter