Re: lt_dllopenext() returns invalid handle (was: 1.1.2 is broken if you have closefrom() )
On June 15, 2006 4:30:19 PM -0400 Alan DeKok <aland@nitros9.org> wrote:
Frank Cusack <fcusack@fcusack.com> wrote:
Dunno if you didn't find time, or weren't able to reproduce it
I couldn't bear the though of fighting with that crap again.
Look at the code, it does the following:
... strcat (tmp, archive_ext); errors = try_dlopen (&handle, tmp); if (errors) return 0;
/* If we found FILENAME, stop searching -- whether we were able to load the file as a module or not. If the file exists but loading failed, it is better to return an error message here than to report FILE_NOT_FOUND when the alternatives (foo.so etc) are not in the module search path. */ if (handle || ((errors > 0) && !file_not_found ())) { LT_DLFREE (tmp); return handle; } ...
Note that if (errors > 0) AND the file was found, it returns the handle. i.e. It finds the CURRENT library, but not one that the current library needs.
The idea of returning the handle if (errors > 0) is something that is mind-blowingly retarded.
If the file was found, but errors > 0, handle is supposed to be NULL. I couldn't find a case (from inspection) where this isn't true. Dependencies are loaded before the library itself is loaded (if there is a .la file and dependencies are known), and if this fails handle is returned as NULL. If dependencies are not known, the system dlopen() is used which will implicitly load dependencies, and again, fail and return NULL if a dependency cannot be loaded. Apparently, it CAN happen that handle is not NULL, but I have to assume that this line of code doesn't think that can happen. (Because of course, it is a pretty bad idea to return a non-NULL handle on failure, and ltdl is obviously not written by idiots.) You can't just read this one line of code and say "handle is returned!", you have to look at what is expected from try_dlopen().
But anyway, it looks easy to address in freeradius (as opposed to ltdl.c) so I'm going to try that.
I don't see how. The problem is that the handle is returned, but is NOT completely populated. So ltdl_sym() de-references a NULL entry in the handle, and dies.
Since the handle structure is opaque, it's impossible for FreeRADIUS to know if the returned handle is OK, is is broken. So I have no idea how you would handle this in FreeRADIUS.
Bah. I didn't realize lt_dlhandle was opaque (because how does gdb know what it looks like). phooey. OK, we'll need to patch ltdl.c, but otherwise (ie autoconf stuff) we can still get away with untouched original source. -frank
Frank Cusack <fcusack@fcusack.com> wrote:
Apparently, it CAN happen that handle is not NULL, but I have to assume that this line of code doesn't think that can happen. (Because of course, it is a pretty bad idea to return a non-NULL handle on failure, and ltdl is obviously not written by idiots.)
Intelligent is not the same as competent. In this case, if (errors > 0), they should return NULL. That results in a memory leak if "handle" wasn't freed, but correct code. As it stands now, they have no *requirement* in that function that it returns NULL on error. It's just an accidental side effect of them sometimes freeing handle.
You can't just read this one line of code and say "handle is returned!", you have to look at what is expected from try_dlopen().
Yes. I've looked at the code, and have no idea what it's doing or why. In any case, I think I'm going to run some analysis on it in the next few weeks, so I should have results as to what execution path causes the problem, and why.
Bah. I didn't realize lt_dlhandle was opaque (because how does gdb know what it looks like). phooey. OK, we'll need to patch ltdl.c, but otherwise (ie autoconf stuff) we can still get away with untouched original source.
Ok. Alan DeKok.
I've completed the libtool and ltdl 1.5.22 update in the 1.1 branch. Please test. -frank
Frank Cusack <fcusack@fcusack.com> wrote:
I've completed the libtool and ltdl 1.5.22 update in the 1.1 branch. Please test.
... configure: configuring in src/modules/rlm_checkval configure: running /usr/pkg/bin/bash './configure' --prefix=/usr/local --enable-ltdl-install --cache-file=/dev/null --srcdir=. l ... creating Makefile creating config.h cat: ./config.h.in: No such file or directory It looks like the "configure" script in src/modules/rlm_checkval wasn't updated. It still references config.h, even though configure.in doesn't. Also, I'm seeing it trying to cache to /dev/null. Is that intentional? Alan DeKok.
On June 23, 2006 11:22:21 AM -0400 Alan DeKok <aland@nitros9.org> wrote:
Frank Cusack <fcusack@fcusack.com> wrote:
I've completed the libtool and ltdl 1.5.22 update in the 1.1 branch. Please test.
... configure: configuring in src/modules/rlm_checkval configure: running /usr/pkg/bin/bash './configure' --prefix=/usr/local --enable-ltdl-install --cache-file=/dev/null --srcdir=. l ... creating Makefile creating config.h cat: ./config.h.in: No such file or directory
It looks like the "configure" script in src/modules/rlm_checkval wasn't updated. It still references config.h, even though configure.in doesn't.
That's not part of my change; nbk did that. It fails in a lot of dirs, it's just that rlm_checkval is last so you only notice it there. If each sub-configure continues to run after failing on config.h it should be harmless. It would be nice to at least suppress the error. When you actually build, you'll also see that rlm_detail fails. This is due to changes in hashtable.
Also, I'm seeing it trying to cache to /dev/null. Is that intentional?
No. I'll look into this. -frank
Frank Cusack <fcusack@fcusack.com> wrote:
That's not part of my change; nbk did that.
OK.
If each sub-configure continues to run after failing on config.h it should be harmless. It would be nice to at least suppress the error.
Or re-generate the configure scripts?
When you actually build, you'll also see that rlm_detail fails. This is due to changes in hashtable.
That's my mistake. I'll fix it. Alan DeKok.
On Fri, Jun 23, 2006 at 12:17:50PM -0400, Alan DeKok said:
If each sub-configure continues to run after failing on config.h it should be harmless. It would be nice to at least suppress the error.
Or re-generate the configure scripts?
Does it actually need the subdirectory configure's? Can't we just add the needed test to the top level configure, and include the autoheader from that? -- -------------------------------------------------------------------------- | Stephen Gran | Drive defensively. Buy a tank. | | steve@lobefin.net | | | http://www.lobefin.net/~steve | | --------------------------------------------------------------------------
Stephen Gran <steve@lobefin.net> wrote:
Does it actually need the subdirectory configure's? Can't we just add the needed test to the top level configure, and include the autoheader From that?
Yes, and no. The configure scripts in each module do module-specific checks. The idea is to isolate everything needed to build a module like rlm_ldap. The alternative is to have one top-level configure script which contains information about *everything*. While that's what the autotools people recommend, it's unmanagable for projects like FreeRADIUS that contain multiple semi-independent sub-projects. Alan DeKok.
On Friday 23 June 2006 11:50, Frank Cusack wrote:
On June 23, 2006 11:22:21 AM -0400 Alan DeKok <aland@nitros9.org> wrote:
Also, I'm seeing it trying to cache to /dev/null. Is that intentional?
No. I'll look into this.
From autoconf 2.59 documentation: By default, `configure' uses no cache file (technically, it uses `--cache-file=/dev/null'), to avoid problems caused by accidental use of stale cache files. To enable caching, `configure' accepts `--config-cache' (or `-C') to cache results in the file `config.cache'. Alternatively, `--cache-file=FILE' specifies that FILE be the cache file. The cache file is created if it does not exist already. When `configure' calls `configure' scripts in subdirectories, it uses the `--cache-file' argument so that they share the same cache. Kevin Bonner
On June 23, 2006 2:08:54 PM -0400 Kevin Bonner <keb@pa.net> wrote:
On Friday 23 June 2006 11:50, Frank Cusack wrote:
On June 23, 2006 11:22:21 AM -0400 Alan DeKok <aland@nitros9.org> wrote:
Also, I'm seeing it trying to cache to /dev/null. Is that intentional?
No. I'll look into this.
From autoconf 2.59 documentation:
By default, `configure' uses no cache file (technically, it uses `--cache-file=/dev/null'), to avoid problems caused by accidental use of stale cache files.
Yep. The question is, is this different than what configure in fr-1.1.2 did. If so, I'll need to setup and use a cache file by default. -frank
On Friday 23 June 2006 14:28, Frank Cusack wrote:
On June 23, 2006 2:08:54 PM -0400 Kevin Bonner <keb@pa.net> wrote:
On Friday 23 June 2006 11:50, Frank Cusack wrote:
On June 23, 2006 11:22:21 AM -0400 Alan DeKok <aland@nitros9.org> wrote:
Also, I'm seeing it trying to cache to /dev/null. Is that intentional?
No. I'll look into this.
From autoconf 2.59 documentation:
By default, `configure' uses no cache file (technically, it uses `--cache-file=/dev/null'), to avoid problems caused by accidental use of stale cache files.
Yep. The question is, is this different than what configure in fr-1.1.2 did. If so, I'll need to setup and use a cache file by default.
-frank
1.1.2 used autoconf version 2.13, and it did use a config.cache file. The latest version in branch_1_1 was built with autoconf 2.59, which uses /dev/null by default. You can verify this by searching for cache_file in the configure file. -Kevin
Frank Cusack wrote:
On June 23, 2006 11:22:21 AM -0400 Alan DeKok <aland@nitros9.org> wrote:
cat: ./config.h.in: No such file or directory
It looks like the "configure" script in src/modules/rlm_checkval wasn't updated. It still references config.h, even though configure.in doesn't.
That's not part of my change; nbk did that.
Indeed. Some modules don't need a config.h, therefore their ./configure script was changed in CVS head a long time ago. I pulled these changes in branch 1.1 but didn't rerun autoconf.
It fails in a lot of dirs, it's just that rlm_checkval is last so you only notice it there. If each sub-configure continues to run after failing on config.h it should be harmless. It would be nice to at least suppress the error.
Just rerun autoconf in the subdirectories and it will be fine. Currently in branch 1.1 of the CVS you have the top-level configure generated by autoconf 2.59 and the sub-configure scripts generated by autoconf 2.13. I wouldn't be surprised if they can't cooperate very well. -- Nicolas Baradakis
participants (5)
-
Alan DeKok -
Frank Cusack -
Kevin Bonner -
Nicolas Baradakis -
Stephen Gran