Re: Mark all GUC variable as PGDLLIMPORT

From: Craig Ringer <craig(dot)ringer(at)enterprisedb(dot)com>
To: Julien Rouhaud <rjuju123(at)gmail(dot)com>
Cc: Magnus Hagander <magnus(at)hagander(dot)net>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Chapman Flack <chap(at)anastigmatix(dot)net>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Michael Paquier <michael(at)paquier(dot)xyz>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Mark all GUC variable as PGDLLIMPORT
Date: 2021-08-27 09:41:40
Message-ID: CAGRY4nxmEVjBT+YKadYvJvzKXwy_n2nK6VVZkJa+VE_Sn=C_uQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, 27 Aug 2021 at 08:59, Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote:

> On Fri, Aug 27, 2021 at 3:42 AM Magnus Hagander <magnus(at)hagander(dot)net>
> wrote:
> >
> > Yeah, but that does move the problem to the other side doesn't it? So
> > if you (as a pure test of course) were to remove the variable
> > completely from the included header and just declare it manually with
> > PGDLLSPEC in your file, it should work?
> >
> > Ugly as it is, I wonder if there's a chance we could just process all
> > the headers at install times and inject the PGDLLIMPORT. We know which
> > symvols it is on account of what we're getting in the DEF file.
> >
> > Not saying that's not a very ugly solution, but it might work?
>
> It's apparently not enough. I tried with autovacuum_max_workers GUC,
> and it still errors out.
> If I add a PGDLLIMPORT, there's a link error when trying to access the
> variable:
> error LNK2019: unresolved external symbol __imp_autovacuum_max_workers
> referenced in function...
>
> If I use PGDLLEXPORT I simply get:
> error LNK2001: unresolved external symbol aytovacuum_max_workers
>

It works, but you can't use PGDLLIMPORT, you have to implement the import
yourself without the help of __declspec(dllimport) .

Where you want

autovacuum_max_workers

you must instead write

*((int*)__imp_autovacuum_max_workers)

Here's the comment I wrote on the topic in something I was working on:

/*
* On Windows, a symbol is not accessible outside the executable or shared
* library (PE object) that it is defined in unless explicitly exported in
* the DLL interface.
*
* It must then be explicitly imported by objects that use it; Windows
doesn't
* do ELF-style fixups.
*
* The export part is usually accomplished by a __declspec(dllexport)
* annotation on the symbol or a .DEF file supplied as linker input.
Postgres
* uses the .DEF approach, auto-exporting all symbols using
* src\tools\msvc\gendef.pl . Internally this hides "symname" from the DLL
* interface and instead generates an export symbol "__imp_symname" that is
a
* pointer to the value of "symname".
*
* The import part is usually done with the __declspec(dllimport)
annotation on
* the symbol. The PGDLLIMPORT macro expands to __declspec(dllimport) when
* postgres headers are included during extension compilation. But not all
the
* symbols that pglogical needs are annotated with PGDLLIMPORT. Attempting
to
* access a symbol that is not so-annotated will fail at link time with an
* error like
*
* error LNK2001: unresolved external symbol
criticalSharedRelcachesBuilt
*
* Because of gendefs.pl, postgres still exports the symbol even if it isn't
* annotated PGDLLIMPORT. So we can just do the shorthand that
* __declspec(dllimport) does for us in the preprocessor instead: replace
each
* symbol with its __imp_symbol indirection and dereference it.
*
* There's one wrinkle in this though. MSVC doesn't generate a definition
for a
* global data symbol that is neither initialized nor explicitly marked
* __declspec(dllexport). gendefs.pl will think such symbols are references
to
* a symbol defined in another object file and will skip them without
emitting
* a DATA entry for them in the DEF file, so no __imp_ stub is generated in
the
* DLL interface. We can't use (*__imp_symbolname) if there's no import
stub.
*
* If they're GUCs, we can round-trip them through their text values
* to read them. Nothing should ever be assigning to GUC storage and
there's no
* reason to take the address of GUC storage, so this should work fine,
albeit
* slower. If we find any that aren't GUCs we're in trouble but so far there
* haven't been any.
*
* See also:
*
* -
https://docs.microsoft.com/en-us/cpp/build/importing-data-using-declspec-dllimport
* - https://docs.microsoft.com/en-us/cpp/build/importing-using-def-files
* -
https://docs.microsoft.com/en-us/cpp/build/exporting-from-a-dll-using-def-files
* -
https://docs.microsoft.com/en-us/cpp/build/determining-which-exporting-method-to-use
*/

This is gruesome and I hadn't planned to mention it, but now someone
noticed the .DEF file exports the symbols I guess it does no harm.

So can we just fix PGDLLIMPORT now?

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Dagfinn Ilmari Mannsåker 2021-08-27 10:32:30 Re: perlcritic: prohibit map and grep in void conext
Previous Message Craig Ringer 2021-08-27 09:36:14 Re: Mark all GUC variable as PGDLLIMPORT