[PATCH 1/4] dlopen: switch to new pathfinder class
Instead of find_exec, without changing behaviour use new pathfinder
class with new allocator_interface around tmp_pathbuf and new vstrlist
class.
* pathfinder.h (pathfinder): New file.
* vstrlist.h (allocator_interface, allocated_type, vstrlist): New file.
* dlfcn.cc (dlopen): Avoid redundant GetModuleHandleExW with RTLD_NOLOAD
and RTLD_NODELETE. Switch to new pathfinder class, using
(tmp_pathbuf_allocator): New class.
(get_full_path_of_dll): Drop.
---
winsup/cygwin/dlfcn.cc | 310 -
winsup/cygwin/pathfinder.h | 208 +
winsup/cygwin/vstrlist.h | 373 +
3 files changed, 782 insertions(+), 109 deletions(-)
create mode 100644 winsup/cygwin/pathfinder.h
create mode 100644 winsup/cygwin/vstrlist.h
diff --git a/winsup/cygwin/dlfcn.cc b/winsup/cygwin/dlfcn.cc
index 255a6d5..e592512 100644
--- a/winsup/cygwin/dlfcn.cc
+++ b/winsup/cygwin/dlfcn.cc
@@ -20,6 +20,74 @@ details. */
#include "cygtls.h"
#include "tls_pbuf.h"
#include "ntdll.h"
+#include "pathfinder.h"
+
+/* Dumb allocator using memory from tmp_pathbuf.w_get ().
+
+ Does not reuse free'd memory areas. Instead, memory
+ is released when the tmp_pathbuf goes out of scope.
+
+ ATTENTION: Requesting memory from an instance of tmp_pathbuf breaks
+ when another instance on a newer stack frame has provided memory. */
+class tmp_pathbuf_allocator
+ : public allocator_interface
+{
+ tmp_pathbuf & tp_;
+ union
+{
+ PWCHAR wideptr;
+ void * voidptr;
+ char * byteptr;
+}freemem_;
+ size_t freesize_;
+
+ /* allocator_interface */
+ virtual void * alloc (size_t need)
+ {
+if (need == 0)
+ need = 1; /* GNU-ish */
+size_t chunksize = NT_MAX_PATH * sizeof (WCHAR);
+if (need > chunksize)
+ api_fatal ("temporary buffer too small for %d bytes", need);
+if (need > freesize_)
+ {
+ /* skip remaining, use next chunk */
+ freemem_.wideptr = tp_.w_get ();
+ freesize_ = chunksize;
+ }
+
+void * ret = freemem_.voidptr;
+
+/* adjust remaining, aligned at 8 byte boundary */
+need = need + 7 - (need - 1) % 8;
+freemem_.byteptr += need;
+if (need > freesize_)
+ freesize_ = 0;
+else
+ freesize_ -= need;
+
+return ret;
+ }
+
+ /* allocator_interface */
+ virtual void free (void *)
+ {
+/* no-op: released by tmp_pathbuf at end of scope */
+ }
+
+ tmp_pathbuf_allocator ();
+ tmp_pathbuf_allocator (tmp_pathbuf_allocator const &);
+ tmp_pathbuf_allocator & operator = (tmp_pathbuf_allocator const &);
+
+public:
+ /* use tmp_pathbuf of current stack frame */
+ tmp_pathbuf_allocator (tmp_pathbuf & tp)
+: allocator_interface ()
+, tp_ (tp)
+, freemem_ ()
+, freesize_ (0)
+ {}
+};
static void
set_dl_error (const char *str)
@@ -28,84 +96,61 @@ set_dl_error (const char *str)
_my_tls.locals.dl_error = 1;
}
-/* Look for an executable file given the name and the environment
- variable to use for searching (eg., PATH); returns the full
- pathname (static buffer) if found or NULL if not. */
-inline const char *
-check_path_access (const char *mywinenv, const char *name, path_conv& buf)
-{
- return find_exec (name, buf, mywinenv, FE_NNF | FE_DLL);
-}
-
-/* Search LD_LIBRARY_PATH for dll, if it exists. Search /usr/bin and /usr/lib
- by default. Return valid full path in path_conv real_filename. */
-static inline bool
-gfpod_helper (const char *name, path_conv &real_filename)
+/* Identify basename and baselen within name,
+ return true if there is a dir in name. */
+static bool
+spot_basename (const char * &basename, int &baselen, const char * name)
{
- if (strchr (name, '/'))
-real_filename.check (name, PC_SYM_FOLLOW | PC_NULLEMPTY);
- else if (!check_path_access ("LD_LIBRARY_PATH", name, real_filename))
-check_path_access ("/usr/bin:/usr/lib", name, real_filename);
- if (!real_filename.exists ())
-real_filename.error = ENOENT;
- return !real_filename.error;
+ basename = strrchr (name, '/');
+ basename = basename ? basename + 1 : name;
+ baselen = name + strlen (name) - basename;
+ return basename > name;
}
-static bool
-get_full_path_of_dll (const char* str, path_conv &real_filename)
+/* Setup basenames using basename and baselen,
+ return true if basenames do have some suffix. */
+static void
+collect_basenames (pathfinder::basenamelist & basenames,
+ const char * basename, int baselen)
{
- int len = strlen (str);
+ /* like strrchr (basename, '.'), but limited to baselen */
+ const char *suffix = basename + baselen;
+ while (--suffix >= basename)
+if (*suffix == '.')
+ break;
- /* empty string? */
- if (len == 0)
+ int suffixlen;
+ if (suffix >= basename)
+suffixlen = basename + baselen - suffix;
+ else
{
- set_errno (EINVAL);
- return false;/* Yes. Let caller deal with it. */
+ suffixlen = 0;
+ suff
[PATCH 3/4] dlopen: on x/lib search x/bin if exe is in x/bin
citing https://cygwin.com/ml/cygwin-developers/2016-08/msg00020.html
> Consider the file /usr/bin/cygz.dll:
> - dlopen (libz.so)success
> - dlopen (/usr/bin/libz.so) success
> - dlopen (/usr/lib/libz.so) fails
* dlfcn.c (dlopen): For dlopen("x/lib/N"), when the application
executable is in "x/bin/", search for "x/bin/N" before "x/lib/N".
---
winsup/cygwin/dlfcn.cc | 36 +++-
1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/winsup/cygwin/dlfcn.cc b/winsup/cygwin/dlfcn.cc
index e592512..f8b8743 100644
--- a/winsup/cygwin/dlfcn.cc
+++ b/winsup/cygwin/dlfcn.cc
@@ -153,6 +153,25 @@ collect_basenames (pathfinder::basenamelist & basenames,
basenames.appendv (basename, baselen, ext, extlen, NULL);
}
+/* Identify dir of current executable into exedirbuf using wpathbuf buffer.
+ Return length of exedirbuf on success, or zero on error. */
+static int
+get_exedir (char * exedirbuf, wchar_t * wpathbuf)
+{
+ /* Unless we have a special cygwin loader, there is no such thing like
+ DT_RUNPATH on Windows we can use to search for dlls, except for the
+ directory of the main executable. */
+ GetModuleFileNameW (NULL, wpathbuf, NT_MAX_PATH);
+ wchar_t * lastwsep = wcsrchr (wpathbuf, L'\\');
+ if (!lastwsep)
+return 0;
+ *lastwsep = L'\0';
+ *exedirbuf = '\0';
+ if (cygwin_conv_path (CCP_WIN_W_TO_POSIX, wpathbuf, exedirbuf, NT_MAX_PATH))
+return 0;
+ return strlen (exedirbuf);
+}
+
extern "C" void *
dlopen (const char *name, int flags)
{
@@ -184,13 +203,28 @@ dlopen (const char *name, int flags)
/* handle for the named library */
path_conv real_filename;
wchar_t *wpath = tp.w_get ();
+ char *cpath = tp.c_get ();
pathfinder finder (allocator, basenames); /* eats basenames */
if (have_dir)
{
+ int dirlen = basename - 1 - name;
+
+ /* if the specified dir is x/lib, and the current executable
+dir is x/bin, do the /lib -> /bin mapping, which is the
+same actually as adding the executable dir */
+ if (dirlen >= 4 && !strncmp (name + dirlen - 4, "/lib", 4))
+ {
+ int exedirlen = get_exedir (cpath, wpath);
+ if (exedirlen == dirlen &&
+ !strncmp (cpath, name, dirlen - 4) &&
+ !strcmp (cpath + dirlen - 4, "/bin"))
+ finder.add_searchdir (cpath, exedirlen);
+ }
+
/* search the specified dir */
- finder.add_searchdir (name, basename - 1 - name);
+ finder.add_searchdir (name, dirlen);
}
else
{
--
2.7.3
[PATCH 0/4] dlopen: improving the search algorithm
Hi Corinna,
based on the discussion on -developers list [1][2], here's a reworked
set of patches for dlopen, split along these features and fixes.
[1] https://cygwin.com/ml/cygwin-developers/2016-06/msg0.html
[2] https://cygwin.com/ml/cygwin-developers/2016-08/msg0.html
*) Switch to pathfinder class for path search iteration, without any
deliberate change in behaviour, but with the pathfinder::criterion
interface to allow for more generic use eventually.
*) Fix search order to search all basenames per one directory
rather than searching all directories per one basename.
*) For dlopen ("/path/lib/libz.so"), search "/path/bin/" first when
the executable calling is located in "/path/bin/". This is for [3].
[3] https://cygwin.com/ml/cygwin-developers/2016-08/msg00020.html
*) Consequently, dlopen ("libAPP.so") without an explicit path
should search the executable's directory first, like the Windows
loader does for linked dlls.
Topics dropped for now:
*) Extra GetModuleHandleEx is not that necessary to me, as it[4]
turns out that LoadLibrary detects a dll as already loaded even
if the underlying dll file has been replaced (renamed actually).
[4] https://cygwin.com/ml/cygwin-developers/2016-08/msg00023.html
*) Add PATH environment variable to list of searchdirs should not
be necessary to me when the executable dir is added.
Thank you!
/haubi/
[PATCH 4/4] dlopen: on unspecified lib dir search exe dir
Applications installed to some prefix like /opt/application do expect
dlopen("libAPP.so") to load "/opt/application/bin/cygAPP.dll", which
is similar to "/opt/application/lib/libAPP.so" on Linux.
See also https://cygwin.com/ml/cygwin-developers/2016-08/msg00020.html
* dlfcn.cc (dlopen): For dlopen("N"), search directory where the
application executable is in.
---
winsup/cygwin/dlfcn.cc | 6 ++
1 file changed, 6 insertions(+)
diff --git a/winsup/cygwin/dlfcn.cc b/winsup/cygwin/dlfcn.cc
index f8b8743..974092e 100644
--- a/winsup/cygwin/dlfcn.cc
+++ b/winsup/cygwin/dlfcn.cc
@@ -232,6 +232,12 @@ dlopen (const char *name, int flags)
not use the LD_LIBRARY_PATH environment variable. */
finder.add_envsearchpath ("LD_LIBRARY_PATH");
+ /* Search the current executable's directory like
+the Windows loader does for linked dlls. */
+ int exedirlen = get_exedir (cpath, wpath);
+ if (exedirlen)
+ finder.add_searchdir (cpath, exedirlen);
+
/* Finally we better have some fallback. */
finder.add_searchdir ("/usr/bin", 8);
finder.add_searchdir ("/usr/lib", 8);
--
2.7.3
[PATCH 2/4] dlopen (pathfinder): try each basename per dir
Rather than searching all search dirs per one basename,
search for all basenames within per one search dir.
pathfinder.h (check_path_access): Interchange dir- and basename-loops.
---
winsup/cygwin/pathfinder.h | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/winsup/cygwin/pathfinder.h b/winsup/cygwin/pathfinder.h
index bbba168..c306604 100644
--- a/winsup/cygwin/pathfinder.h
+++ b/winsup/cygwin/pathfinder.h
@@ -182,12 +182,12 @@ public:
basenamelist::member const ** found_basename = NULL)
{
char const * critname = criterion.name ();
-for (basenamelist::iterator name = basenames_.begin ();
-name != basenames_.end ();
-++name)
- for (searchdirlist::iterator dir(searchdirs_.begin ());
- dir != searchdirs_.end ();
- ++dir)
+for (searchdirlist::iterator dir(searchdirs_.begin ());
+dir != searchdirs_.end ();
+++dir)
+ for (basenamelist::iterator name = basenames_.begin ();
+ name != basenames_.end ();
+ ++name)
if (criterion.test (dir, name))
{
debug_printf ("(%s), take %s%s", critname,
--
2.7.3
Re: [PATCH 1/4] dlopen: switch to new pathfinder class
Hi Michael, On Aug 31 20:07, Michael Haubenwallner wrote: > Instead of find_exec, without changing behaviour use new pathfinder > class with new allocator_interface around tmp_pathbuf and new vstrlist > class. > * pathfinder.h (pathfinder): New file. > * vstrlist.h (allocator_interface, allocated_type, vstrlist): New file. > * dlfcn.cc (dlopen): Avoid redundant GetModuleHandleExW with RTLD_NOLOAD > and RTLD_NODELETE. Switch to new pathfinder class, using > (tmp_pathbuf_allocator): New class. > (get_full_path_of_dll): Drop. > [...] Just one nit here: > +/* Dumb allocator using memory from tmp_pathbuf.w_get (). > + > + Does not reuse free'd memory areas. Instead, memory > + is released when the tmp_pathbuf goes out of scope. > + > + ATTENTION: Requesting memory from an instance of tmp_pathbuf breaks > + when another instance on a newer stack frame has provided memory. */ > +class tmp_pathbuf_allocator > + : public allocator_interface You didn't reply to https://cygwin.com/ml/cygwin-developers/2016-08/msg00013.html So, again, why didn't you simply integrate a tmp_pathbuf member into the pathfinder class, rather than having to create some additional allocator class? I'm probably not the most diligent C++ hacker, but to me this additional allocator is a bit confusing. The rest of the patch looks good. I'll look further into the patchset later tomorrow. Thanks, Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Maintainer cygwin AT cygwin DOT com Red Hat signature.asc Description: PGP signature
