diff mbox series

[v2] configury : Fix LEB128 support for non-GNU assemblers.

Message ID B6F978AE-F404-41EC-93C4-7976D9FD9174@googlemail.com
State New
Headers show
Series [v2] configury : Fix LEB128 support for non-GNU assemblers. | expand

Commit Message

Iain Sandoe Nov. 27, 2020, 10:29 a.m. UTC
Hi Folks.

Iain Sandoe via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:

> Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote:
>>> maybe this is enough to cover all bases without having to do any  
>>> version or
>>> target checks. (untested)
>>>
>>> objdump is not available on quite a few Darwin versions with perfectly
>>> functional
>>> uleb128 - so I don’t want to punt on those for absence of it.  We already
>>> check for
>>> otool elsewhere.
>>>
>>> thoughts?
>>
>> LGTM, with one nit:
>>
>>> [[
>>> if test "x$gcc_cv_objdump" != x; then
>>> if $gcc_cv_objdump --full-contents conftest.o 2>/dev/null \
>>
>> I'd use $gcc_cv_objdump -s here (maybe adding -j .data), matching the
>> other equivalent objdump invocations in gcc/configure.
>
> OK - I need to check on compatibility between GNU objdump and LLVM objdump.
> (since newer Darwin and GCN will be using the latter).
>
> -s might work OK since we only have one section, but -j is problematic wit
> different section naming conventions.

It turns out that ‘-s' is more compatible across the range of LLVM objdump
than —full-contents (which is not accepted by earlier versions).  -j is a  
non-
starter without we make the match more complex.  Perhaps there should
be a section early in configure that checks all the object dumper stuff and
sets some flags that are easier to test (instead of repeating the name  
checks)
—— not for today tho.

I’ve tested the following cases :
1/ as does not accept the leb128 directives
2/ as crashes with len128 directives
3/ as gives wrong output with leb128 directives
4/ as gives right output
   - with GNU objdump
   - with LLVM objdump from Darwin14..Darwin20
   - with Darwin cctools otool on Darwin10.

@Andrew - I don’t expect this to make any difference on GCN - but that does
depend on the target toolset having an objdump in the path.

If there are no futher comments, and assuming that a regstrap is successful  
on
AIX, Solaris (sparc), Linux and Darwin - OK for master?

thanks
Iain

--------

[PATCH] configury : Fix LEB128 support for non-GNU assemblers.

The current configuration test for LEB128 support in the assembler is
(a) specific to GNU assemblers and (b) only checks that the directives
are accepted, not that they give correct output.

The patch extends the asm test to cover one failure case present in
assemblers based off an older version of GAS (where a 64 bit value with
the MSB set presented to a .uleb128 directive causes a fail).

The test is now generalized such that it does not make use of any
specific test for assembler source/version, but checks that the output
is as expected.  We cater for scanning the object with objdump (either
binutils or LLVM) or Darwin otool.

gcc/ChangeLog:

	* configure.ac (check leb128 support): Check that assemblers both
	accept the LEB128 directives and also give the expected output.
	Add a test for uleb128 with the MSB set for a 64 bit value.
	* configure: Regenerated.
---
  gcc/configure    | 36 ++++++++++++++++++++----------------
  gcc/configure.ac | 48 ++++++++++++++++++++++++++----------------------
  2 files changed, 46 insertions(+), 38 deletions(-)

Comments

Jakub Jelinek Nov. 27, 2020, 10:37 a.m. UTC | #1
On Fri, Nov 27, 2020 at 10:29:53AM +0000, Iain Sandoe wrote:
> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -3114,34 +3114,38 @@ AC_MSG_RESULT($gcc_cv_ld_ro_rw_mix)
>  gcc_AC_INITFINI_ARRAY
> 
>  # Check if we have .[us]leb128, and support symbol arithmetic with it.
> +# Older versions of GAS and some non-GNU assemblers, have a bugs handling

s/a bugs/bugs/

Otherwise LGTM, thanks.

	Jakub
diff mbox series

Patch

diff --git a/gcc/configure b/gcc/configure
index e9510d39937..c6fd3819727 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -24000,6 +24000,8 @@  _ACEOF
 
 
  # Check if we have .[us]leb128, and support symbol arithmetic with it.
+# Older versions of GAS and some non-GNU assemblers, have a bugs handling
+# these directives, even when they appear to accept them.
  { $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for .sleb128 and .uleb128" >&5
  $as_echo_n "checking assembler for .sleb128 and .uleb128... " >&6; }
  if ${gcc_cv_as_leb128+:} false; then :
@@ -24017,7 +24019,9 @@  fi
  L1:
  	.uleb128 1280
  	.sleb128 -1010
-L2:' > conftest.s
+L2:
+	.uleb128 0x8000000000000000
+' > conftest.s
      if { ac_try='$gcc_cv_as $gcc_cv_as_flags  -o conftest.o conftest.s >&5'
    { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
    (eval $ac_try) 2>&5
@@ -24025,22 +24029,22 @@  L2:' > conftest.s
    $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
    test $ac_status = 0; }; }
      then
-	# GAS versions before 2.11 do not support uleb128,
-  # despite appearing to.
-  # ??? There exists an elf-specific test that will crash
-  # the assembler.  Perhaps it's better to figure out whether
-  # arbitrary sections are supported and try the test.
-  as_ver=`$gcc_cv_as --version 2>/dev/null | sed 1q`
-  if echo "$as_ver" | grep GNU > /dev/null; then
-    as_vers=`echo $as_ver | sed -n \
-	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*.*\)$,\1,p'`
-    as_major=`expr "$as_vers" : '\([0-9]*\)'`
-    as_minor=`expr "$as_vers" : '[0-9]*\.\([0-9]*\)'`
-    if test $as_major -eq 2 && test $as_minor -lt 11
-    then :
-    else gcc_cv_as_leb128=yes
-    fi
+
+if test "x$gcc_cv_objdump" != x; then
+  if $gcc_cv_objdump -s conftest.o 2>/dev/null \
+     | grep '04800a8e 78808080 80808080 808001' >/dev/null; then
+    gcc_cv_as_leb128=yes
+  fi
+elif test "x$gcc_cv_otool" != x; then
+  if $gcc_cv_otool -d conftest.o 2>/dev/null \
+     | grep '04 80 0a 8e 78 80 80 80 80 80 80 80 80 80 01' >/dev/null; then
+    gcc_cv_as_leb128=yes
    fi
+else
+  # play safe, assume the assembler is broken.
+  :
+fi
+
      else
        echo "configure: failed program was" >&5
        cat conftest.s >&5
diff --git a/gcc/configure.ac b/gcc/configure.ac
index cc27d099f00..bc39665ce12 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -3114,34 +3114,38 @@  AC_MSG_RESULT($gcc_cv_ld_ro_rw_mix)
  gcc_AC_INITFINI_ARRAY
 
  # Check if we have .[us]leb128, and support symbol arithmetic with it.
+# Older versions of GAS and some non-GNU assemblers, have a bugs handling
+# these directives, even when they appear to accept them.
  gcc_GAS_CHECK_FEATURE([.sleb128 and .uleb128], gcc_cv_as_leb128,
-  [elf,2,11,0],,
+ [elf,2,11,0],,
  [	.data
  	.uleb128 L2 - L1
  L1:
  	.uleb128 1280
  	.sleb128 -1010
-L2:],
-[[# GAS versions before 2.11 do not support uleb128,
-  # despite appearing to.
-  # ??? There exists an elf-specific test that will crash
-  # the assembler.  Perhaps it's better to figure out whether
-  # arbitrary sections are supported and try the test.
-  as_ver=`$gcc_cv_as --version 2>/dev/null | sed 1q`
-  if echo "$as_ver" | grep GNU > /dev/null; then
-    as_vers=`echo $as_ver | sed -n \
-	-e 's,^.*[	 ]\([0-9][0-9]*\.[0-9][0-9]*.*\)$,\1,p'`
-    as_major=`expr "$as_vers" : '\([0-9]*\)'`
-    as_minor=`expr "$as_vers" : '[0-9]*\.\([0-9]*\)'`
-    if test $as_major -eq 2 && test $as_minor -lt 11
-    then :
-    else gcc_cv_as_leb128=yes
-    fi
-  fi]],
-  [AC_DEFINE(HAVE_AS_LEB128, 1,
-    [Define if your assembler supports .sleb128 and .uleb128.])],
-  [AC_DEFINE(HAVE_AS_LEB128, 0,
-    [Define if your assembler supports .sleb128 and .uleb128.])])
+L2:
+	.uleb128 0x8000000000000000
+],
+[[
+if test "x$gcc_cv_objdump" != x; then
+  if $gcc_cv_objdump -s conftest.o 2>/dev/null \
+     | grep '04800a8e 78808080 80808080 808001' >/dev/null; then
+    gcc_cv_as_leb128=yes
+  fi
+elif test "x$gcc_cv_otool" != x; then
+  if $gcc_cv_otool -d conftest.o 2>/dev/null \
+     | grep '04 80 0a 8e 78 80 80 80 80 80 80 80 80 80 01' >/dev/null; then
+    gcc_cv_as_leb128=yes
+  fi
+else
+  # play safe, assume the assembler is broken.
+  :
+fi
+]],
+ [AC_DEFINE(HAVE_AS_LEB128, 1,
+   [Define if your assembler supports .sleb128 and .uleb128.])],
+ [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 () {