[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: mdemo ltdl failure

From: Charles Wilson
Subject: Re: mdemo ltdl failure
Date: Thu, 19 Apr 2007 18:25:27 -0400
User-agent: Thunderbird (Windows/20070221)

Hopefully the attached patch addresses all comments...Recapping:

The argz functions (specifically, argz_insert) supplied by cygwin are buggy, in wierd use-dependent malloc-related ways. I've already submitted a patch to newlib to fix that error which has been accepted
but (a) there's no telling when a new cygwin kernel with that fix will be released, and (b) libtool ought to work with any relatively recent cygwin kernel.

Testing for actual broken argz behavior, as I did in my first patch
is horrendously ugly, brittle, and all-around bad -- even if that is the "recommended" Way Of Autoconf. This patch is a refinement of an alternative, first proposed here:
where we instead check the platform and OS version for the few (only?) known-bad systems which both declare and provide the desired argz functions, but whose argz implementation is broken.

The basic idea of this patch is:

(1) if argz is found on the system, check to see if the $host_os
    and os version are such that the system argz is "known bad"

(2) if not, everything is fine

(3) if so (currently: cygwin, 1.5.24 or older), then we need to force
    use of the libltdl-provided argz
    (a) define a new symbol SYSTEM_ARGZ_IS_BROKEN
    (b) set ARGZ_H and AC_LIBOBJ, just as if we didn't find
        argz on the system
    (c) use SYSTEM_ARGZ_IS_BROKEN in lt__glibc.h along with
        HAVE_ARGZ_H to determine whether to
        #define argz* lt__argz*


One other change: you can now force the use of libltdl's argz on any system, which was not possible before when HAVE_ARGZ_H was true. BUT, doing this with HAVE_ARGZ_H true carries some risk: we've already decided whether to #define error_t ourselves, and/or set __error_t_defined, based on what we detected AFTER #including the system's argz.h -- but by forcing the use of libltdl's argz, we won't use the system's argz.h, so at compile-time we might not "see" what the error_t test "saw".

Therefore, libltdl's argz_.h needs to include most of the common places where error_t may have been "picked up" by the system argz.h. On newlib systems, this is <errno.h>. On glibc systems, this is also <errno.h>, but you must #define __need_error_t first (which glibc's argz.h does). So, I've modified libltdl's argz_.h to do that, too.

This is really a minor issue IMO. If the system argz is found, it should be used except in rare circumstances (like, it's broken). At present, the only known system where this applies is cygwin, and cygwin doesn't need the extra stuff specified in this section. But, trying to be thorough...


I've tested argzfix-3.patch (in conjunction with the functionalize wrapper generation patch) under the following conditions:

(NOTES: "works" means that mdemo-conf/mdemo-make/mdemo-exec passes. Also, I checked every libltdl ahd mdemo/mlib shared library explicitly using objdump or nm to determine whether system argz functions were imported, or whether it exported its own replacement argz functions. In each case, the results "track" with what should be expected, for each of the six test cases below)

(1) broken cygwin kernel (1.5.24-2 used, but any older would do)
     --reports that system argz is broken, builds successfully
       using libltdl's argz
     --resultant libraries and mdemo tests also work after dropping
       in a fixed cygwin kernel.

(2) fixed cygwin kernel (official snapshot from 20070330)
     --reports that system argz works, builds successfully using
       system argz
     --resultant libraries coredump if you drop in a broken cygwin
       kernel after the fact.  This is expected: broken cygwin is
       "broken" precisely because its argz facility coredumps on

(3) fixed cygwin kernel, but with 'export $lt_cv_sys_argz_works=no'.
     --reports that system argz does not work (cached), and builds
       successfully using libltdl's argz
     --resultant libraries works fine even after dropping in a
       broken cygwin kernel.

(4) linux (whose system argz is OK)
     --reports that system argz works, builds successfully using
       system argz, works.

(5) linux, but with 'export $lt_cv_sys_argz_works=no'.
     --reports that system argz does not work (cached), and builds
       successfully using libltdl's argz

(6) mingw, which doesn't have any system argz facility at all
     --because the argz functions are not declared, the section
       of the configure script that reports whether system argz
       works is not even run, nor is the cache checked.  configure
       has already decided to use the libltdl-supplied argz.
     --and, in fact, build is completed successfully using libltdl
       supplied argz, and the result works.

Also, under case (1), ran the entire testsuite.  All old-style tests pass:
All 115 tests passed
(9 tests were not run) ---> gcc-3.4.5 doesn't supply gfortran, only g77.

Under case (1), currently running the new-style testsuite. Will report that later in a follow-up message. I expect the following:
 14: Java convenience archives       FAILED (
 16: Link order of deplibs.          FAILED (
 49: Run tests with low max_cmd_len  FAILED (
which IMO are longstanding on cygwin, and certainly have nothing to do with either this patch or the 'functionalize wrapper generation' patch.

Ralf suggests testing this patch on solaris. I can't, but if Ralf does then I expect that the results will be the same as (4) and (5).


2007-04-19  Charles Wilson  <address@hidden>

        * libltdl/argz_.h: ensure error_t definition is obtained
        in same mechanism system argz.h would have.
        * libltdl/libltdl/lt__glibc.h: also detect if
        SYSTEM_ARGZ_IS_BROKEN when determining whether to re#def
        argz* functions.
        * libltdl/m4/argz.m4 (gl_FUNC_ARGZ): add new test to check
        if $host's argz facilities are known bad.

Index: libltdl/argz_.h
RCS file: /cvsroot/libtool/libtool/libltdl/argz_.h,v
retrieving revision 1.7
diff -u -r1.7 argz_.h
--- libltdl/argz_.h     25 Mar 2007 12:12:42 -0000      1.7
+++ libltdl/argz_.h     19 Apr 2007 19:35:58 -0000
@@ -32,6 +32,8 @@
 #define LT__ARGZ_H 1
 #include <stdlib.h>
+#define __need_error_t
+#include <errno.h>
 #include <sys/types.h>
 #if defined(LTDL)
Index: libltdl/libltdl/lt__glibc.h
RCS file: /cvsroot/libtool/libtool/libltdl/libltdl/lt__glibc.h,v
retrieving revision 1.8
diff -u -r1.8 lt__glibc.h
--- libltdl/libltdl/lt__glibc.h 25 Mar 2007 12:12:43 -0000      1.8
+++ libltdl/libltdl/lt__glibc.h 19 Apr 2007 19:35:58 -0000
@@ -37,7 +37,7 @@
 #  include <config.h>
-#if !defined(HAVE_ARGZ_H)
+#if !defined(HAVE_ARGZ_H) || defined(SYSTEM_ARGZ_IS_BROKEN)
 /* Redefine any glibc symbols we reimplement to import the
    implementations into our lt__ namespace so we don't ever
    clash with the system library if our clients use argz_*
Index: libltdl/m4/argz.m4
RCS file: /cvsroot/libtool/libtool/libltdl/m4/argz.m4,v
retrieving revision 1.4
diff -u -r1.4 argz.m4
--- libltdl/m4/argz.m4  25 Mar 2007 12:12:43 -0000      1.4
+++ libltdl/m4/argz.m4  19 Apr 2007 19:35:59 -0000
@@ -27,6 +27,41 @@
 AC_CHECK_FUNCS([argz_append argz_create_sep argz_insert argz_next \
        argz_stringify], [], [ARGZ_H=argz.h; AC_LIBOBJ([argz])])
+dnl if have system argz functions, allow forced use of 
+dnl libltdl-supplied implementation (and default to do so
+dnl on "known bad" systems). Could use a runtime check, but
+dnl (a) detecting malloc issues is notoriously unreliable
+dnl (b) only known system that declares argz functions,
+dnl     provides them, yet they are broken, is cygwin
+dnl     releases prior to 16-Mar-2007 (1.5.24 and earlier)
+dnl So, it's more straightforward simply to special case 
+dnl this for known bad systems.
+AS_IF([test -z "$ARGZ_H"],
+        [if argz actually works],
+        [lt_cv_sys_argz_works],
+        [AS_CASE($host_os,
+        [*cygwin*],
+            [lt_os_ver=`uname -r | $SED -e 's,^\([[0-9\.]]*\).*,\1,'`
+             lt_os_major=`echo $lt_os_ver | $SED -e 's,^\([[0-9]]*\).*,\1,'`
+             lt_os_ver_tmp=`echo $lt_os_ver | $SED -e "s,^${lt_os_major},," -e 
+             lt_os_minor=`echo $lt_os_ver_tmp | $SED -e 
+             lt_os_micro=`echo $lt_os_ver_tmp | $SED -e "s,^${lt_os_minor},," 
-e 's,^\.*,,'`
+             AS_IF([test -z "$lt_os_major"],[lt_os_major=0])
+             AS_IF([test -z "$lt_os_minor"],[lt_os_minor=0])
+             AS_IF([test -z "$lt_os_micro"],[lt_os_micro=0])
+             AS_IF([test "$lt_os_major"           -gt 1 
+                 [AS_IF([test "$lt_os_minor"      -gt 5 
+                      [AS_IF([test "$lt_os_micro" -gt 
+                          [lt_cv_sys_argz_works=no])])])],
+        [lt_cv_sys_argz_works=yes])])
+     AS_IF([test $lt_cv_sys_argz_works != yes],
+                   [This value is set to 1 to indicate that the system argz 
facility does not work])
+        ARGZ_H=argz.h
+        AC_LIBOBJ([argz])])])

reply via email to

[Prev in Thread] Current Thread [Next in Thread]