diff mbox series

[build] Fix HAVE_GAS_CFI_DIRECTIVE for x86_64-pc-solaris2.*

Message ID yddinbie5e7.fsf@CeBiTec.Uni-Bielefeld.DE
State New
Headers show
Series [build] Fix HAVE_GAS_CFI_DIRECTIVE for x86_64-pc-solaris2.* | expand

Commit Message

Rainer Orth Jan. 31, 2018, 9:36 a.m. UTC 
When compiling MariaDB 10.2 (which uses cfi directives in inline asm,
but gets the conditionals wrong) on Solaris 11.4 with an
x86_64-pc-solaris2.11 gcc using gas, I noticed that this one incorrectly
doesn't use cfi directives although it could/should.

Looking closer, I noticed that the logic in gcc/configure.ac's
gcc_cv_as_cfi_directive test is not only hard to follow due to deeply
nested ifs, but plain wrong in the current case: for 64-bit Solaris/x86,
the .eh_frame section is (and should be) read-only, unlike the 32-bit
case, but is incorrectly rejected.

Unlike what we currently do, configure.ac needs to verify that both the
32 and 64-bit .eh_frame sections are as the toolchain requires them to
be, which is what the current patch does.

Since .eh_frame section test already appeared repeatedly, making the
whole code hard to read due to the redundancy, I've moved it to a shell
function.  While gcc/configure.ac itself currently doesn't use those
directly, the generated configure already does, so we should be save
here.

Tested with a standalone script with the necessary boilerplate added on
Solaris 10, 11.3 and 11.4 with /bin/as, bundled gas (2.15, 2.23.1,
2.29), and gas 2.30 on both sparc and x86.

Also bootstrapped without regressions on i386-pc-solaris2.11 and
sparc-sun-solaris2.11 (as/ld, gas/ld, both 32 and 64-bit).

While this only touches Solaris-specific code, it would be nice if one
of the build maintainers could have a look.

Ok for mainline?

	Rainer

Comments

Rainer Orth Feb. 6, 2018, 6:36 p.m. UTC | #1 
Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:

> When compiling MariaDB 10.2 (which uses cfi directives in inline asm,
> but gets the conditionals wrong) on Solaris 11.4 with an
> x86_64-pc-solaris2.11 gcc using gas, I noticed that this one incorrectly
> doesn't use cfi directives although it could/should.
>
> Looking closer, I noticed that the logic in gcc/configure.ac's
> gcc_cv_as_cfi_directive test is not only hard to follow due to deeply
> nested ifs, but plain wrong in the current case: for 64-bit Solaris/x86,
> the .eh_frame section is (and should be) read-only, unlike the 32-bit
> case, but is incorrectly rejected.
>
> Unlike what we currently do, configure.ac needs to verify that both the
> 32 and 64-bit .eh_frame sections are as the toolchain requires them to
> be, which is what the current patch does.
>
> Since .eh_frame section test already appeared repeatedly, making the
> whole code hard to read due to the redundancy, I've moved it to a shell
> function.  While gcc/configure.ac itself currently doesn't use those
> directly, the generated configure already does, so we should be save
> here.
>
> Tested with a standalone script with the necessary boilerplate added on
> Solaris 10, 11.3 and 11.4 with /bin/as, bundled gas (2.15, 2.23.1,
> 2.29), and gas 2.30 on both sparc and x86.
>
> Also bootstrapped without regressions on i386-pc-solaris2.11 and
> sparc-sun-solaris2.11 (as/ld, gas/ld, both 32 and 64-bit).
>
> While this only touches Solaris-specific code, it would be nice if one
> of the build maintainers could have a look.
>
> Ok for mainline?

After a week with no word from the build maintainers, I've installed the
patch on mainline since it's purely Solaris-specific.

	Rainer
diff mbox series

Patch

# HG changeset patch
# Parent  f67a82dfb4ca8993e0153ceba8d8b064da71ad4d
Fix HAVE_GAS_CFI_DIRECTIVE for x86_64-pc-solaris2.*

diff --git a/gcc/configure.ac b/gcc/configure.ac
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -2754,6 +2754,13 @@  L2:],
   [AC_DEFINE(HAVE_AS_LEB128, 0,
     [Define if your assembler supports .sleb128 and .uleb128.])])
 
+# Determine if an .eh_frame section is read-only.
+gcc_fn_eh_frame_ro () {
+  $gcc_cv_as $1 -o conftest.o conftest.s > /dev/null 2>&1 && \
+    $gcc_cv_objdump -h conftest.o 2>/dev/null | \
+    sed -e '/.eh_frame/!d' -e N | grep READONLY > /dev/null
+}
+
 # Check if we have assembler support for unwind directives.
 gcc_GAS_CHECK_FEATURE([cfi directives], gcc_cv_as_cfi_directive,
   ,,
@@ -2769,41 +2776,41 @@  gcc_GAS_CHECK_FEATURE([cfi directives], 
     # If the linker used on Solaris (like Sun ld) isn't capable of merging
     # read-only and read-write sections, we need to make sure that the
     # assembler used emits read-write .eh_frame sections.
-    if test "x$gcc_cv_ld_ro_rw_mix" != xread-write; then
-      if test "x$gcc_cv_objdump" != x; then
-	if $gcc_cv_objdump -h conftest.o 2>/dev/null | \
-		sed -e /.eh_frame/!d -e N | grep READONLY > /dev/null; then
-	  gcc_cv_as_cfi_directive=no
-	else
-	  case "$target" in
-	    i?86-*-solaris2.1[[0-9]]* | x86_64-*-solaris2.1[[0-9]]*)
-	      # On Solaris/x86, make sure that GCC and assembler agree on using
-	      # read-only .eh_frame sections for 64-bit.
-	      if test x$gas = xyes; then
-	         as_ix86_64_opt="--64"
-	      else
-	         as_ix86_64_opt="-xarch=amd64"
-	      fi
-	      if $gcc_cv_as $as_ix86_64_opt -o conftest.o conftest.s > /dev/null 2>&1 && \
-		$gcc_cv_objdump -h conftest.o 2>/dev/null | \
-			sed -e /.eh_frame/!d -e N | \
-			grep READONLY > /dev/null; then
-		gcc_cv_as_cfi_directive=yes
-	      else
-		gcc_cv_as_cfi_directive=no
-	      fi
-	      ;;
-	    *)
-	      gcc_cv_as_cfi_directive=yes
-	      ;;
-	  esac 
-	fi
+    if test "x$gcc_cv_ld_ro_rw_mix" = xread-write; then
+      gcc_cv_as_cfi_directive=yes
+    elif test "x$gcc_cv_objdump" = x; then
+      # No objdump, err on the side of caution.
+      gcc_cv_as_cfi_directive=no
+    else
+      if test x$gas = xyes; then
+	as_32_opt="--32"
+	as_64_opt="--64"
       else
-        # no objdump, err on the side of caution
-	gcc_cv_as_cfi_directive=no
+	as_32_opt="-m32"
+	as_64_opt="-m64"
       fi
-    else
-      gcc_cv_as_cfi_directive=yes
+      case "$target" in
+	sparc*-*-solaris2.*)
+	  # On Solaris/SPARC, .eh_frame sections should always be read-write.
+	  if gcc_fn_eh_frame_ro $as_32_opt \
+	     || gcc_fn_eh_frame_ro $as_64_opt; then
+	    gcc_cv_as_cfi_directive=no
+	  else
+	    gcc_cv_as_cfi_directive=yes
+	  fi
+	  ;;
+	i?86-*-solaris2.* | x86_64-*-solaris2.*)
+	  # On Solaris/x86, make sure that GCC and assembler agree on using
+	  # read-only .eh_frame sections for 64-bit.
+	  if gcc_fn_eh_frame_ro $as_32_opt; then
+	    gcc_cv_as_cfi_directive=no
+	  elif gcc_fn_eh_frame_ro $as_64_opt; then
+	    gcc_cv_as_cfi_directive=yes
+	  else
+	    gcc_cv_as_cfi_directive=no
+	  fi
+	  ;;
+      esac
     fi
     ;;
   *-*-*)