On Wed, Oct 22, 2014 at 12:12:36AM -0400, Noah Misch wrote:
> On Mon, Oct 20, 2014 at 01:03:31AM -0400, Noah Misch wrote:
> > I reproduced narwhal's problem using its toolchain on another 32-bit Windows
> > Server 2003 system.  The crash happens at the SHGetFolderPath() call in
> > pqGetHomeDirectory().  A program can acquire that function via shfolder.dll 
> > or
> > via shell32.dll; we've used the former method since commit 889f038, for 
> > better
> > compatibility[1] with Windows NT 4.0.  On this system, shfolder.dll's 
> > version
> > loads and unloads shell32.dll.  In PostgreSQL built using this older 
> > compiler,
> > shfolder.dll:SHGetFolderPath() unloads libpq in addition to unloading 
> > shell32!
> > That started with commit 846e91e.  I don't expect to understand the 
> > mechanism
> > behind it, but I recommend we switch back to linking libpq with shell32.dll.
> > The MSVC build already does that in all supported branches, and it feels 
> > right
> > for the MinGW build to follow suit in 9.4+.  Windows versions that lack the
> > symbol in shell32.dll are now ancient history.
> 
> That allowed narwhal to proceed a bit further than before, but it crashes
> later in the dblink test suite.  Will test again...

Calls to ldap_init() exhibit the same problem shfolder.dll:SHGetFolderPath()
exhibited: it loads and unloads some DLLs, and it manages to unload libpq in
passing.  There's nothing comparable to the above workaround this time, but I
found a more fundamental fix.

Each DLL header records the DLL's presumed name, viewable with "objdump -x
libpq.dll | grep ^Name".  Per the GNU ld documentation, one can override it in
a .def file:

  The optional LIBRARY <name> command indicates the internal name of the
  output DLL. If `<name>' does not include a suffix, the default library
  suffix, `.DLL' is appended.

libpqdll.def uses "LIBRARY LIBPQ", which yields an internal name of
"LIBPQ.dll" under MinGW-w64 i686-4.9.1-release-win32-dwarf-rt_v3-rev1.  Our
MSVC build process also gives "LIBPQ.dll".  Under narwhal's older toolchain,
libpq.dll gets a name of just "LIBPQ".  The attached patch brings the product
of narwhal's toolchain in line with the others.  I don't know by what magic
wldap32.dll:ldap_init() and shfolder.dll:SHGetFolderPath() care about finding
".dll" in the names of loaded libraries, but it does fix the bug.

This erases the impetus for my recent commit 53566fc.  I'm inclined to keep
that commit in the name of consistency with the MSVC build, but one could
argue for reverting its 9.4 counterpart.  I don't feel strongly either way, so
I expect to let 2f51f42 stand.

FYI, the problem is reproducible in a 32-bit PostgreSQL build running on
32-bit or 64-bit Windows Server 2003.  It does not occur on 64-bit Windows
Server 2008, even after marking postgres.exe to run in the compatibility mode
for Windows Server 2003.  (I did not try 32-bit Windows Server 2008.)
commit 912846f (HEAD, master)
Author:     Noah Misch <n...@leadboat.com>
AuthorDate: Sun Oct 26 18:02:55 2014 -0400
Commit:     Noah Misch <n...@leadboat.com>
CommitDate: Sun Oct 26 18:17:34 2014 -0400

    MinGW: Include .dll extension in .def file LIBRARY commands.
    
    Newer toolchains append the extension implicitly if missing, but
    buildfarm member narwhal (gcc 3.4.2, ld 2.15.91 20040904) does not.
    This affects most core libraries having an exports.txt file, namely
    libpq and the ECPG support libraries.  On Windows Server 2003, Windows
    API functions that load and unload DLLs internally will mistakenly
    unload a libpq whose DLL header reports "LIBPQ" instead of "LIBPQ.dll".
    When, subsequently, control would return to libpq, the backend crashes.
    Back-patch to 9.4, like commit 846e91e0223cf9f2821c3ad4dfffffbb929cb027.
    Before that commit, we used a different linking technique that yielded
    "libpq.dll" in the DLL header.
    
    Commit 53566fc0940cf557416b13252df57350a4511ce4 worked around this by
    eliminating a call to a function that loads and unloads DLLs internally.
    That commit is no longer necessary for correctness, but its improving
    consistency with the MSVC build remains valid.

diff --git a/src/Makefile.shlib b/src/Makefile.shlib
index 012dd12..e3a06ef 100644
--- a/src/Makefile.shlib
+++ b/src/Makefile.shlib
@@ -423,13 +423,13 @@ UC_NAME = $(shell echo $(NAME) | tr 
'abcdefghijklmnopqrstuvwxyz' 'ABCDEFGHIJKLMN
 
 lib$(NAME)dll.def: $(SHLIB_EXPORTS)
        echo '; DEF file for MS VC++' >$@
-       echo 'LIBRARY LIB$(UC_NAME)' >>$@
+       echo 'LIBRARY LIB$(UC_NAME).dll' >>$@
        echo 'EXPORTS' >>$@
        sed -e '/^#/d' -e 's/^\(.*[     ]\)\([0-9][0-9]*\)/    \1@ \2/' $< >>$@
 
 lib$(NAME)ddll.def: $(SHLIB_EXPORTS)
        echo '; DEF file for MS VC++' >$@
-       echo 'LIBRARY LIB$(UC_NAME)D' >>$@
+       echo 'LIBRARY LIB$(UC_NAME)D.dll' >>$@
        echo 'EXPORTS' >>$@
        sed -e '/^#/d' -e 's/^\(.*[     ]\)\([0-9][0-9]*\)/    \1@ \2/' $< >>$@
 
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to