Re: lt_dlerror changes
Hi Chuck, [Warning: thread hijack - please start a new thread and paste in any context you need if you care to reply further.] On 25 Jun 2010, at 06:23, Charles Wilson wrote: On 6/17/2010 4:54 PM, Peter O'Gorman wrote: Well, this is what I ended up with, it does not change the currently documented saving of error messages until lt_dlerror() is called, it copies the error message to ensure that we don't return garbage when lt_dlerror is called. I think I also got all the places where we were setting file not found. Still, I am not overjoyed with this. I'm sorry I sidetracked this thread, with the discussion on *using* this patch to track down an error in an different part of libltdl on cygwin. But... I think this patch is a decent, incremental approach to fixing the error reporting in libltdl. Unless somebody else is working on something better in secret, I think this version ought to be pushed. Agreed. As an aside, I *am* working on something in secret actually. I finally gave up trying to fully grok the spaghetti code at the core of libltdl, and with all the lessons I learned in teasing out the other parts of it between 1.5.x and 2.2.x, I am in the early stages of a complete rewrite. I have spent so many hours staring at the original code that I'm now of the opinion that I can put out a cleaner, more stable, ground up rewrite in less time than it would take to bring the incumbent implementation any closer to sanity than what we already have. A ton of smart talented programmers have been defeated by the error reporting of the current code base alone: Here we are 8 years down the line... and *still* we don't have it working quite right. If I'd started this rewrite a year or two ago, we'd have something better by now already, so any effort I would otherwise be investing in the current libltdl tree goes to the rewrite instead. However. with this rewrite, something useable is a long way off yet. I'm still on the fence regarding whether dlfcn.h is even a good starting point for the API... any nuggets of wisdom or insight would be appreciated more at this point than they will when I post a draft implementation in several months. More here: http://lists.gnu.org/archive/html/libtool/2010-06/msg00057.html Cheers, -- Gary V. Vaughan (g...@gnu.org)
Re: lt_dlerror changes
On 6/17/2010 4:54 PM, Peter O'Gorman wrote: Well, this is what I ended up with, it does not change the currently documented saving of error messages until lt_dlerror() is called, it copies the error message to ensure that we don't return garbage when lt_dlerror is called. I think I also got all the places where we were setting file not found. Still, I am not overjoyed with this. I'm sorry I sidetracked this thread, with the discussion on *using* this patch to track down an error in an different part of libltdl on cygwin. But... I think this patch is a decent, incremental approach to fixing the error reporting in libltdl. Unless somebody else is working on something better in secret, I think this version ought to be pushed. Not that my opinion counts for much, but Peter's patch DID help in tracking down a problem already, so that's definitely a point in its favor. -- Chuck
Re: lt_dlerror changes
Den 2010-06-18 07:21 skrev Peter O'Gorman: On 06/17/2010 10:35 PM, Peter O'Gorman wrote: On 06/17/2010 10:21 PM, Charles Wilson wrote: On 6/17/2010 10:24 PM, Peter O'Gorman wrote: Unfortunately, this doesn't magically assist solving my problem with 71. dlloader-api.at:23: FAILED (dlloader-api.at:422) but that's not a reason to object to the patch. Well that sucks :( I suggest changing the LT__SETERROR macro in libltdl/libltdl/lt__private.h to print __FILE__ and __LINE__ etc. if errorcode == FILE_NOT_FOUND, then running test 71 again. It doesn't look like it's happening in loadlibrary.c. ../libtool/libltdl/loaders/preopen.c (188) ../libtool/libltdl/loaders/preopen.c (188) ../libtool/libltdl/loaders/preopen.c (188) Hmm. Adding more debug output to preopen.c:vm_open ... Searching for preloaded symbol table for dlopen.a Err...why is it looking for a module named /usr/bin/last? Is it somehow using a $PATH search, and matching last to /usr/bin/last.exe? cygwin's dlopen does do some funky things with $PATH, because that's how windows looks for DLLs -- but we're in preopen, here... Ok, it's adding a loader (at the end of the list of loaders), then trying to lt_dlopen(last), that's going to try all other loaders before getting to the added loader, and in your case the loadlibrary loader finds /usr/bin/last (because /usr/bin is properly in the search path). My changes to lt_dlerror should have given you the error from loadlibrary.c, but didn't, I will check why, it looks like the patch needs more work though. That error could just as well come from the last_open function. I've got this nagging feeling that something clobbers the module name before it reaches the last_open function and that it's that clobbering that is the cause of the failure. Not time to check the details ATM though, sorry. So, I'm still curious about the output with the patch in the below message applied (on Chucks setup). http://lists.gnu.org/archive/html/bug-libtool/2010-06/msg00054.html Perhaps the presence of /usr/bin/last.exe is triggering that clobbering. Serious WAG warning... Cheers, Peter
Re: lt_dlerror changes
On 6/18/2010 6:42 AM, Peter Rosin wrote: That error could just as well come from the last_open function. I've got this nagging feeling that something clobbers the module name before it reaches the last_open function and that it's that clobbering that is the cause of the failure. Not time to check the details ATM though, sorry. So, I'm still curious about the output with the patch in the below message applied (on Chucks setup). http://lists.gnu.org/archive/html/bug-libtool/2010-06/msg00054.html Perhaps the presence of /usr/bin/last.exe is triggering that clobbering. Serious WAG warning... Sorry Peter (Rosin), I'm not subscribed to bug@ so I've been monitoring via the web. But I missed your post. Here are the results: first_sym (first): first_ctx result: first_symbol first_close (first): first_ctx -first_open denies a request +first_open denies request for ./.libs/module.dll result: module_symbol -first_open denies a request -last_open (last): last_ctx -last_sym (last): last_ctx -result: last_symbol -first_open denies a request -last_open denies a request +first_open denies request for /usr/bin/last +last_open denies request for /usr/bin/last +lt_dlopen failed: file not found first_exit: first_ctx -last_close (last): last_ctx last_exit: last_ctx If I rename /usr/bin/last.exe to something else: first_sym (first): first_ctx result: first_symbol first_close (first): first_ctx -first_open denies a request +first_open denies request for ./.libs/module.dll result: module_symbol -first_open denies a request +first_open denies request for last last_open (last): last_ctx last_sym (last): last_ctx result: last_symbol -first_open denies a request -last_open denies a request +first_open denies request for no-module +last_open denies request for no-module first_exit: first_ctx last_close (last): last_ctx last_exit: last_ctx Which tells me if I revert your patch, it'll pass...which it does (because all my other debugging mods output to stderr, which this test ignores). Here is that stderr output, during this passing scenario (e.g. with /usr/bin/last.exe removed): Searching for preloaded symbol table for dlopen.a Found preloaded symbol table for libltdlc Found preloaded symbol table for dlopen.a oops, bailing at line 183 Searching for preloaded symbol table for loadlibrary.a Found preloaded symbol table for libltdlc Found preloaded symbol table for dlopen.a Found preloaded symbol table for dlopen_LTX_get_vtable Found preloaded symbol table for loadlibrary.a oops, bailing at line 183 Searching for preloaded symbol table for first.a Found preloaded symbol table for libltdlc Found preloaded symbol table for dlopen.a Found preloaded symbol table for dlopen_LTX_get_vtable Found preloaded symbol table for loadlibrary.a Found preloaded symbol table for loadlibrary_LTX_get_vtable Found preloaded symbol table for @PROGRAM@ ../libtool/libltdl/loaders/preopen.c (191) Searching for preloaded symbol table for module.a Found preloaded symbol table for libltdlc Found preloaded symbol table for dlopen.a Found preloaded symbol table for dlopen_LTX_get_vtable Found preloaded symbol table for loadlibrary.a Found preloaded symbol table for loadlibrary_LTX_get_vtable Found preloaded symbol table for @PROGRAM@ ../libtool/libltdl/loaders/preopen.c (191) Searching for preloaded symbol table for last.a Found preloaded symbol table for libltdlc Found preloaded symbol table for dlopen.a Found preloaded symbol table for dlopen_LTX_get_vtable Found preloaded symbol table for loadlibrary.a Found preloaded symbol table for loadlibrary_LTX_get_vtable Found preloaded symbol table for @PROGRAM@ ../libtool/libltdl/loaders/preopen.c (191) Searching for preloaded symbol table for last Found preloaded symbol table for libltdlc Found preloaded symbol table for dlopen.a Found preloaded symbol table for dlopen_LTX_get_vtable Found preloaded symbol table for loadlibrary.a Found preloaded symbol table for loadlibrary_LTX_get_vtable Found preloaded symbol table for @PROGRAM@ Searching for preloaded symbol table for no_module.a Found preloaded symbol table for libltdlc Found preloaded symbol table for dlopen.a Found preloaded symbol table for dlopen_LTX_get_vtable Found preloaded symbol table for loadlibrary.a Found preloaded symbol table for loadlibrary_LTX_get_vtable Found preloaded symbol table for @PROGRAM@ ../libtool/libltdl/loaders/preopen.c (191) Searching for preloaded symbol table for no-module Found preloaded symbol table for libltdlc Found preloaded symbol table for dlopen.a Found preloaded symbol table for dlopen_LTX_get_vtable Found preloaded symbol table for loadlibrary.a Found preloaded symbol table for loadlibrary_LTX_get_vtable Found preloaded symbol table for @PROGRAM@ Here's the key bit: Searching for preloaded symbol table for last vs Searching for preloaded symbol table for /usr/bin/last SO, before preopen:vmopen is called, somebody -- one of the other loaders? -- modified 'filename' simply because
Re: lt_dlerror changes
On 06/18/2010 08:09 AM, Charles Wilson wrote: Here's the key bit: Searching for preloaded symbol table for last vs Searching for preloaded symbol table for /usr/bin/last SO, before preopen:vmopen is called, somebody -- one of the other loaders? -- modified 'filename' simply because /usr/bin/last.exe exists. But I thought preopen was the very first loader. Yes, the preopen loader gets tried first with last.a, it failed, then find_handle is called which does an access() check for last in every dir in path, and then tryall_dlopen()'s it if it exists. tryall_dlopen also tries the preopen loader. Peter
Re: lt_dlerror changes
On 06/18/2010 08:36 AM, Peter O'Gorman wrote:
On 06/18/2010 08:33 AM, Peter O'Gorman wrote:
On 06/18/2010 08:09 AM, Charles Wilson wrote:
Here's the key bit:
Searching for preloaded symbol table for last
vs
Searching for preloaded symbol table for /usr/bin/last
SO, before preopen:vmopen is called, somebody -- one of the other
loaders? -- modified 'filename' simply because /usr/bin/last.exe exists.
But I thought preopen was the very first loader.
This should (hopefully) stop the preopen loader getting used twice, and
perhaps (I'm really dreaming now) give a better error.
Peter
diff --git a/libltdl/ltdl.c b/libltdl/ltdl.c
index 1213f0d..094673f 100644
--- a/libltdl/ltdl.c
+++ b/libltdl/ltdl.c
@@ -424,8 +424,11 @@ tryall_dlopen (lt_dlhandle *phandle, const char *filename,
if (vtable)
loader_vtable = vtable;
else
- loader_vtable = lt_dlloader_get (loader);
-
+ {
+ loader_vtable = lt_dlloader_get (loader);
+ /* We already tried the preopen loader, no need to do it again */
+ if (loader_vtable == lt_dlloader_find (lt_preopen)) continue;
+ }
#ifdef LT_DEBUG_LOADERS
fprintf (stderr, Calling %s-module_open (%s)\n,
(loader_vtable loader_vtable-name) ? loader_vtable-name : (null),
Re: lt_dlerror changes
On 6/18/2010 10:50 AM, Charles Wilson wrote:
although oddly, loadlibrary sets CANNOT_OPEN instead of FILE_NOT_FOUND
(loadlibary.c:vmopen):
232 if (!module)
233 LOADLIB_SETERROR (CANNOT_OPEN);
234 else if (cur)
235 {
236 LT__SETERROR (CANNOT_OPEN);
237 module = 0;
238 }
I think there should be an earlier check for !module @ 206, and THAT one
should set FILE_NOT_FOUND. However, this isn't the cause of the current
problem.
Never mind this bit. We already know the file exists, because access()
called in find_handle_callback() told us that. CANNOT_OPEN is the
correct value here.
--
Chuck
Re: lt_dlerror changes
On 6/18/2010 9:36 AM, Peter O'Gorman wrote:
On 06/18/2010 08:33 AM, Peter O'Gorman wrote:
Yes, the preopen loader gets tried first with last.a, it failed, then
find_handle is called which does an access() check for last in every dir
in path,
Correction for lack of coffee: not PATH, but LTDL_LIBRARY_PATH,
LD_LIBRARY_PATH (or whatever) and the standard library paths.
Nope, I'm debugging right now, and I'm looking at
#0 foreach_dirinpath (search_path=0x10d8b5d
/usr/local/bin:/usr/bin:/usr/bin:/tex/miktex/bin:/c/program
files/graphicsmagick-1.1.11-q8:/c/windows/system32:/c/windows:/c/windows/system32/wbem:/c/program
files/cyberlink/power2go/:/c/Program Files..., base_name=0x10e9ec0
last, func=0x405040 find_handle_callback, data1=0x22cc30, data2=0x0)
at ../libtool/libltdl/ltdl.c:681
#1 0x0040448c in find_handle (phandle=0x22cc78, filename=value
optimized out, ext=0x10e9ec4 , advise=0x0) at
../libtool/libltdl/ltdl.c:806
#2 try_dlopen (phandle=0x22cc78, filename=value optimized out,
ext=0x10e9ec4 , advise=0x0) at ../libtool/libltdl/ltdl.c:1466
#3 0x00404e4e in lt_dlopenadvise (filename=0x408244 last, advise=0x0)
at ../libtool/libltdl/ltdl.c:1674
#4 0x00405039 in lt_dlopen (filename=0x408244 last) at
../libtool/libltdl/ltdl.c:1628
#5 0x004014a2 in main (argc=value optimized out, argv=value
optimized out) at main.c:288
That is, search_path = $PATH. This is because (ltdl.c:try_dlopen):
1466 if ((dir || (!find_handle (user_search_path, base_name,
1467 newhandle, advise)
1468 !find_handle (getenv (LTDL_SEARCHPATH_VAR), base_name
1469 newhandle, advise)
1470 #if defined(LT_MODULE_PATH_VAR)
1471 !find_handle (getenv (LT_MODULE_PATH_VAR), base_name,
1472 newhandle, advise)
1473 #endif
1474 #if defined(LT_DLSEARCH_PATH)
1475 !find_handle (sys_dlsearch_path, base_name,
1476 newhandle, advise)
1477 #endif
1478 )))
where
config.h:#define LT_MODULE_PATH_VAR PATH
config.h:#define LT_DLSEARCH_PATH /lib:/usr/lib
(and the env var $LTDL_LIBRARY_PATH is not set)
So, of the four calls to find_handle above, I'm in the third one. While
iterating thru $PATH, of course ONE of the possibilities is
/usr/bin/last. Most of the loaders fail:
first
preopen
dlopen
loadlibrary
although oddly, loadlibrary sets CANNOT_OPEN instead of FILE_NOT_FOUND
(loadlibary.c:vmopen):
232 if (!module)
233 LOADLIB_SETERROR (CANNOT_OPEN);
234 else if (cur)
235 {
236 LT__SETERROR (CANNOT_OPEN);
237 module = 0;
238 }
I think there should be an earlier check for !module @ 206, and THAT one
should set FILE_NOT_FOUND. However, this isn't the cause of the current
problem.
But in any case, it appears the problem is (ltdl.c):
778 static int
779 find_handle_callback (char *filename, void *data, void *data2)
780 {
781 lt_dlhandle *phandle = (lt_dlhandle *) data;
782 int notfound= access (filename, R_OK);
783 lt_dladvise advise = (lt_dladvise) data2;
784
785 /* Bail out if file cannot be read... */
786 if (notfound)
787 return 0;
when called with '/usr/bin/last', cygwin's access returns 0, because of
the .exe magic.
However, the real issue isn't the exe magic, it's the fact that the test
makes assumptions about what files ARE or ARE NOT in the
LT_MODULE_PATH_VAR. We *hope* that there is no last or first or
module or no_module anywhere in there.
Because of the design decision in find_handle_callback:
789 /* Try to dlopen the file, but do not continue searching in any
790 case. */
791 if (tryall_dlopen (phandle, filename, advise, 0) != 0)
792 *phandle = 0;
793
794 return 1;
795 }
If we EVER find a file anywhere in LT_MODULE_PATH_VAR that exists and
matches the desired (base)name, but cannot open it, then we never search
any more in the path (see foreach_dirinpath) -- NOR do we ever just try
to open the name without any path prefixed to it. That is, we never hit
line 1480 in ltdl.c:try_dlopen:
1480 if (tryall_dlopen (newhandle, attempt, advise, 0) != 0)
1481{
1482 newhandle = NULL;
1483}
which WOULD HAVE succeeded using the 'last' loader. So the test fails
simply because /usr/bin/last.exe exists, causing access
(/usr/bin/last, ...) to return 0 rather than -1.
NOTE that it makes no difference whether the loader is first in the
chain or last: I created /usr/bin/first.exe (since our test of the
'first' loader tries to load a module named 'first') and I got the same
error, only this time (a) much earlier in main(), and (b) with
/usr/bin/first. All loaders will encounter the code around
ltdl.c(try_dlopen):1466, with the behavior below:
I guess the real question is, why is
tryall_dlopen (..., attempt, ...)
the last resort when attempt is a bare name, after each of
find_handle
Re: lt_dlerror changes
On 6/18/2010 10:27 AM, Peter O'Gorman wrote: This should (hopefully) stop the preopen loader getting used twice, It does, and I think this is the right thing to do here. But... and perhaps (I'm really dreaming now) give a better error. Nope, doesn't help this issue. But I think clobbering the error message is a red herring; we're actually seeing the error set by the final loader that is called. The final loader called, for /usr/bin/last -- which exists -- is the 'last' loader. In last_open(), it sets LT_ERROR_FILE_NOT_FOUND as a sort of generic error value -- which, strangely, is exactly the OPPOSITE of the problem here. The real problem is there actually IS a file named /usr/bin/last, but because that name does not match exactly last, last_open() reports what it considers a generic error. -- Chuck
Re: lt_dlerror changes
On Fri, 18 Jun 2010, Charles Wilson wrote: The final loader called, for /usr/bin/last -- which exists -- is the 'last' loader. In last_open(), it sets LT_ERROR_FILE_NOT_FOUND as a sort of generic error value -- which, strangely, is exactly the OPPOSITE of the problem here. The real problem is there actually IS a file named /usr/bin/last, but because that name does not match exactly last, last_open() reports what it considers a generic error. I seem to be backlogged by the flurry of email, but hopefully you guys will get this figured out. I would be astonished if Windows LoadLibrary() is not willing to load a Windows executable file similar to the way it loads a DLL. If so, this raises security implications that we want to avoid. Bob -- Bob Friesenhahn bfrie...@simple.dallas.tx.us, http://www.simplesystems.org/users/bfriesen/ GraphicsMagick Maintainer,http://www.GraphicsMagick.org/
Re: lt_dlerror changes
On 6/18/2010 2:32 PM, Bob Friesenhahn wrote: The real problem is there actually IS a file named /usr/bin/last, but because that name does not match exactly last, last_open() reports what it considers a generic error. I seem to be backlogged by the flurry of email, but hopefully you guys will get this figured out. I would be astonished if Windows LoadLibrary() is not willing to load a Windows executable file similar to the way it loads a DLL. Well, there IS one difference AFAIK: if a file exists named C:\temp\bob.dll and I try to LoadLibrary C:\temp\bob, it will succeed because LoadLibrary will automatically append a .dll (unless you put a trailing '.' on it): http://msdn.microsoft.com/en-us/library/ms684175%28VS.85%29.aspx If no file name extension is specified in the lpFileName parameter, the default library extension .dll is appended. However, the file name string can include a trailing point character (.) to indicate that the module name has no extension. When no path is specified, the function searches for loaded modules whose base name matches the base name of the module to be loaded. If the name matches, the load succeeds. Otherwise, the function searches for the file. But it won't automatically append .exe. In this case, we have a disagreement betweeen Windows and cygwin -- cygwin's access() function (and stat, and fopen, and lots of other things) will automatically check for bob.exe when asked about bob. So, libltdl thinks /usr/bin/last exists, because C:\cygwin\bin\last.exe does. However, LoadLibrary, when passed the dos-ified version of /usr/bin/last -- that is, C:\cygwin\bin\last with no extension -- fails, because LoadLibrary doesn't try to append .exe like cygwin's file access functions do. If so, this raises security implications that we want to avoid. I don't think so. -- Chuck
Re: lt_dlerror changes
On Fri, 18 Jun 2010, Charles Wilson wrote: If so, this raises security implications that we want to avoid. I don't think so. Hopefully not. If a binary from an executable program is placed at the path C:\cygwin\bin\last (with no .exe extension) does LoadLibrary() load it? Since we are on the subject, it is good to make sure that Windows really is in good shape security-wise. Windows paranoia about downloaded files might go away if the file extension is missing so it is good to know if it will still attempt to load an exectuable or DLL which has its file extension missing. Bob -- Bob Friesenhahn bfrie...@simple.dallas.tx.us, http://www.simplesystems.org/users/bfriesen/ GraphicsMagick Maintainer,http://www.GraphicsMagick.org/
Re: lt_dlerror changes
On 6/18/2010 2:52 PM, Bob Friesenhahn wrote: On Fri, 18 Jun 2010, Charles Wilson wrote: If so, this raises security implications that we want to avoid. I don't think so. Hopefully not. If a binary from an executable program is placed at the path C:\cygwin\bin\last (with no .exe extension) does LoadLibrary() load it? On Vista, no. I moved last.exe out of the way (last-foo). I copied last-foo to 'last' with no extension, and verified that it did not, in fact, have the .exe added. I then stepped thru the code, and both dlopen and LoadLibrary returned null when given (in either unix or dos format, as appropriate) /usr/bin/last However, since this exe image had no exports, I thought perhaps it might be failing for that reason. So, I copied a DLL to /usr/bin/last, and tried again. Same story: neither dlopen nor LoadLibrary opened it. Now, this is on Vista. I dunno how earlier OSes might react. Since we are on the subject, it is good to make sure that Windows really is in good shape security-wise. Ooooh, oooh, pick me, pick me -- I know this one... It (meaning windows, not necessarily libltdl on windows) isn't. But we're NOT on the subject. We're talking about a patch for lt_dlerror, and how it -- or, with slight modifications to the machinery the patch puts into place -- enables easier debugging of a long-standing cygwin regression. Now, however, we've drifted off topic more thoroughly into the details of that cygwin regression and that's my fault. Sorry Peter. But we shouldn't go off on yet another tangent, so if we want to start a new Let's make Bill Gates' masterpiece secure for him thread, let's do that and not make this one more incoherent than it already is. Windows paranoia about downloaded files might go away if the file extension is missing so it is good to know if it will still attempt to load an exectuable or DLL which has its file extension missing. Well, apparently it won't -- on Vista. -- Chuck
lt_dlerror changes
Hi,
Well, this is what I ended up with, it does not change the currently
documented saving of error messages until lt_dlerror() is called, it
copies the error message to ensure that we don't return garbage when
lt_dlerror is called. I think I also got all the places where we were
setting file not found.
Still, I am not overjoyed with this.
Peter
From 40202fade8f55891b5f4acfee54eba02636b91e8 Mon Sep 17 00:00:00 2001
From: Peter O'Gorman pe...@pogma.com
Date: Thu, 17 Jun 2010 12:42:28 -0500
Subject: [PATCH] Improve libltdl error messages.
* libltdl/lt_error.c: Add new functions to copy error messages
and keep them until lt_dlerror() is called.
* libltdl/libltdl/lt__private.h: Prototypes for new functions,
new macros.
* libltdl/ltdl.c: Use new macros and functions.
* libltdl/loaders/preopen.c: Likewise.
* tests/lt_dlerror.at: New test.
* Makefile.am: Add new test.
* tests/lt_dlopen.at: Make it pass.
---
ChangeLog | 13
Makefile.am |1 +
libltdl/libltdl/lt__private.h | 14 +++-
libltdl/loaders/preopen.c | 10 ++-
libltdl/lt_error.c| 44 +-
libltdl/ltdl.c| 141 ++---
tests/lt_dlerror.at | 88 +
tests/lt_dlopen.at|7 +--
8 files changed, 253 insertions(+), 65 deletions(-)
create mode 100644 tests/lt_dlerror.at
diff --git a/ChangeLog b/ChangeLog
index a5676ef..c6f0179 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2010-06-17 Peter O'Gorman pe...@pogma.com
+
+ Improve libltdl error messages.
+ * libltdl/lt_error.c: Add new functions to copy error messages
+ and keep them until lt_dlerror() is called.
+ * libltdl/libltdl/lt__private.h: Prototypes for new functions,
+ new macros.
+ * libltdl/ltdl.c: Use new macros and functions.
+ * libltdl/loaders/preopen.c: Likewise.
+ * tests/lt_dlerror.at: New test.
+ * Makefile.am: Add new test.
+ * tests/lt_dlopen.at: Make it pass.
+
2010-06-16 Ralf Wildenhues ralf.wildenh...@gmx.de
Optimize func_ltwrapper_scriptname to assume a cwrapper.
diff --git a/Makefile.am b/Makefile.am
index d0688ee..40fce35 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -488,6 +488,7 @@ TESTSUITE_AT = tests/testsuite.at \
tests/lt_dlopen.at \
tests/lt_dlopen_a.at \
tests/lt_dlopenext.at \
+ tests/lt_dlerror.at \
tests/ltdl-libdir.at \
tests/ltdl-api.at \
tests/dlloader-api.at \
diff --git a/libltdl/libltdl/lt__private.h b/libltdl/libltdl/lt__private.h
index f4c4a3d..dc043cc 100644
--- a/libltdl/libltdl/lt__private.h
+++ b/libltdl/libltdl/lt__private.h
@@ -137,13 +137,19 @@ struct lt__advise {
#define LT__STRERROR(name) lt__error_string(LT_CONC(LT_ERROR_,name))
#define LT__GETERROR(lvalue) (lvalue) = lt__get_last_error()
-#define LT__SETERRORSTR(errormsg) lt__set_last_error(errormsg)
-#define LT__SETERROR(errorcode) LT__SETERRORSTR(LT__STRERROR(errorcode))
+#define LT__SETERRORSTR(errormsg) lt__set_last_error(errormsg, 0)
+#define LT__SETERROR(errorcode) if (0 == lt__get_last_error()) \
+ LT__SETERRORSTR(LT__STRERROR(errorcode))
+#define LT__ENTER_PUBLIC_FUNC(x) lt__enter_err()
+#define LT__PUBLIC_FUNC_RETURN(x) lt__keep_error(); return x;
+#define LT__FORCEERROR(errorstr) LT__SETERRORSTR(errorstr)
LT_SCOPE const char *lt__error_string (int errorcode);
LT_SCOPE const char *lt__get_last_error (void);
-LT_SCOPE const char *lt__set_last_error (const char *errormsg);
-
+LT_SCOPE const char *lt__set_last_error (const char *errormsg, int mustfree);
+LT_SCOPE voidlt__keep_error (void);
+LT_SCOPE voidlt__enter_err (void);
+LT_SCOPE const char *lt__dlerror(void);
LT_END_C_DECLS
#endif /*!defined(LT__PRIVATE_H)*/
diff --git a/libltdl/loaders/preopen.c b/libltdl/loaders/preopen.c
index 7149287..8b14121 100644
--- a/libltdl/loaders/preopen.c
+++ b/libltdl/loaders/preopen.c
@@ -186,7 +186,6 @@ vm_open (lt_user_data LT__UNUSED loader_data, const char *filename,
}
LT__SETERROR (FILE_NOT_FOUND);
-
done:
return module;
}
@@ -292,8 +291,9 @@ add_symlist (const lt_dlsymlist *symlist)
int
lt_dlpreload_default (const lt_dlsymlist *preloaded)
{
+ LT__ENTER_PUBLIC_FUNC ();
default_preloaded_symbols = preloaded;
- return 0;
+ LT__PUBLIC_FUNC_RETURN (0);
}
@@ -303,6 +303,7 @@ int
lt_dlpreload (const lt_dlsymlist *preloaded)
{
int errors = 0;
+ LT__ENTER_PUBLIC_FUNC ();
if (preloaded)
{
@@ -318,7 +319,7 @@ lt_dlpreload (const lt_dlsymlist *preloaded)
}
}
- return errors;
+ LT__PUBLIC_FUNC_RETURN (errors);
}
@@ -331,6 +332,7 @@ lt_dlpreload_open (const char *originator, lt_dlpreload_callback_func *func)
symlist_chain *list;
int errors = 0;
int found = 0;
+ LT__ENTER_PUBLIC_FUNC ();
/* For each symlist in the chain... */
for (list = preloaded_symlists; list; list = list-next)
@@ -371,5 +373,5 @@
Re: lt_dlerror changes
On 6/17/2010 4:54 PM, Peter O'Gorman wrote: Well, this is what I ended up with, it does not change the currently documented saving of error messages until lt_dlerror() is called, it copies the error message to ensure that we don't return garbage when lt_dlerror is called. I think I also got all the places where we were setting file not found. Still, I am not overjoyed with this. Fails on cygwin. However, adding -no-undefined fixes that. AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o good-plugin.la -rpath $libdir ]dnl --here- [-module -avoid-version good-plugin.lo], [], [ignore], [ignore]) Unfortunately, this doesn't magically assist solving my problem with 71. dlloader-api.at:23: FAILED (dlloader-api.at:422) but that's not a reason to object to the patch. -- Chuck
Re: lt_dlerror changes
On 06/17/2010 08:36 PM, Charles Wilson wrote: On 6/17/2010 4:54 PM, Peter O'Gorman wrote: Well, this is what I ended up with, it does not change the currently documented saving of error messages until lt_dlerror() is called, it copies the error message to ensure that we don't return garbage when lt_dlerror is called. I think I also got all the places where we were setting file not found. Still, I am not overjoyed with this. Fails on cygwin. However, adding -no-undefined fixes that. AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o good-plugin.la -rpath $libdir ]dnl --here- [-module -avoid-version good-plugin.lo], [], [ignore], [ignore]) Unfortunately, this doesn't magically assist solving my problem with 71. dlloader-api.at:23: FAILED (dlloader-api.at:422) but that's not a reason to object to the patch. Well that sucks :( I suggest changing the LT__SETERROR macro in libltdl/libltdl/lt__private.h to print __FILE__ and __LINE__ etc. if errorcode == FILE_NOT_FOUND, then running test 71 again. It doesn't look like it's happening in loadlibrary.c. Thanks for checking this out! Peter
Re: lt_dlerror changes
On 6/17/2010 10:24 PM, Peter O'Gorman wrote:
Unfortunately, this doesn't magically assist solving my problem with
71. dlloader-api.at:23: FAILED (dlloader-api.at:422)
but that's not a reason to object to the patch.
Well that sucks :(
I suggest changing the LT__SETERROR macro in
libltdl/libltdl/lt__private.h to print __FILE__ and __LINE__ etc. if
errorcode == FILE_NOT_FOUND, then running test 71 again. It doesn't look
like it's happening in loadlibrary.c.
../libtool/libltdl/loaders/preopen.c (188)
../libtool/libltdl/loaders/preopen.c (188)
../libtool/libltdl/loaders/preopen.c (188)
Hmm. Adding more debug output to preopen.c:vm_open ...
Searching for preloaded symbol table for dlopen.a
Found preloaded symbol table for libltdlc
Found preloaded symbol table for dlopen.a
oops, bailing at line 183
Searching for preloaded symbol table for loadlibrary.a
Found preloaded symbol table for libltdlc
Found preloaded symbol table for dlopen.a
Found preloaded symbol table for dlopen_LTX_get_vtable
Found preloaded symbol table for loadlibrary.a
oops, bailing at line 183
Searching for preloaded symbol table for first.a
Found preloaded symbol table for libltdlc
Found preloaded symbol table for dlopen.a
Found preloaded symbol table for dlopen_LTX_get_vtable
Found preloaded symbol table for loadlibrary.a
Found preloaded symbol table for loadlibrary_LTX_get_vtable
Found preloaded symbol table for @PROGRAM@
../libtool/libltdl/loaders/preopen.c (191)
Searching for preloaded symbol table for module.a
Found preloaded symbol table for libltdlc
Found preloaded symbol table for dlopen.a
Found preloaded symbol table for dlopen_LTX_get_vtable
Found preloaded symbol table for loadlibrary.a
Found preloaded symbol table for loadlibrary_LTX_get_vtable
Found preloaded symbol table for @PROGRAM@
../libtool/libltdl/loaders/preopen.c (191)
Searching for preloaded symbol table for last.a
Found preloaded symbol table for libltdlc
Found preloaded symbol table for dlopen.a
Found preloaded symbol table for dlopen_LTX_get_vtable
Found preloaded symbol table for loadlibrary.a
Found preloaded symbol table for loadlibrary_LTX_get_vtable
Found preloaded symbol table for @PROGRAM@
../libtool/libltdl/loaders/preopen.c (191)
Searching for preloaded symbol table for /usr/bin/last
Found preloaded symbol table for libltdlc
Found preloaded symbol table for dlopen.a
Found preloaded symbol table for dlopen_LTX_get_vtable
Found preloaded symbol table for loadlibrary.a
Found preloaded symbol table for loadlibrary_LTX_get_vtable
Found preloaded symbol table for @PROGRAM@
Err...why is it looking for a module named /usr/bin/last? Is it
somehow using a $PATH search, and matching last to
/usr/bin/last.exe? cygwin's dlopen does do some funky things with
$PATH, because that's how windows looks for DLLs -- but we're in
preopen, here...
Peter, can you try the attached patch on top of your mods, on some
platform OTHER than cygwin, and tell me what output you get?
--
Chuck
diff --git a/libltdl/loaders/preopen.c b/libltdl/loaders/preopen.c
index 7149287..8249c7d 100644
--- a/libltdl/loaders/preopen.c
+++ b/libltdl/loaders/preopen.c
@@ -163,11 +163,13 @@ vm_open (lt_user_data LT__UNUSED loader_data, const char *filename,
filename = @PROGRAM@;
}
+ fprintf(stderr, Searching for preloaded symbol table for %s\n, filename);
for (lists = preloaded_symlists; lists; lists = lists-next)
{
const lt_dlsymlist *symbol;
for (symbol= lists-symlist; symbol-name; ++symbol)
{
+ fprintf (stderr, Found preloaded symbol table for %s\n, symbol-name);
if (!symbol-address streq (symbol-name, filename))
{
/* If the next symbol's name and address is 0, it means
@@ -178,6 +180,7 @@ vm_open (lt_user_data LT__UNUSED loader_data, const char *filename,
const lt_dlsymlist *next_symbol = symbol +1;
if (next_symbol-address next_symbol-name)
{
+ fprintf(stderr, oops, bailing at line %d\n, __LINE__);
module = (lt_module) lists-symlist;
goto done;
}
--- lt__private.h-bak 2010-06-17 22:33:21.82020 -0400
+++ lt__private.h 2010-06-17 22:53:39.98860 -0400
@@ -138,8 +138,13 @@
#define LT__GETERROR(lvalue) (lvalue) = lt__get_last_error()
#define LT__SETERRORSTR(errormsg) lt__set_last_error(errormsg, 0)
-#define LT__SETERROR(errorcode) if (0 == lt__get_last_error()) \
- LT__SETERRORSTR(LT__STRERROR(errorcode))
+#define LT__SETERROR(errorcode) do { if (0 == lt__get_last_error()) \
+ if (LT_CONC(LT_ERROR_,FILE_NOT_FOUND) == LT_CONC(LT_ERROR_,errorcode)) { \
+ fprintf (stderr, %s (%d)\n, __FILE__, __LINE__); \
+ LT__SETERRORSTR(LT__STRERROR(errorcode)); \
+ } else { \
+ LT__SETERRORSTR(LT__STRERROR(errorcode)); \
+ }} while (0)
#define LT__ENTER_PUBLIC_FUNC(x) lt__enter_err()
#define LT__PUBLIC_FUNC_RETURN(x) lt__keep_error(); return x;
#define LT__FORCEERROR(errorstr) LT__SETERRORSTR(errorstr)
Re: lt_dlerror changes
On 06/17/2010 10:21 PM, Charles Wilson wrote: On 6/17/2010 10:24 PM, Peter O'Gorman wrote: Unfortunately, this doesn't magically assist solving my problem with 71. dlloader-api.at:23: FAILED (dlloader-api.at:422) but that's not a reason to object to the patch. Well that sucks :( I suggest changing the LT__SETERROR macro in libltdl/libltdl/lt__private.h to print __FILE__ and __LINE__ etc. if errorcode == FILE_NOT_FOUND, then running test 71 again. It doesn't look like it's happening in loadlibrary.c. ../libtool/libltdl/loaders/preopen.c (188) ../libtool/libltdl/loaders/preopen.c (188) ../libtool/libltdl/loaders/preopen.c (188) Hmm. Adding more debug output to preopen.c:vm_open ... Searching for preloaded symbol table for dlopen.a Found preloaded symbol table for libltdlc Found preloaded symbol table for dlopen.a oops, bailing at line 183 Searching for preloaded symbol table for loadlibrary.a Found preloaded symbol table for libltdlc Found preloaded symbol table for dlopen.a Found preloaded symbol table for dlopen_LTX_get_vtable Found preloaded symbol table for loadlibrary.a oops, bailing at line 183 Searching for preloaded symbol table for first.a Found preloaded symbol table for libltdlc Found preloaded symbol table for dlopen.a Found preloaded symbol table for dlopen_LTX_get_vtable Found preloaded symbol table for loadlibrary.a Found preloaded symbol table for loadlibrary_LTX_get_vtable Found preloaded symbol table for @PROGRAM@ ../libtool/libltdl/loaders/preopen.c (191) Searching for preloaded symbol table for module.a Found preloaded symbol table for libltdlc Found preloaded symbol table for dlopen.a Found preloaded symbol table for dlopen_LTX_get_vtable Found preloaded symbol table for loadlibrary.a Found preloaded symbol table for loadlibrary_LTX_get_vtable Found preloaded symbol table for @PROGRAM@ ../libtool/libltdl/loaders/preopen.c (191) Searching for preloaded symbol table for last.a Found preloaded symbol table for libltdlc Found preloaded symbol table for dlopen.a Found preloaded symbol table for dlopen_LTX_get_vtable Found preloaded symbol table for loadlibrary.a Found preloaded symbol table for loadlibrary_LTX_get_vtable Found preloaded symbol table for @PROGRAM@ ../libtool/libltdl/loaders/preopen.c (191) Searching for preloaded symbol table for /usr/bin/last Found preloaded symbol table for libltdlc Found preloaded symbol table for dlopen.a Found preloaded symbol table for dlopen_LTX_get_vtable Found preloaded symbol table for loadlibrary.a Found preloaded symbol table for loadlibrary_LTX_get_vtable Found preloaded symbol table for @PROGRAM@ Err...why is it looking for a module named /usr/bin/last? Is it somehow using a $PATH search, and matching last to /usr/bin/last.exe? cygwin's dlopen does do some funky things with $PATH, because that's how windows looks for DLLs -- but we're in preopen, here... Peter, can you try the attached patch on top of your mods, on some platform OTHER than cygwin, and tell me what output you get? Got this on fedora 13. Peter # -*- compilation -*- 72. dlloader-api.at:23: testing ... ++ cat ++ cat ++ cat ++ case $host_os in ++ : -I/home/pogma/gitco/cw/../libtool/libltdl ++ : /home/pogma/gitco/cw/tests/../libltdl/libltdlc.la ++ set +x ../../libtool/tests/dlloader-api.at:400: case $LIBLTDL in #( */_inst/lib/*) test -f $LIBLTDL || (exit 77) ;; esac Not enabling shell tracing (command contains an embedded newline) stdout: ++ CPPFLAGS='-I/home/pogma/gitco/cw/../libtool/libltdl ' ++ set +x ../../libtool/tests/dlloader-api.at:406: $LIBTOOL --mode=compile $CC $CPPFLAGS $CFLAGS -c module.c ++ /home/pogma/gitco/cw/libtool --mode=compile gcc -I/home/pogma/gitco/cw/../libtool/libltdl -g -O2 -c module.c stderr: stdout: libtool: compile: gcc -I/home/pogma/gitco/cw/../libtool/libltdl -g -O2 -c module.c -fPIC -DPIC -o .libs/module.o libtool: compile: gcc -I/home/pogma/gitco/cw/../libtool/libltdl -g -O2 -c module.c -o module.o /dev/null 21 ++ set +x ../../libtool/tests/dlloader-api.at:408: $LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o module.la -rpath /nowhere -module -avoid-version -no-undefinedmodule.lo ++ /home/pogma/gitco/cw/libtool --mode=link gcc -g -O2 -o module.la -rpath /nowhere -module -avoid-version -no-undefined module.lo stderr: stdout: libtool: link: gcc -shared .libs/module.o -Wl,-soname -Wl,module.so -o .libs/module.so libtool: link: ar cru .libs/module.a module.o libtool: link: ranlib .libs/module.a libtool: link: ( cd .libs rm -f module.la ln -s ../module.la module.la ) ++ . ./module.la +++ dlname=module.so +++ library_names='module.so module.so module.so' +++ old_library=module.a +++ inherited_linker_flags= +++ dependency_libs= +++ weak_library_names= +++ current=0 +++ age=0 +++ revision=0 +++ installed=no +++ shouldnotlink=yes +++ dlopen= +++ dlpreopen= +++ libdir=/nowhere ++ set +x ../../libtool/tests/dlloader-api.at:415: test -n $dlname || (exit 77) ++ test -n module.so ++
Re: lt_dlerror changes
On 06/17/2010 10:35 PM, Peter O'Gorman wrote: On 06/17/2010 10:21 PM, Charles Wilson wrote: On 6/17/2010 10:24 PM, Peter O'Gorman wrote: Unfortunately, this doesn't magically assist solving my problem with 71. dlloader-api.at:23: FAILED (dlloader-api.at:422) but that's not a reason to object to the patch. Well that sucks :( I suggest changing the LT__SETERROR macro in libltdl/libltdl/lt__private.h to print __FILE__ and __LINE__ etc. if errorcode == FILE_NOT_FOUND, then running test 71 again. It doesn't look like it's happening in loadlibrary.c. ../libtool/libltdl/loaders/preopen.c (188) ../libtool/libltdl/loaders/preopen.c (188) ../libtool/libltdl/loaders/preopen.c (188) Hmm. Adding more debug output to preopen.c:vm_open ... Searching for preloaded symbol table for dlopen.a Err...why is it looking for a module named /usr/bin/last? Is it somehow using a $PATH search, and matching last to /usr/bin/last.exe? cygwin's dlopen does do some funky things with $PATH, because that's how windows looks for DLLs -- but we're in preopen, here... Ok, it's adding a loader (at the end of the list of loaders), then trying to lt_dlopen(last), that's going to try all other loaders before getting to the added loader, and in your case the loadlibrary loader finds /usr/bin/last (because /usr/bin is properly in the search path). My changes to lt_dlerror should have given you the error from loadlibrary.c, but didn't, I will check why, it looks like the patch needs more work though. Peter
