Cygwin's sqlite3 modifies DLL search order

Cygwin's sqlite3 modifies DLL search order

[E. Madison Bray]

Hi all,

After some significant headache I discovered a problem introduced by
the Cygwin patches for sqlite3.  The effect of this patch is that it
modifies the DLL search order for all subsequent DLL loads (by
filename instead of absolute path) in the application.

It contains the following patch in code which is executed in the
library's initialization:

   sqlite3_vfs_register(&winVfs, 1);

+#if !defined(SQLITE_OMIT_LOAD_EXTENSION)
+  if( cygwin_conv_path ){
+    WCHAR buf[MAX_PATH];
+    cygwin_conv_path(CCP_POSIX_TO_WIN_W, "/usr/bin",
+        buf, MAX_PATH*sizeof(WCHAR));
+    osSetDllDirectoryW(buf);
+#ifdef _WIN32
+  }else if( cygwin_conv_to_full_win32_path ){
+    WCHAR buf[MAX_PATH];
+    char *buf1 = (char *)buf;
+    int i = MAX_PATH;
+    cygwin_conv_to_full_win32_path("/usr/bin", buf1);
+    while(--i>=0) buf[i] = buf1[i];
+    osSetDllDirectoryW(buf);
+#endif
+  }
+#endif
+

The call to SetDllDirectoryW modifies the default DLL search path by
always inserting /usr/bin, superseding the normal $PATH search.  Why
it does this I'm not sure.  It seems related to this patch, though
it's taken on a rather different form over the years:

http://sqlite.1065341.n5.nabble.com/Wrong-filename-handling-in-sqlite3-load-extension-for-Cygwin-td74049.html

It is related to how sqlite3 searches for extension libraries.  Though
I'd argue this patch is not even correct, since if the sqlite3 DLL is
installed somewhere other than /usr/bin (say /usr/local/bin) this
patch still won't be correct.

This is a problem since after using sqlite3 in an application,
subsequent DLL searches will always default to looking in /usr/bin
before any overrides I have on $PATH.  Is there any way we can get
this fixed?

Thanks,
Madison

Re: Cygwin's sqlite3 modifies DLL search order

[Jan Nijtmans]

Op do 16 jul. 2020 om 18:48 schreef E. Madison Bray via Cygwin:
>
> Hi all,
>
> After some significant headache I discovered a problem introduced by
> the Cygwin patches for sqlite3.  The effect of this patch is that it
> modifies the DLL search order for all subsequent DLL loads (by
> filename instead of absolute path) in the application.

Thanks for bringing this to my attention. I'm open to suggestions how
to fix this. Yes, this code is already in for a long time. It even contains
an experimental part (the part within #ifdef _WIN32), when I was
trying to make a Win32 build of SQLite work in a Cygwin environment,
oviously a bad idea ......

Thinking about it, I have an idea how to fix it:  Currently extensions
(when using the win32 vfs) are loaded using the Win32 function
LoadLibrary().  Changing that to use dlopen() might already have
the desired effect, then this section can simply be removed
completely.

Thanks!
       Jan Nijtmans

Re: Cygwin's sqlite3 modifies DLL search order

[E. Madison Bray]

(Oops, resending this upon realizing I didn't reply to the list)

On Fri, Jul 17, 2020, 09:34 Jan Nijtmans  wrote:
>
> Op do 16 jul. 2020 om 18:48 schreef E. Madison Bray via Cygwin:
> >
> > Hi all,
> >
> > After some significant headache I discovered a problem introduced by
> > the Cygwin patches for sqlite3.  The effect of this patch is that it
> > modifies the DLL search order for all subsequent DLL loads (by
> > filename instead of absolute path) in the application.
>
> Thanks for bringing this to my attention. I'm open to suggestions how
> to fix this. Yes, this code is already in for a long time. It even contains
> an experimental part (the part within #ifdef _WIN32), when I was
> trying to make a Win32 build of SQLite work in a Cygwin environment,
> oviously a bad idea ......
>
> Thinking about it, I have an idea how to fix it:  Currently extensions
> (when using the win32 vfs) are loaded using the Win32 function
> LoadLibrary().  Changing that to use dlopen() might already have
> the desired effect, then this section can simply be removed
> completely.
>
> Thanks!
>        Jan Nijtmans


Hi Jan,

Thanks for commenting on this, and thank you for all the work you've
done maintaining this port!

Yes, the original patch you came up with made sense to me so I was
still a little confused by why the SetDllDirectory stuff was added.
But I'm sure it fixed something at the time.

As long as we're patching it for Cygwin anyways I think your idea to
use dlopen() makes sense. Cygwin's dlopen should do all the path
management properly anyways :)