Re: narwhal and PGDLLIMPORT

From: Craig Ringer <craig(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Andres Freund <andres(at)2ndquadrant(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: narwhal and PGDLLIMPORT
Date: 2014-02-06 02:14:47
Message-ID: 52F2F017.1050907@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 02/05/2014 01:52 PM, Tom Lane wrote:
> Craig Ringer <craig(at)2ndquadrant(dot)com> writes:
>> On 02/05/2014 06:29 AM, Tom Lane wrote:
>>> I had been okay with the manual PGDLLIMPORT-sprinkling approach
>>> (not happy with it, of course, but prepared to tolerate it) as long
>>> as I believed the buildfarm would reliably tell us of the need for
>>> it. That assumption has now been conclusively disproven, though.
>
>> I'm kind of horrified that the dynamic linker doesn't throw its toys
>> when it sees this.
>
> Indeed :-(.
>
> The truly strange part of this is that it seems that the one Windows
> buildfarm member that's telling the truth (or most nearly so, anyway)
> is narwhal, which appears to have the oldest and cruftiest toolchain
> of the lot. I'd really like to come out the other end of this
> investigation with a clear understanding of why the newer toolchains
> are failing to report a link problem, and yet not building working
> executables.

I've done some digging and asked for some input from people with
appropriate knowledge. Getting a hard answer's going to require some
quality time with a debugger that I won't have until after the CF.
Here's what I've found so far:

In a .DEF file, symbol exports default to code exports if unspecified.

http://msdn.microsoft.com/en-us/library/hyx1zcd3.aspx

(I'm told that .DEF files take precedence over annotations in code, but
haven't found a reference to confirm that yet, and I'm not convinced
it's true since data annotated correctly with __declspec(dllexport)
seems to work. You'd think places like this would mention it, but they
don't: http://msdn.microsoft.com/en-us/library/7k30y2k5.aspx)

So we're probably generating code thunk functions for our global
variables in the import library.

There seem to be two possibilities for what's happening when you access
an extern variable defined in another module without __declspec(dllimport):

1. Our extern variable points to a thunk function's definition; or

2. Our extern variable points to the indirected pointer to the real
variable, which is what you get if you link to a CONSTANT export without
using __declspec(dllimport).

I'd need to go poking in disassemblies with the debugger to confirm
which it is, and don't have time right now. After the CF, maybe. I
suspect #1, since we're probably generating thunk functions for our data
exports.

Either way, the key thing is:

http://msdn.microsoft.com/en-us/library/hyx1zcd3.aspx

"Note that when you export a variable from a DLL with a .def file, you
do not need to specify __declspec(dllexport) on the variable. However,
in any file that uses the DLL, you must still use __declspec(dllimport)
on the declaration of data."

See also "rules and limitations for dllimport/dllexport":

http://msdn.microsoft.com/en-us/library/da6zd0a4.aspx

This page notes that you should annotate .DEF entries for data with DATA
"to reduce the likelihood that incorrect coding will cause a problem".

http://msdn.microsoft.com/en-us/library/54xsd65y.aspx

That will prevent generation of the linker symbol that's a pointer to
the real data, so we should get linker errors when we fail to specify
__declspec(dllimport).

To do this, src/tools/msvc/gendefs.pl needs to be modified to correctly
annotate globals with DATA based on the dumpbin output. I'll have a
crack at that after the CF is closed.

However, *this modification will not get rid of the need for PGDLLIMPORT
macros in headers*, because it's still necessary to explicitly tag them
with __declspec(dllimport) for correct results.

In fact, I'm now wondering if we're using inefficient thunk based
function calls from modules because we're not specifying
__declspec(dllimport) on external functions, too. Again, that's a
post-CF thing to check out using the MSVC debugger.

BTW, there's a general reference for __declspec(dllexport) here:

http://msdn.microsoft.com/en-us/library/a90k134d.aspx

and higher level concepts here:

http://msdn.microsoft.com/en-us/library/z4zxe9k8.aspx
http://msdn.microsoft.com/en-us/library/900axts6.aspx

Other references:

http://msdn.microsoft.com/en-us/library/da6zd0a4.aspx
http://msdn.microsoft.com/en-us/library/zw3za17w.aspx
http://msdn.microsoft.com/en-us/library/619w14ds.aspx
http://msdn.microsoft.com/en-us/library/54xsd65y.aspx
http://blogs.msdn.com/b/oldnewthing/archive/2006/07/20/672695.aspx
http://blogs.msdn.com/b/oldnewthing/archive/2006/07/18/669668.aspx

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2014-02-06 02:26:18 Re: should we add a XLogRecPtr/LSN SQL type?
Previous Message Peter Geoghegan 2014-02-06 00:52:38 Re: Failure while inserting parent tuple to B-tree is not fun