public inbox for libc-ports@sourceware.org
 help / color / mirror / Atom feed
* [PATCH roland/arm-preconfigure] Clean up ARM preconfigure.
@ 2013-03-08 22:52 Roland McGrath
  2013-03-09  1:32 ` Joseph S. Myers
  0 siblings, 1 reply; 5+ messages in thread
From: Roland McGrath @ 2013-03-08 22:52 UTC (permalink / raw)
  To: libc-ports

There are multiple problems with the ARM preconfigure script.

1. It uses "echo" and "exit" rather than proper friendly Autoconf methods
   for emitting messages and diagnosing errors.
2. Its indentation does not meet our standards for shell script fragments.
3. The submachine selection logic is only performed for
   arm*-*-linux-gnueabi* configurations, though it has nothing whatsoever
   to do with Linux.
4. The CFLAGS tweak is highly suspect.

This fixes the first three of those things.

The CFLAGS tweak at the very least needs a comment explaining what it's
there for, and why it is being done at configure time instead of just
adding to sysdep-CFLAGS in sysdeps/arm/Makefile or something like that.
But I have no idea what those rationales are, so I can't help there.


Thanks,
Roland


ports/ChangeLog.arm
2013-03-08  Roland McGrath  <roland@hack.frob.com>

	* sysdeps/arm/preconfigure.in: New file.
	* sysdeps/arm/preconfigure: Replaced with generated file.

--- a/ports/sysdeps/arm/preconfigure
+++ b/ports/sysdeps/arm/preconfigure
@@ -1,53 +1,58 @@
+# This file is generated from configure.in by Autoconf.  DO NOT EDIT!
+ # Local preconfigure fragment for sysdeps/arm
+
 case "$machine" in
 arm*)
-	base_machine=arm
-	case $config_os in
-	linux-gnueabi*)
-		# Lets ask the compiler which ARM family we've got
-		# Unfortunately it doesn't define any flags for implementations
-		# that you might pass to -mcpu or -mtune
-		# Note if you add patterns here you must ensure that
-		# an appropriate directory exists in sysdeps/arm
-		archcppflag=`echo "" |
-		$CC $CFLAGS $CPPFLAGS -E -dM - |
-		  grep '__ARM_ARCH_[0-9].*__' |
-		  sed -e 's/^#define //' -e 's/ .*//'`
+  case $config_os in
+  linux-gnueabi*)
+    if  "${CFLAGS+set}" != "set" ; then
+      CFLAGS="-g -O2"
+    fi
+    CFLAGS="$CFLAGS -fno-unwind-tables"
+    ;;
+  linux*)
+    as_fn_error $? "Old ABI no longer supported" "$LINENO" 5
+    ;;
+  esac
+
+  base_machine=arm
+  # Lets ask the compiler which ARM family we've got
+  # Unfortunately it doesn't define any flags for implementations
+  # that you might pass to -mcpu or -mtune
+  # Note if you add patterns here you must ensure that
+  # an appropriate directory exists in sysdeps/arm
+  archcppflag=`$CC $CFLAGS $CPPFLAGS -E -dM -xc /dev/null |
+    sed -n 's/^#define \(__ARM_ARCH_[0-9].*__\) .*$/\1/p'`
 
-		case x$archcppflag in
-		x__ARM_ARCH_[89]*__)
-		  machine=armv7
-		  echo "Found compiler is configured for something newer than v7 - using v7"
-		  ;;
+  case "x$archcppflag" in
+  x__ARM_ARCH_89*__)
+    machine=armv7
+    { $as_echo "$as_me:${as_lineno-$LINENO}: Found compiler is configured for something newer than v7 - using v7" >&5
+$as_echo "$as_me: Found compiler is configured for something newer than v7 - using v7" >&6;}
+    ;;
 
-		x__ARM_ARCH_7A__)
-		  machine=armv7
-		  echo "Found compiler is configured for $machine"
-		  ;;
+  x__ARM_ARCH_7A__)
+    machine=armv7
+    { $as_echo "$as_me:${as_lineno-$LINENO}: Found compiler is configured for $machine" >&5
+$as_echo "$as_me: Found compiler is configured for $machine" >&6;}
+    ;;
 
-		x__ARM_ARCH_6T2__)
-		  machine=armv6t2
-		  echo "Found compiler is configured for $machine"
-		  ;;
-		x__ARM_ARCH_6*__)
-		  machine=armv6
-		  echo "Found compiler is configured for $machine"
-		  ;;
-		*)
-		  machine=arm
-		  echo 2>&1 "arm/preconfigure: Did not find ARM architecture type; using default"
-		  ;;
-		esac
+  x__ARM_ARCH_6T2__)
+    machine=armv6t2
+    { $as_echo "$as_me:${as_lineno-$LINENO}: Found compiler is configured for $machine" >&5
+$as_echo "$as_me: Found compiler is configured for $machine" >&6;}
+    ;;
+  x__ARM_ARCH_6*__)
+    machine=armv6
+    { $as_echo "$as_me:${as_lineno-$LINENO}: Found compiler is configured for $machine" >&5
+$as_echo "$as_me: Found compiler is configured for $machine" >&6;}
+    ;;
+  *)
+    machine=arm
+    { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: arm/preconfigure: Did not find ARM architecture type; using default" >&5
+$as_echo "$as_me: WARNING: arm/preconfigure: Did not find ARM architecture type; using default" >&2;}
+    ;;
+  esac
 
-		machine=arm/$machine
-		if [ "${CFLAGS+set}" != "set" ]; then
-		  CFLAGS="-g -O2"
-		fi
-		CFLAGS="$CFLAGS -fno-unwind-tables"
-		;;
-	linux*)
-		echo "Old ABI no longer supported" 2>&1
-		exit 1
-		;;
-	esac
-	;;
+  machine=arm/$machine
 esac
--- /dev/null
+++ b/ports/sysdeps/arm/preconfigure.in
@@ -0,0 +1,53 @@
+GLIBC_PROVIDES dnl See aclocal.m4 in the top level source directory.
+# Local preconfigure fragment for sysdeps/arm
+
+case "$machine" in
+arm*)
+  case $config_os in
+  linux-gnueabi*)
+    if [ "${CFLAGS+set}" != "set" ]; then
+      CFLAGS="-g -O2"
+    fi
+    CFLAGS="$CFLAGS -fno-unwind-tables"
+    ;;
+  linux*)
+    AC_MSG_ERROR([Old ABI no longer supported])
+    ;;
+  esac
+
+  base_machine=arm
+  # Lets ask the compiler which ARM family we've got
+  # Unfortunately it doesn't define any flags for implementations
+  # that you might pass to -mcpu or -mtune
+  # Note if you add patterns here you must ensure that
+  # an appropriate directory exists in sysdeps/arm
+  archcppflag=`$CC $CFLAGS $CPPFLAGS -E -dM -xc /dev/null |
+    sed -n 's/^#define \(__ARM_ARCH_[0-9].*__\) .*$/\1/p'`
+
+  case "x$archcppflag" in
+  x__ARM_ARCH_[89]*__)
+    machine=armv7
+    AC_MSG_NOTICE([Found compiler is configured for something newer than v7 - using v7])
+    ;;
+
+  x__ARM_ARCH_7A__)
+    machine=armv7
+    AC_MSG_NOTICE([Found compiler is configured for $machine])
+    ;;
+
+  x__ARM_ARCH_6T2__)
+    machine=armv6t2
+    AC_MSG_NOTICE([Found compiler is configured for $machine])
+    ;;
+  x__ARM_ARCH_6*__)
+    machine=armv6
+    AC_MSG_NOTICE([Found compiler is configured for $machine])
+    ;;
+  *)
+    machine=arm
+    AC_MSG_WARN([arm/preconfigure: Did not find ARM architecture type; using default])
+    ;;
+  esac
+
+  machine=arm/$machine
+esac

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH roland/arm-preconfigure] Clean up ARM preconfigure.
  2013-03-08 22:52 [PATCH roland/arm-preconfigure] Clean up ARM preconfigure Roland McGrath
@ 2013-03-09  1:32 ` Joseph S. Myers
  2013-03-09  2:17   ` Roland McGrath
  0 siblings, 1 reply; 5+ messages in thread
From: Joseph S. Myers @ 2013-03-09  1:32 UTC (permalink / raw)
  To: Roland McGrath; +Cc: libc-ports

On Fri, 8 Mar 2013, Roland McGrath wrote:

> This fixes the first three of those things.

The patch is OK.

> The CFLAGS tweak at the very least needs a comment explaining what it's
> there for, and why it is being done at configure time instead of just
> adding to sysdep-CFLAGS in sysdeps/arm/Makefile or something like that.
> But I have no idea what those rationales are, so I can't help there.

It's to avoid undefined references to unwinding functions at 
configure-test time that would cause problems for -nostdlib link tests, so 
it's set in the preconfigure script and then removed again in 
sysdeps/unix/sysv/linux/arm/configure.in; it's not relevant to the build 
itself.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH roland/arm-preconfigure] Clean up ARM preconfigure.
  2013-03-09  1:32 ` Joseph S. Myers
@ 2013-03-09  2:17   ` Roland McGrath
  2013-03-11 23:57     ` Joseph S. Myers
  0 siblings, 1 reply; 5+ messages in thread
From: Roland McGrath @ 2013-03-09  2:17 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: libc-ports

> On Fri, 8 Mar 2013, Roland McGrath wrote:
> 
> > This fixes the first three of those things.
> 
> The patch is OK.

Thanks.  I'll merge it on Monday probably, or feel free to merge the branch
yourself.

> It's to avoid undefined references to unwinding functions at 
> configure-test time that would cause problems for -nostdlib link tests, so 
> it's set in the preconfigure script and then removed again in 
> sysdeps/unix/sysv/linux/arm/configure.in; it's not relevant to the build 
> itself.

Please add some comments to both files explaining the situation.


Thanks,
Roland

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH roland/arm-preconfigure] Clean up ARM preconfigure.
  2013-03-09  2:17   ` Roland McGrath
@ 2013-03-11 23:57     ` Joseph S. Myers
  2013-03-12  0:07       ` Roland McGrath
  0 siblings, 1 reply; 5+ messages in thread
From: Joseph S. Myers @ 2013-03-11 23:57 UTC (permalink / raw)
  To: Roland McGrath; +Cc: libc-ports

On Fri, 8 Mar 2013, Roland McGrath wrote:

> > It's to avoid undefined references to unwinding functions at 
> > configure-test time that would cause problems for -nostdlib link tests, so 
> > it's set in the preconfigure script and then removed again in 
> > sysdeps/unix/sysv/linux/arm/configure.in; it's not relevant to the build 
> > itself.
> 
> Please add some comments to both files explaining the situation.

I have applied this patch to add such comments.

diff --git a/ports/ChangeLog.arm b/ports/ChangeLog.arm
index 9db0b1b..8536181 100644
--- a/ports/ChangeLog.arm
+++ b/ports/ChangeLog.arm
@@ -1,3 +1,12 @@
+2013-03-11  Joseph Myers  <joseph@codesourcery.com>
+
+	* sysdeps/arm/preconfigure.in: Add comment about
+	-fno-unwind-tables addition to CFLAGS.
+	* sysdeps/arm/preconfigure: Regenerated.
+	* sysdeps/unix/sysv/linux/arm/configure.in: Add comment about
+	-fno-unwind-tables removal from CFLAGS.
+	* sysdeps/unix/sysv/linux/arm/configure: Regenerated.
+
 2013-03-11  Roland McGrath  <roland@hack.frob.com>
 
 	* sysdeps/arm/bits/setjmp.h: Test [!__ASSEMBLER__] rather than [!_ASM].
diff --git a/ports/sysdeps/arm/preconfigure b/ports/sysdeps/arm/preconfigure
index 0042aaf..7ba1749 100644
--- a/ports/sysdeps/arm/preconfigure
+++ b/ports/sysdeps/arm/preconfigure
@@ -5,6 +5,11 @@ case "$machine" in
 arm*)
   case $config_os in
   linux-gnueabi*)
+    # If the compiler enables unwind tables by default, this causes
+    # problems with undefined symbols in -nostdlib link tests.  To
+    # avoid this, add -fno-unwind-tables here and remove it in
+    # sysdeps/unix/sysv/linux/arm/configure.in after those tests have
+    # been run.
     if  "${CFLAGS+set}" != "set" ; then
       CFLAGS="-g -O2"
     fi
diff --git a/ports/sysdeps/arm/preconfigure.in b/ports/sysdeps/arm/preconfigure.in
index f3272f1..99f2128 100644
--- a/ports/sysdeps/arm/preconfigure.in
+++ b/ports/sysdeps/arm/preconfigure.in
@@ -5,6 +5,11 @@ case "$machine" in
 arm*)
   case $config_os in
   linux-gnueabi*)
+    # If the compiler enables unwind tables by default, this causes
+    # problems with undefined symbols in -nostdlib link tests.  To
+    # avoid this, add -fno-unwind-tables here and remove it in
+    # sysdeps/unix/sysv/linux/arm/configure.in after those tests have
+    # been run.
     if [ "${CFLAGS+set}" != "set" ]; then
       CFLAGS="-g -O2"
     fi
diff --git a/ports/sysdeps/unix/sysv/linux/arm/configure b/ports/sysdeps/unix/sysv/linux/arm/configure
index cb94cc5..f66b158 100644
--- a/ports/sysdeps/unix/sysv/linux/arm/configure
+++ b/ports/sysdeps/unix/sysv/linux/arm/configure
@@ -2,4 +2,5 @@
  # Local configure fragment for sysdeps/unix/sysv/linux/arm.
 
 libc_cv_gcc_unwind_find_fde=no
+# Remove -fno-unwind-tables that was added in sysdeps/arm/preconfigure.in.
 CFLAGS=${CFLAGS% -fno-unwind-tables}
diff --git a/ports/sysdeps/unix/sysv/linux/arm/configure.in b/ports/sysdeps/unix/sysv/linux/arm/configure.in
index 3e67dee..8fffe94 100644
--- a/ports/sysdeps/unix/sysv/linux/arm/configure.in
+++ b/ports/sysdeps/unix/sysv/linux/arm/configure.in
@@ -2,4 +2,5 @@ GLIBC_PROVIDES dnl See aclocal.m4 in the top level source directory.
 # Local configure fragment for sysdeps/unix/sysv/linux/arm.
 
 libc_cv_gcc_unwind_find_fde=no
+# Remove -fno-unwind-tables that was added in sysdeps/arm/preconfigure.in.
 CFLAGS=${CFLAGS% -fno-unwind-tables}

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH roland/arm-preconfigure] Clean up ARM preconfigure.
  2013-03-11 23:57     ` Joseph S. Myers
@ 2013-03-12  0:07       ` Roland McGrath
  0 siblings, 0 replies; 5+ messages in thread
From: Roland McGrath @ 2013-03-12  0:07 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: libc-ports

Thanks.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-03-12  0:07 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-08 22:52 [PATCH roland/arm-preconfigure] Clean up ARM preconfigure Roland McGrath
2013-03-09  1:32 ` Joseph S. Myers
2013-03-09  2:17   ` Roland McGrath
2013-03-11 23:57     ` Joseph S. Myers
2013-03-12  0:07       ` Roland McGrath

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).