On Fri, 27 Aug 2021 at 08:59, Julien Rouhaud <rjuju...@gmail.com> wrote:
> On Fri, Aug 27, 2021 at 3:42 AM Magnus Hagander <mag...@hagander.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?