Re: [patch] Dangling Pointer in libltdl

[Ralf Wildenhues]

* quoting myself:
> * Dave Brolley wrote on Wed, Jan 24, 2007 at 11:14:56PM CET:
> > Given time, I should be able to come up with a test case if necessary.
> 
> I have a test.  Will post and apply both when I have it cleaned up.

Here's what I've come up with and applied.  The HEAD test can also be
made to work with branch-1-5's libltdl, by something like
  make check-local TESTSUITEFLAGS='-k lt_dlexit' \
    LTDLINCL=/path/to/branch-1-5/libltdl/ \
    LIBLTDL=/path/to/branch-1-5/libltdl/libltdlc.la
    LIBTOOL=/path/to/branch-1-5/libtool

(writing from memory, I think that was it).

Cheers,
Ralf

branch-1-5:
2007-01-28  Dave Brolley  <address@hidden>

        * libltdl/ltdl.c (lt_dlexit): Make sure that 'cur' is not NULL
        before checking that it is still in the list.

Index: libltdl/ltdl.c
===================================================================
RCS file: /cvsroot/libtool/libtool/libltdl/ltdl.c,v
retrieving revision 1.174.2.26
diff -u -r1.174.2.26 ltdl.c
--- libltdl/ltdl.c      28 Jan 2007 13:40:49 -0000      1.174.2.26
+++ libltdl/ltdl.c      28 Jan 2007 14:51:56 -0000
@@ -2341,6 +2341,19 @@
              lt_dlhandle tmp = cur;
              cur = cur->next;
              if (!LT_DLIS_RESIDENT (tmp))
+                 /* Make sure that the handle pointed to by 'cur' still exists.
+                    lt_dlclose recursively closes dependent libraries which 
removes
+                    them from the linked list.  One of these might be the one
+                    pointed to by 'cur'.  */
+                 if (cur)
+                   {
+                     for (tmp = handles; tmp; tmp = tmp->next)
+                       if (tmp == cur)
+                         break;
+                     if (! tmp)
+                       cur = handles;
+                   }
+
                saw_nonresident = 1;
              if (!LT_DLIS_RESIDENT (tmp) && tmp->info.ref_count <= level)
                {

HEAD:
2007-01-28  Dave Brolley  <address@hidden>
            Ralf Wildenhues  <address@hidden>

        * libltdl/ltdl.c (lt_dlexit): Make sure that 'cur' is not NULL
        before checking that it is still in the list.
        * tests/lt_dlexit.at: New test.
        * Makefile.am (TESTSUITE_AT): Adjust.
        (check-local): Also depend on libltdl/libltdlc.la.
        (check-recursive): Removed, unnecessary use of Automake
        internals.

Index: Makefile.am
===================================================================
RCS file: /cvsroot/libtool/libtool/Makefile.am,v
retrieving revision 1.204
diff -u -r1.204 Makefile.am
--- Makefile.am 28 Jan 2007 12:43:36 -0000      1.204
+++ Makefile.am 28 Jan 2007 14:50:59 -0000
@@ -411,6 +411,7 @@
                  tests/search-path.at \
                  tests/old-m4-iface.at \
                  tests/am-subdir.at \
+                 tests/lt_dlexit.at \
                  tests/standalone.at \
                  tests/subproject.at \
                  tests/nonrecursive.at \
@@ -444,8 +445,6 @@
        LIBTOOL="$(bindir)/`echo libtool | sed '$(program_transform_name)'`" \
        tst_aclocaldir="$(aclocaldir)"
 
-check-recursive: $(srcdir)/$(TESTSUITE)
-
 # Use `$(srcdir)' for the benefit of non-GNU makes: this is
 # how `testsuite' appears in our dependencies.
 $(srcdir)/$(TESTSUITE): $(srcdir)/tests/package.m4 $(TESTSUITE_AT) Makefile.am
@@ -471,7 +470,7 @@
 CD_TESTDIR     = abs_srcdir=`$(lt__cd) $(srcdir) && pwd`; cd tests
 
 # Hook the test suite into the check rule
-check-local: tests/atconfig $(srcdir)/$(TESTSUITE)
+check-local: tests/atconfig $(srcdir)/$(TESTSUITE) libltdl/libltdlc.la
        $(CD_TESTDIR); \
        CONFIG_SHELL="$(SHELL)" $(SHELL) $$abs_srcdir/$(TESTSUITE) \
          $(TESTS_ENVIRONMENT) $(BUILDCHECK_ENVIRONMENT) $(TESTSUITEFLAGS)
Index: libltdl/ltdl.c
===================================================================
RCS file: /cvsroot/libtool/libtool/libltdl/ltdl.c,v
retrieving revision 1.245
diff -u -r1.245 ltdl.c
--- libltdl/ltdl.c      13 Oct 2006 14:11:18 -0000      1.245
+++ libltdl/ltdl.c      28 Jan 2007 14:50:59 -0000
@@ -283,6 +283,18 @@
                        {
                          ++errors;
                        }
+                     /* Make sure that the handle pointed to by 'cur' still 
exists.
+                        lt_dlclose recursively closes dependent libraries 
which removes
+                        them from the linked list.  One of these might be the 
one
+                        pointed to by 'cur'.  */
+                     if (cur)
+                       {
+                         for (tmp = handles; tmp; tmp = tmp->next)
+                           if (tmp == cur)
+                             break;
+                         if (! tmp)
+                           cur = handles;
+                       }
                    }
                }
            }
--- /dev/null   2007-01-26 00:38:36.692344249 +0100
+++ tests/lt_dlexit.at  2007-01-28 15:44:32.000000000 +0100
@@ -0,0 +1,137 @@
+# Hand crafted tests for GNU Libtool.                         -*- Autotest -*-
+# Copyright 2007 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301, USA.
+
+# Test libltdl functionality.
+# Try to keep the new interfaces of HEAD separate from those of
+# branch-1-5 to facilitate testing of older releases.
+
+AT_BANNER([Libltdl functionality.])
+
+AT_SETUP([lt_dlexit unloading libs])
+AT_KEYWORDS([libltdl])
+
+# Test for
+# http://lists.gnu.org/archive/html/bug-libtool/2007-01/msg00014.html
+
+AT_DATA([main.c],
+[[#include <ltdl.h>
+#include <stdio.h>
+#include <string.h>
+
+typedef int (*pfun_T) (int);
+typedef int *pvar_T;
+
+/* lt_dlopen wrapper */
+static lt_dlhandle
+xdlopen (const char *filename)
+{
+  lt_dlhandle handle = lt_dlopen (filename);
+  if (!handle) {
+    fprintf (stderr, "can't open the module %s!\n", filename);
+    fprintf (stderr, "error was: %s\n", lt_dlerror());
+  }
+  return handle;
+}
+
+/* lt_dlsym wrapper: try one function and one variable */
+static int
+xdlsymtest (lt_dlhandle handle, const char *func, const char *var)
+{
+  pfun_T pf = lt_dlsym (handle, func);
+  pvar_T pv = lt_dlsym (handle, var);
+  if (pf == NULL) {
+    fprintf (stderr, "function `%s' not found\n", func);
+    return 1;
+  }
+  if (pv == NULL) {
+    fprintf (stderr, "variable `%s' not found\n", var);
+    return 1;
+  }
+  return (*pf) (*pv);
+}
+
+static int
+callback (const char *filename, void *data)
+{
+  printf ("%s: %s\n", (char *)data, filename);
+  return 0;
+}
+
+static int
+try_iterate (const char *search_path)
+{
+  char *s = "try_iterate";
+  return lt_dlforeachfile (search_path, callback, s);
+}
+
+int
+main (int argc, char **argv)
+{
+  int i;
+  lt_dlhandle b1;
+
+  /* LTDL_SET_PRELOADED_SYMBOLS(); */
+  if (lt_dlinit() != 0) {
+    fprintf (stderr, "error during initialization: %s\n", lt_dlerror());
+    return 1;
+  }
+  if (!(b1 = xdlopen ("modb1.la"))) return 1;
+  if (xdlsymtest (b1, "fb1", "vb1")) return 1;
+  /* do not lt_dlclose here on purpose.  */
+
+  if (lt_dlexit() != 0) {
+    fprintf (stderr, "error during exit: %s\n", lt_dlerror());
+    return 1;
+  }
+  return 0;
+}
+]])
+
+
+AT_DATA([a1.c],
+[[int f1 (int x) { return x - 1; }
+int v1 = 1;
+]])
+
+AT_DATA([b1.c],
+[[extern int f1 (int), v1;
+int fb1 (int x) { return f1 (v1) + x - 3; }
+int vb1 = 3;
+]])
+
+: ${LTDLINCL="-I$top_srcdir/libltdl"}
+: ${LIBLTDL="$abs_builddir/../libltdl/libltdlc.la"}
+
+CPPFLAGS="$CPPFLAGS $LTDLINCL"
+LDFLAGS="$LDFLAGS -no-undefined"
+
+for file in a1 b1 main; do
+  $LIBTOOL --mode=compile $CC $CPPFLAGS $CFLAGS -c $file.c
+done
+AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o liba1.la a1.lo \
+          -rpath /foo -avoid-version], [], [ignore], [ignore])
+AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o modb1.la b1.lo \
+          -rpath /foo -module -avoid-version liba1.la], [], [ignore], [ignore])
+
+for dlopen in -dlopen -dlpreopen; do
+  AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o main main.$OBJEXT \
+           $dlopen modb1.la $LIBLTDL], [], [ignore], [ignore])
+  LT_AT_EXEC_CHECK([./main])
+done
+
+AT_CLEANUP