This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix build on PE-COFF targets and PR40125


* Dave Korn wrote on Sun, Nov 28, 2010 at 02:09:38PM CET:
>   I think it would be better if you separated out that second part (multilib
> improvements) into a stand-alone patch and omitted the first part altogether,
> in favour of this patch which unifies and rationalises the -no-undefined
> handling across the top-level libraries.

Yes, smaller and more logical changes are definitely better.

> config/ChangeLog:
> 
> 	* ltundefflags.m4: New file.
> 	(ACX_LT_UNDEF_FLAGS): Define.
> 
> libgfortran/ChangeLog:
> 
> 	* configure.ac: Invoke ACX_LT_UNDEF_FLAGS.
> 	* Makefile.am (LTLDFLAGS): Use lt_undef_flags.
> 	* aclocal.m4: Regenerate.
> 	* configure: Regenerate.
> 	* Makefile.in: Regenerate.
> 
> libgomp/ChangeLog:
> 
> 	* configure.ac: Invoke ACX_LT_UNDEF_FLAGS.
> 	* Makefile.am (libgomp_la_LDFLAGS): Use lt_undef_flags.
> 	* aclocal.m4: Regenerate.
> 	* configure: Regenerate.
> 	* Makefile.in: Regenerate.
> 	* testsuite/Makefile.in: Regenerate.
> 
> libjava/ChangeLog:
> 
> 	* configure.ac: Invoke ACX_LT_UNDEF_FLAGS.
> 	* configure.host (libgcj_sublib_ltflags): Use lt_undef_flags.
> 	* aclocal.m4: Regenerate.
> 	* configure: Regenerate.
> 	* Makefile.in: Regenerate.
> 	* gcj/Makefile.in: Regenerate.
> 	* include/Makefile.in: Regenerate.
> 	* testsuite/Makefile.in: Regenerate.
> 
> libobjc/ChangeLog:
> 
> 	* configure.ac (extra_ldflags_libobjc): Invoke ACX_LT_UNDEF_FLAGS.
> 	* Makefile.in (lt_undef_flags): Import AC_SUBST'd value.
> 	* aclocal.m4: Regenerate.
> 	* configure: Regenerate.
> 
> libquadmath/ChangeLog:
> 
> 	* configure.ac: Invoke ACX_LT_UNDEF_FLAGS.
> 	* Makefile.am (LTLDFLAGS): Use lt_undef_flags.
> 	(libquadmath_la_LINK): Fix typo.
> 	* aclocal.m4: Regenerate.
> 	* configure: Regenerate.
> 	* Makefile.in: Regenerate.
> 
> libssp/ChangeLog:
> 
> 	* configure.ac: Invoke ACX_LT_UNDEF_FLAGS.
> 	* Makefile.am (libssp_la_LDFLAGS): Use lt_undef_flags.
> 	* aclocal.m4: Regenerate.
> 	* configure: Regenerate.
> 	* Makefile.in: Regenerate.
> 
> libstdc++-v3/ChangeLog:
> 
> 	* configure.ac: Invoke ACX_LT_UNDEF_FLAGS.
> 	* configure.host (OPT_LDFLAGS): Use lt_undef_flags.
> 	* aclocal.m4: Regenerate.
> 	* configure: Regenerate.
> 	* Makefile.in: Regenerate.
> 	* doc/Makefile.in: Regenerate.
> 	* include/Makefile.in: Regenerate.
> 	* libsupc++/Makefile.in: Regenerate.
> 	* po/Makefile.in: Regenerate.
> 	* python/Makefile.in: Regenerate.
> 	* src/Makefile.in: Regenerate.
> 	* testsuite/Makefile.in: Regenerate.
> 
> lto-plugin/ChangeLog:
> 
> 	* configure.ac: Invoke ACX_LT_UNDEF_FLAGS.
> 	* Makefile.am (liblto_plugin_la_LDFLAGS): Use lt_undef_flags but
> 	override -bindir setting.
> 	* aclocal.m4: Regenerate.
> 	* configure: Regenerate.
> 	* Makefile.in: Regenerate.
> 
>   This patch has already passed bootstrap and install in a native
> i686-pc-cygwin build, and I'm about to try testing it in cross-compiler build.
> 
>   IainS, is there a chance that you could try this on Darwin?  1) I hope it
> might fix your current bootstrap problem as well, because the current
> libquadmath Makefile already has -no-undefined in it which this patch would
> effectively remove, 2) you might like to consider whether the -single_module
> flag should be added to lt_undef_flags since a lot of the library makefiles
> think it's useful on darwin; would it be valid everywhere?
> 
> [  Applying this patch is a tad tricky: don't forget to regenerate everything.
>  aclocal requires the args "-I ../config -I . -I .. " in every directory
> except libjava where it requires "-I . -I .. -I ../config -I libltdl".
> autoconf is required in each of the top-level subdirs touched, automake in all
> but libobjc.  Ask off-list if you want a full version of the patch with all
> the generated files in it.  ]

You can make life easier for you by either
- running 'autoreconf -v' in every directory needing updates, or
- having an '--enable-maintainer-mode' build tree lying around (enabling
  all languages).

Then you don't need to remember aclocal arguments.  Caveat for the
second strategy is <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43171>.

>   Ralf, how does this look to you?

I see several issues with the patch.

> --- config/ltundefflags.m4	(revision 0)
> +++ config/ltundefflags.m4	(revision 0)
> @@ -0,0 +1,36 @@
> +dnl Copyright (C) 2010 Free Software Foundation, Inc.
> +dnl This file is free software, distributed under the terms of the GNU
> +dnl General Public License.  As a special exception to the GNU General
> +dnl Public License, this file may be distributed as part of a program
> +dnl that contains a configuration script generated by Autoconf, under
> +dnl the same distribution terms as the rest of that program.
> +
> +dnl usage: ACX_LT_UNDEF_FLAGS([default_flags])
> +dnl Defines and AC_SUBSTs lt_undef_flags
> +
> +
> +AC_DEFUN([ACX_LT_UNDEF_FLAGS], [
> +AC_REQUIRE([AC_CANONICAL_SYSTEM])
> +
> +case $host in
> +  *-darwin*)
> +    lt_undef_flags=-undefined dynamic_lookup

This doesn't do what you think.  It temporarily sets the variable
lt_undef_flags for the duration of a call the (nonexistent?) program
dynamic_lookup.

Libtool has its undefined flag set to something like this.  Why would it
be necessary to try to override it?  Also, the libtool script will eat
those arguments not prefixed with -Wl, anyway.  To me it seems the
darwin special case is not needed at all.

> +    ;;
> +  *-cygwin* | *-mingw*)
> +    # 'host' will be top-level target in the case of a target lib,
> +    # we must compare to with_cross_host to decide if this is a native
> +    # or cross-compiler and select where to install dlls appropriately.
> +    if test -n "$with_cross_host" &&
> +	test x"$with_cross_host" != x"no"; then
> +      lt_undef_flags='-no-undefined -bindir "$(toolexeclibdir)"';
> +    else
> +      lt_undef_flags='-no-undefined -bindir "$(bindir)"';
> +    fi

The naming is not intuitive.  Why should 'lt_undef_flags' have something
to do with the installation directory?

> +    ;;
> +  *)
> +    lt_undef_flags=[$1]

Why m4 quoting here?  OTOH, shell quoting would make sense:
       lt_undef_flags="$1"

> +    ;;
> +esac
> +
> +AC_SUBST(lt_undef_flags)
> +])

Taken together it seems to me what you want is to override -bindir only.
And the macro for that should be named that way too.

A while ago, Niels Möller suggested a slightly more general strategy,
using a new directory variable 'sharedlibdir' for this thing.  It might
seems useful to move in that direction; e.g., plainly using
  -bindir $(sharedlibdir)

in the Makefile.am files, and having a small macro to call from
configure.ac that AC_SUBSTs this to '$(libdir)' everywhere else,
with special cases above.  WDYT?


> Index: libgfortran/configure.ac
> ===================================================================
> --- libgfortran/configure.ac	(revision 167194)
> +++ libgfortran/configure.ac	(working copy)
> @@ -202,6 +202,7 @@ AC_PROG_INSTALL
>  #AC_MSG_NOTICE([====== Starting libtool configuration])
>  AC_LIBTOOL_DLOPEN
>  AM_PROG_LIBTOOL
> +ACX_LT_UNDEF_FLAGS()

No need for parens if you don't pass arguments.  More instances below.

>  AC_SUBST(enable_shared)
>  AC_SUBST(enable_static)
>  #AC_MSG_NOTICE([====== Finished libtool configuration]) ; sleep 10
> Index: libgfortran/Makefile.am
> ===================================================================
> --- libgfortran/Makefile.am	(revision 167194)
> +++ libgfortran/Makefile.am	(working copy)
> @@ -31,7 +31,7 @@ version_dep =
>  endif
>  
>  LTLDFLAGS = $(shell $(SHELL) $(top_srcdir)/../libtool-ldflags $(LDFLAGS)) \
> -	    -no-undefined -bindir "$(bindir)"
> +	    $(lt_undef_flags)
>  
>  toolexeclib_LTLIBRARIES = libgfortran.la
>  toolexeclib_DATA = libgfortran.spec
> Index: libgomp/configure.ac
> ===================================================================
> --- libgomp/configure.ac	(revision 167194)
> +++ libgomp/configure.ac	(working copy)
> @@ -135,6 +135,7 @@ AM_CONDITIONAL(BUILD_INFO, test $gcc_cv_prog_makei
>  
>  # Configure libtool
>  AM_PROG_LIBTOOL
> +ACX_LT_UNDEF_FLAGS()
>  AC_SUBST(enable_shared)
>  AC_SUBST(enable_static)
>  
> Index: libgomp/Makefile.am
> ===================================================================
> --- libgomp/Makefile.am	(revision 167194)
> +++ libgomp/Makefile.am	(working copy)
> @@ -53,7 +53,7 @@ libgomp_version_dep =
>  endif
>  libgomp_version_info = -version-info $(libtool_VERSION)
>  libgomp_la_LDFLAGS = $(libgomp_version_info) $(libgomp_version_script) \
> -        -no-undefined -bindir "$(bindir)"
> +        $(lt_undef_flags)
>  libgomp_la_DEPENDENCIES = $(libgomp_version_dep)
>  libgomp_la_LINK = $(LINK) $(libgomp_la_LDFLAGS)
>  
> Index: libjava/configure.ac
> ===================================================================
> --- libjava/configure.ac	(revision 167194)
> +++ libjava/configure.ac	(working copy)
> @@ -547,6 +547,7 @@ AC_SUBST(INCLTDL)
>  AC_SUBST(LIBLTDL)
>  AC_SUBST(DIRLTDL)
>  AC_PROG_LIBTOOL
> +ACX_LT_UNDEF_FLAGS()
>  AM_PROG_GCJ
>  AM_PROG_CC_C_O
>  
> Index: libjava/configure.host
> ===================================================================
> --- libjava/configure.host	(revision 167194)
> +++ libjava/configure.host	(working copy)
> @@ -356,7 +356,7 @@ case "${host}" in
>    	BACKTRACESPEC=
>  	# Win32 DLLs are limited to 64k exported symbols each.
>  	enable_libgcj_sublibs_default=yes
> -	libgcj_sublib_ltflags='-no-undefined -bindir $(bindir) \
> +	libgcj_sublib_ltflags='$(lt_undef_flags) \
>  	    -Wl,-u,__ZN3org4ietf4jgss10GSSManagerC1Ev,-L..,-lgcj-noncore-dummy'
>  	libgcj_sublib_core_extra_deps=libgcj-noncore-dummy.dll.a
>    ;;
> Index: libobjc/configure.ac
> ===================================================================
> --- libobjc/configure.ac	(revision 167194)
> +++ libobjc/configure.ac	(working copy)
> @@ -156,14 +156,15 @@ AC_PROG_CC
>  m4_rename_force([real_PRECIOUS],[_AC_ARG_VAR_PRECIOUS])
>  
>  # extra LD Flags which are required for targets
> +ACX_LT_UNDEF_FLAGS()
>  case "${host}" in
>    *-darwin*)
>      # Darwin needs -single_module when linking libobjc
> -    extra_ldflags_libobjc=-Wl,-single_module
> +    extra_ldflags_libobjc='$(lt_undef_flags) -Wl,-single_module'
>      ;;
>    *-cygwin*|*-mingw*)
>      # Tell libtool to build DLLs on Windows
> -    extra_ldflags_libobjc='-no-undefined -bindir $(bindir)'
> +    extra_ldflags_libobjc='$(lt_undef_flags)'
>      ;;
>  esac
>  AC_SUBST(extra_ldflags_libobjc)
> Index: libobjc/Makefile.in
> ===================================================================
> --- libobjc/Makefile.in	(revision 167194)
> +++ libobjc/Makefile.in	(working copy)
> @@ -43,6 +43,7 @@ toolexeclibdir = @toolexeclibdir@
>  includedirname = @includedirname@
>  libsuffix = @libsuffix@
>  
> +lt_undef_flags = @lt_undef_flags@
>  extra_ldflags_libobjc = @extra_ldflags_libobjc@
>  
>  top_builddir = .
> Index: libquadmath/configure.ac
> ===================================================================
> --- libquadmath/configure.ac	(revision 167194)
> +++ libquadmath/configure.ac	(working copy)
> @@ -42,6 +42,7 @@ AC_MSG_RESULT($version_specific_libs)
>  
>  # Configure libtool
>  AM_PROG_LIBTOOL
> +ACX_LT_UNDEF_FLAGS()
>  AC_SUBST(enable_shared)
>  AC_SUBST(enable_static)
>  
> Index: libquadmath/Makefile.am
> ===================================================================
> --- libquadmath/Makefile.am	(revision 167194)
> +++ libquadmath/Makefile.am	(working copy)
> @@ -34,13 +34,13 @@ version_dep =
>  endif
>  
>  LTLDFLAGS = $(shell $(SHELL) $(top_srcdir)/../libtool-ldflags $(LDFLAGS)) \
> -            -no-undefined
> +            $(lt_undef_flags)
>  
>  
>  toolexeclib_LTLIBRARIES = libquadmath.la
>  libquadmath_la_LIBADD = 
>  libquadmath_la_LDFLAGS = $(LTLDFLAGS) -version-info `grep -v '^\#' $(srcdir)/libtool-version` $(version_arg) -lm
> -libquadmath_la_LINK = $(LINK) $(libgfortran_la_LDFLAGS)
> +libquadmath_la_LINK = $(LINK) $(libquadmath_la_LDFLAGS)
>  libquadmath_la_DEPENDENCIES = $(version_dep)
>  
>  include_HEADERS = quadmath.h quadmath_weak.h
> Index: libssp/configure.ac
> ===================================================================
> --- libssp/configure.ac	(revision 167194)
> +++ libssp/configure.ac	(working copy)
> @@ -148,6 +148,7 @@ fi
>  AC_SUBST(ssp_have_usable_vsnprintf)
>  
>  AM_PROG_LIBTOOL
> +ACX_LT_UNDEF_FLAGS()
>  AC_SUBST(enable_shared)
>  AC_SUBST(enable_static)
>  
> Index: libssp/Makefile.am
> ===================================================================
> --- libssp/Makefile.am	(revision 167194)
> +++ libssp/Makefile.am	(working copy)
> @@ -50,7 +50,7 @@ libssp_la_SOURCES = \
>  libssp_la_LIBADD = 
>  libssp_la_DEPENDENCIES = $(version_dep) $(libssp_la_LIBADD)
>  libssp_la_LDFLAGS = -version-info `grep -v '^\#' $(srcdir)/libtool-version` \
> -		    $(version_arg) -no-undefined -bindir "$(bindir)"
> +		    $(version_arg) $(lt_undef_flags)
>  
>  libssp_nonshared_la_SOURCES = \
>  	ssp-local.c
> Index: libstdc++-v3/configure.ac
> ===================================================================
> --- libstdc++-v3/configure.ac	(revision 167194)
> +++ libstdc++-v3/configure.ac	(working copy)
> @@ -96,6 +96,7 @@ if test "x${with_newlib}" != "xyes"; then
>    AC_LIBTOOL_DLOPEN
>  fi
>  AM_PROG_LIBTOOL
> +ACX_LT_UNDEF_FLAGS()
>  AC_SUBST(enable_shared)
>  AC_SUBST(enable_static)
>  
> Index: libstdc++-v3/configure.host
> ===================================================================
> --- libstdc++-v3/configure.host	(revision 167194)
> +++ libstdc++-v3/configure.host	(working copy)
> @@ -209,7 +209,7 @@ case "${host_os}" in
>      ;;
>    cygwin*)
>      os_include_dir="os/newlib"
> -    OPT_LDFLAGS="${OPT_LDFLAGS} -no-undefined -bindir \$(bindir)"
> +    OPT_LDFLAGS="${OPT_LDFLAGS} \$(lt_undef_flags)"
>      ;;
>    darwin | darwin[1-7] | darwin[1-7].*)
>      # On Darwin, performance is improved if libstdc++ is single-module.
> @@ -256,7 +256,7 @@ case "${host_os}" in
>    mingw32*)
>      os_include_dir="os/mingw32"
>      error_constants_dir="os/mingw32"
> -    OPT_LDFLAGS="${OPT_LDFLAGS} -no-undefined -bindir \$(bindir)"
> +    OPT_LDFLAGS="${OPT_LDFLAGS} \$(lt_undef_flags)"
>      ;;
>    netbsd*)
>      os_include_dir="os/bsd/netbsd"
> Index: lto-plugin/configure.ac
> ===================================================================
> --- lto-plugin/configure.ac	(revision 167194)
> +++ lto-plugin/configure.ac	(working copy)
> @@ -7,6 +7,7 @@ AM_MAINTAINER_MODE
>  AC_PROG_CC
>  AC_SYS_LARGEFILE
>  AM_PROG_LIBTOOL
> +ACX_LT_UNDEF_FLAGS()
>  AC_SUBST(target_noncanonical)
>  # Trying to get this information from gcc's config is tricky.
>  case $target in
> Index: lto-plugin/Makefile.am
> ===================================================================
> --- lto-plugin/Makefile.am	(revision 167194)
> +++ lto-plugin/Makefile.am	(working copy)
> @@ -15,7 +15,8 @@ libexecsub_LTLIBRARIES = liblto_plugin.la
>  liblto_plugin_la_SOURCES = lto-plugin.c
>  liblto_plugin_la_LIBADD = \
>  	$(if $(wildcard ../libiberty/pic/libiberty.a),../libiberty/pic/libiberty.a,)
> -liblto_plugin_la_LDFLAGS = -no-undefined -bindir $(libexecsubdir) \
> +# Note that we intentionally override the bindir supplied by ACX_LT_UNDEF_FLAGS
> +liblto_plugin_la_LDFLAGS = $(lt_undef_flags) -bindir $(libexecsubdir) \
>  	$(if $(wildcard ../libiberty/pic/libiberty.a),,-Wc,../libiberty/libiberty.a)
>  
>  all: copy_lto_plugin


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]