diff mbox

[libbacktrace/libsanitizer/libquadmath/libcilkrts] fix multilib builds

Message ID 54C26C53.1090209@ubuntu.com
State New
Headers show

Commit Message

Matthias Klose Jan. 23, 2015, 3:44 p.m. UTC 
[CC ing build, libquadmath, libcilkrts maintainers]

On 01/23/2015 02:53 AM, Ian Lance Taylor wrote:
> On Thu, Jan 22, 2015 at 3:44 PM, Matthias Klose <doko@ubuntu.com> wrote:
>>
>> However for the libbacktrace and the libsanitizer builds, the AM_ENABLE_MULTILIB
>> macro is included way too late.  Scan the generated configure file for
>> "cross_compiling" and see that the above snippet is added after the failing
>> checks. The fix seems to be simple, just move the AM_ENABLE_MULTILIB macro up so
>> that the "cross_compiling" value is corrected before it is tested.  This is what
>> the patch is doing.
>>
>> However ... for every runtime library there is still something wrong, at least
>> one linker check in the generated configure is done before the expansion of the
>> AM_ENABLE_MULTILIB macro. I'm not trying to fix that with this patch ...
>> autoconf offers a AC_REQUIRE macro, so AM_ENABLE_MULTILIB should add one to
>> ensure that this macros is always expanded before the first check for
>> "cross_compiling", but I'm not sure which one.
>>
>> Small patch, but took me some time to find out  :-/
>>
>> This patch should go to trunk and all active branches. Ok to check in?
> 
> The libbacktrace library is unusual because we build it for both the
> host and the target.  When building for the host, we do not want to
> multilib it--that would make no sense.  You've moved the
> AM_ENABLE_MULTILIB out of the test -n "${with_target_subdir}"; don't
> do that.

ok, moved the macro into the test again.

> It's fine with me if you move AM_ENABLE_MULTILIB earlier as long as
> you keep the test.
> 
> Probably a build maintainer should take a look at this patch.

CCed, and there are more directories needing an update: libquadmath and
libcilkrts.  Note that libcilkrts didn't have an AM_ENABLE_MULTILIB call at all,
but nevertheless builds the multilib libraries.

I then tried to patch config.multi.m4 to warn when _AC_COMPILER_EXEEXT was
called before, but I didn't see any warning emitted. No idea yet how this is
supposed to work.



The attached patch just moves the AM_ENABLE_MULTILIB calls up in the files, so
that these are found before GCC_NO_EXECUTABLES (which is not called in every
libXXdir) and AC_PROC_CC.

x32 multilib enabled x86_64-linux-gnu and i686-linux-gnu configurations built
successfully on a non-x32 enabled kernel.

  Matthias

Comments

Tobias Burnus Jan. 26, 2015, 2:30 p.m. UTC | #1 
Matthias Klose wrote:
> >> However for the libbacktrace and the libsanitizer builds, the AM_ENABLE_MULTILIB
> >> macro is included way too late.  Scan the generated configure file for
> >> "cross_compiling" and see that the above snippet is added after the failing
> >> checks. The fix seems to be simple, just move the AM_ENABLE_MULTILIB macro up so
> >> that the "cross_compiling" value is corrected before it is tested.  This is what
> >> the patch is doing.
[...]
> CCed, and there are more directories needing an update: libquadmath and
> libcilkrts.  Note that libcilkrts didn't have an AM_ENABLE_MULTILIB call at all,
> but nevertheless builds the multilib libraries.

> The attached patch just moves the AM_ENABLE_MULTILIB calls up in the files, so
> that these are found before GCC_NO_EXECUTABLES (which is not called in every
> libXXdir) and AC_PROC_CC.
> 
> x32 multilib enabled x86_64-linux-gnu and i686-linux-gnu configurations built
> successfully on a non-x32 enabled kernel.

From my side, the libquadmath part of patch is okay.


Ian Lance Taylor wrote to the libbacktrace part of the patch:
> Probably a build maintainer should take a look at this patch.

I concur that that wouldn't harm.

Tobias
Jeff Law Jan. 26, 2015, 10:16 p.m. UTC | #2 
On 01/26/15 07:30, Tobias Burnus wrote:
> Matthias Klose wrote:
>>>> However for the libbacktrace and the libsanitizer builds, the AM_ENABLE_MULTILIB
>>>> macro is included way too late.  Scan the generated configure file for
>>>> "cross_compiling" and see that the above snippet is added after the failing
>>>> checks. The fix seems to be simple, just move the AM_ENABLE_MULTILIB macro up so
>>>> that the "cross_compiling" value is corrected before it is tested.  This is what
>>>> the patch is doing.
> [...]
>> CCed, and there are more directories needing an update: libquadmath and
>> libcilkrts.  Note that libcilkrts didn't have an AM_ENABLE_MULTILIB call at all,
>> but nevertheless builds the multilib libraries.
>
>> The attached patch just moves the AM_ENABLE_MULTILIB calls up in the files, so
>> that these are found before GCC_NO_EXECUTABLES (which is not called in every
>> libXXdir) and AC_PROC_CC.
>>
>> x32 multilib enabled x86_64-linux-gnu and i686-linux-gnu configurations built
>> successfully on a non-x32 enabled kernel.
>
>  From my side, the libquadmath part of patch is okay.
As is the libcilkrts part of the patch.

jeff
diff mbox

Patch

Index: b/src/libbacktrace/configure.ac
===================================================================
--- a/src/libbacktrace/configure.ac
+++ b/src/libbacktrace/configure.ac
@@ -34,6 +34,10 @@  AC_INIT(package-unused, version-unused,,
 AC_CONFIG_SRCDIR(backtrace.h)
 AC_CONFIG_HEADER(config.h)
 
+if test -n "${with_target_subdir}"; then
+  AM_ENABLE_MULTILIB(, ..)
+fi
+
 AC_CANONICAL_SYSTEM
 target_alias=${target_alias-$host_alias}
 
@@ -83,7 +87,6 @@  backtrace_supported=yes
 if test -n "${with_target_subdir}"; then
   # We are compiling a GCC library.  We can assume that the unwind
   # library exists.
-  AM_ENABLE_MULTILIB(, ..)
   BACKTRACE_FILE="backtrace.lo simple.lo"
 else
   AC_CHECK_HEADER([unwind.h],
Index: b/src/libsanitizer/configure.ac
===================================================================
--- a/src/libsanitizer/configure.ac
+++ b/src/libsanitizer/configure.ac
@@ -5,6 +5,8 @@  AC_PREREQ([2.64])
 AC_INIT(package-unused, version-unused, libsanitizer)
 AC_CONFIG_SRCDIR([include/sanitizer/common_interface_defs.h])
 
+AM_ENABLE_MULTILIB(, ..)
+
 AC_MSG_CHECKING([for --enable-version-specific-runtime-libs])
 AC_ARG_ENABLE(version-specific-runtime-libs,
 [  --enable-version-specific-runtime-libs    Specify that runtime libraries should be installed in a compiler-specific directory ],
@@ -26,7 +28,6 @@  AC_SUBST(target_alias)
 GCC_LIBSTDCXX_RAW_CXX_FLAGS
 
 AM_INIT_AUTOMAKE(foreign no-dist)
-AM_ENABLE_MULTILIB(, ..)
 AM_MAINTAINER_MODE
 
 # Calculate toolexeclibdir
--- a/src/libcilkrts/configure.ac
+++ b/src/libcilkrts/configure.ac
@@ -43,6 +43,8 @@ 
 
 AM_MAINTAINER_MODE
 
+AM_ENABLE_MULTILIB(, ..)
+
 # Build a DLL on Windows
 # AC_LIBTOOL_WIN32_DLL
 AC_PROG_CC
@@ -50,7 +52,6 @@ 
 # AC_PROG_LIBTOOL
 # AC_CONFIG_MACRO_DIR([..])
 AC_CONFIG_FILES([Makefile libcilkrts.spec])
-AM_ENABLE_MULTILIB(, ..)
 AC_FUNC_ALLOCA
 
 # Check whether the target supports protected visibility.
Index: libquadmath/configure.ac
===================================================================
--- a/src/libquadmath/configure.ac
+++ b/src/libquadmath/configure.ac
@@ -23,6 +23,8 @@ 
 AC_CANONICAL_SYSTEM
 ACX_NONCANONICAL_TARGET
 
+AM_ENABLE_MULTILIB(, ..)
+
 target_alias=${target_alias-$host_alias}
 AC_SUBST(target_alias)
 
@@ -60,7 +62,6 @@ 
 AC_SUBST(enable_static)
 
 AM_MAINTAINER_MODE
-AM_ENABLE_MULTILIB(, ..)
 
 AC_LANG_C
 # The same as in boehm-gc and libstdc++. Have to borrow it from there.