Обсуждение: problem with msvc linker - cannot build orafce

Pavel Stehule <pavel.stehule@gmail.com> writes:
> I am trying to build Orafce and I have problem due access to exported
> variable session_timezone.
> Any idea what can be broken?

Lack of PGDLLIMPORT on the extern declaration, no doubt.

The fact that we've not heard this before implies that either nobody has
ever tried to use orafce on Windows, or it only very recently grew a
dependency on session_timezone.
        regards, tom lane

On 11/23/15 15:14, Tom Lane wrote:
> Pavel Stehule <pavel.stehule@gmail.com> writes:
>> I am trying to build Orafce and I have problem due access to exported
>> variable session_timezone.
>> Any idea what can be broken?
> 
> Lack of PGDLLIMPORT on the extern declaration, no doubt.
> 
> The fact that we've not heard this before implies that either nobody has
> ever tried to use orafce on Windows, or it only very recently grew a
> dependency on session_timezone.

Or something changed in the build process. I've had Ken Olson report the
same symbol inaccessible when he builds PL/Java with MSVC, and he says
it has happened since PG 9.4.

I read myself to sleep about a week ago catching up on this old thread:

http://www.postgresql.org/message-id/24290.1391303038@sss.pgh.pa.us

What I (think I) took away from it was:

1.  Un-PGDLLIMPORTed references to global *functions* work ok.   Maybe they are thunked and a little less efficient,
butthey work.
 

2.  Un-PGDLLIMPORTed references to global *variables*, not so much.   They used to silently link (at least on some
buildfarmcritters)   but hold bogus data (maybe a thunk, taken as data?).
 

3.  The one concrete *action* taken in the course of that thread was to   tweak the build process to make sure such
casesat least *fail*   because that's better than silent bogosity.
 

So it's possible that (3) is what makes both Orafce and PL/Java seem to
have started failing "recently" even though the code in question may be
years old (and for most of that time, while linking without complaint,
may have been bogus without someone noticing).

The question that interests me most right now: how, if at all, can the
extension author/maintainer work around this issue when it crops up?
Obviously, the Right Thing To Do is report it and get the PGDLLIMPORT
added here, but still for years to come the extension will have to cope
with being built against PG distributions that lack it. (Never mind the
reason, when the extension doesn't build, it's the extension that looks
bad.)

Now, I thought I spotted, somewhere in that long thread, the hint of an
idea that the magic works as long as the *extension* has the variable
declared PGDLLIMPORT, even if it wasn't declared that way when the PG
executable itself was built. Anybody else remember that, or did I
imagine it?

The snag seemed to be, MSVC won't tolerate two extern declarations, one
PGDLLIMPORT and one without, so you can't get away with including the .h
file (which might not have the PGDLLIMPORT) and then declaring it yourself.

You *might* get away with creating a separate C file (how about
chamberofhorrors.c?) that, rather revoltingly, *doesn't* include the
proper PostgreSQL .h files, only duplicates the necessary declarations
with PGDLLIMPORT added, and exports some getter/setter methods
to the rest of the extension code.  (I like the idea of one chamberofhorrors
better than scattering such rubbish all over the project.)

That might work ok for log_min_messages and client_min_messages, which are
just ints. Trickier maybe for session_timezone, which is a pg_tz, so you
can't really avoid pgtime.h. That leaves even more revolting options, like

#define session_timezone decoy_session_timezone
#include <pgtime.h>
#undef session_timezone
extern PGDLLIMPORT pg_tz session_timezone

pg_tz decoy_session_timezone; /* unused */


Has anyone got the stomach to try such a thing and see what happens?
I don't have MSVC here.

-Chap

On 11/24/2015 05:33 AM, Kisung Kim wrote:
> 2015-11-24 8:12 GMT+09:00 Chapman Flack <chap@anastigmatix.net>:
>> On 11/23/15 15:14, Tom Lane wrote:
>>> Lack of PGDLLIMPORT on the extern declaration, no doubt.
>>
> Actually, we encountered the situation before couple of months.
> A client wanted to use orafce on Windows and the same build problem
> occurred.
> We performed a workaround to edit the PG source to export unresolved
> symbols,
> which I think of not a good solution.

>> Has anyone got the stomach to try such a thing and see what happens?
>> I don't have MSVC here.
> 
> We have the environment to test your ideas.
> Can you explain your ideas with more detail?

Well, the main idea is just this:  *IF* it is sufficient to declare
a variable PGDLLIMPORT only in the code that is importing it (the
big IF because I don't know whether that is true, but something I
saw in that long earlier thread seemed to suggest it) ...

Then ... the chief problem that needs to be solved is only that
MSVC won't allow you to redeclare something with PGDLLIMPORT if
it is also declared without PGDLLIMPORT in a .h file that you
include. In other words, you can't simply:

#include <pgtime.h>
extern PGDLLIMPORT pg_tz session_timezone; /* the right way now */

because it was already declared the wrong way in pgtime.h.

So one idea is just this:

#define session_timezone decoy_session_timezone;
#include <pgtime.h>
#undef decoy_session_timezone;

extern PGDLLIMPORT pg_tz session_timezone; /* the right way now */

which is not a multiple declaration of the same thing, because
what got declared the wrong way in pgtime.h is now some other thing
named decoy_session_timezone.  You might need to supply a thing by
that name, to avoid a linker complaint:

pg_tz decoy_session_timezone; /* never used */

IF the original premise is true, then this technique ought to be
usable in most cases. It would, however, break in cases where the
.h file declares macros or inline functions that refer to the
symbol, because they would all end up referring to the decoy.

My other idea, especially if there were several symbols needing
to be treated this way, would be to do it all in one dedicated
.c file, so any of the possible problems with #defining away parts
of an .h file would be contained in one place, and that file could
have a simple getter function:

pg_tz getSessionTimezone() { return session_timezone; }

which would be used in the rest of the code instead of referring
to the global directly. (In that case, of course, the same getter
function would have to be provided in the non-MSVC case too.)
A setter function could also be made, if the code needs it.

<p dir="ltr"><br /> Dne 24. 11. 2015 15:44 napsal uživatel "Chapman Flack" <<a
href="mailto:chap@anastigmatix.net">chap@anastigmatix.net</a>>:<br/> ><br /> > On 11/24/2015 05:33 AM, Kisung
Kimwrote:<br /> > > 2015-11-24 8:12 GMT+09:00 Chapman Flack <<a
href="mailto:chap@anastigmatix.net">chap@anastigmatix.net</a>>:<br/> > >> On 11/23/15 15:14, Tom Lane
wrote:<br/> > >>> Lack of PGDLLIMPORT on the extern declaration, no doubt.<br /> > >><br /> >
>Actually, we encountered the situation before couple of months.<br /> > > A client wanted to use orafce on
Windowsand the same build problem<br /> > > occurred.<br /> > > We performed a workaround to edit the PG
sourceto export unresolved<br /> > > symbols,<br /> > > which I think of not a good solution.<br /> ><br
/>> >> Has anyone got the stomach to try such a thing and see what happens?<br /> > >> I don't have
MSVChere.<br /> > ><br /> > > We have the environment to test your ideas.<br /> > > Can you explain
yourideas with more detail?<br /> ><br /> > Well, the main idea is just this:  *IF* it is sufficient to
declare<br/> > a variable PGDLLIMPORT only in the code that is importing it (the<br /> > big IF because I don't
knowwhether that is true, but something I<br /> > saw in that long earlier thread seemed to suggest it) ...<br />
><br/> > Then ... the chief problem that needs to be solved is only that<br /> > MSVC won't allow you to
redeclaresomething with PGDLLIMPORT if<br /> > it is also declared without PGDLLIMPORT in a .h file that you<br />
>include. In other words, you can't simply:<br /> ><br /> > #include <pgtime.h><br /> > extern
PGDLLIMPORTpg_tz session_timezone; /* the right way now */<br /> ><br /> > because it was already declared the
wrongway in pgtime.h.<br /> ><br /> > So one idea is just this:<br /> ><br /> > #define session_timezone
decoy_session_timezone;<br/> > #include <pgtime.h><br /> > #undef decoy_session_timezone;<br /> ><br />
>extern PGDLLIMPORT pg_tz session_timezone; /* the right way now */<br /> ><br /> > which is not a multiple
declarationof the same thing, because<br /> > what got declared the wrong way in pgtime.h is now some other thing<br
/>> named decoy_session_timezone.  You might need to supply a thing by<br /> > that name, to avoid a linker
complaint:<br/> ><br /> > pg_tz decoy_session_timezone; /* never used */<br /> ><br /> > IF the original
premiseis true, then this technique ought to be<br /> > usable in most cases. It would, however, break in cases
wherethe<br /> > .h file declares macros or inline functions that refer to the<br /> > symbol, because they would
allend up referring to the decoy.<br /> ><br /> > My other idea, especially if there were several symbols
needing<br/> > to be treated this way, would be to do it all in one dedicated<br /> > .c file, so any of the
possibleproblems with #defining away parts<br /> > of an .h file would be contained in one place, and that file
could<br/> > have a simple getter function:<br /> ><br /> > pg_tz getSessionTimezone() { return
session_timezone;}<br /> ><br /> > which would be used in the rest of the code instead of referring<br /> > to
theglobal directly. (In that case, of course, the same getter<br /> > function would have to be provided in the
non-MSVCcase too.)<br /> > A setter function could also be made, if the code needs it.<br /> ><p dir="ltr">I'll
dothese checks tomorrow.<p dir="ltr">Thank you very much, msvc is big unknown for me<p dir="ltr">regards<p
dir="ltr">Pavel

Craig Ringer <craig@2ndquadrant.com> writes:
> Actually, if __declspec(dllexport) or a .DEF entry was added in, say,
> 9.4.5, you could probably just:

> #if PG_VERSION_NUM < 90405
> extern int* log_min_messages_p;
> #define log_min_messages (*log_min_messages_p)
> #endif

> after including all PostgreSQL headers. It won't work for inline functions
> defined in PostgreSQL headers, but should otherwise be OK I think.

Some of these workarounds look like they would break if we add the missing
PGDLLIMPORT in future releases.  That would be nasty :-(.  Am I misreading
it?
        regards, tom lane