public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] configury : Fix LEB128 support for non-GNU assemblers.
@ 2020-11-25  9:49 Iain Sandoe
  2020-11-25 10:37 ` Rainer Orth
  2020-11-25 17:49 ` Jeff Law
  0 siblings, 2 replies; 15+ messages in thread
From: Iain Sandoe @ 2020-11-25  9:49 UTC (permalink / raw)
  To: GCC Patches; +Cc: David Edelsohn, Andrew Stubbs, Rainer Orth

Hi,

I’ve had this patch in my Darwin trees for (literally) years, it is  
relevant to
the GCC ports that use a non-binutils assembler.

At present, even if such an assembler supports LEB128,  the GCC config
is setting HAVE_LEB128 = 0.

The ports I know of that can benefit from a change here are:

AIX (unaffected by this change, since the assembler [at least on gcc119] does
         not support leb128 directives)

Darwin (checked the various different assemblers)

GCN (I can’t test this, so Andrew, please could you say if the change
           is OK for that)

Solaris (bootstrapped and tests running on GCC211, but maybe Rainer would
             want wider checks).

I guess we could exclude specific ports that don’t want to use leb128 with
a target elif in the configuration.

OK for master?

thanks
Iain

===== commit message

Some assemblers that either do not respond to --version, or are non-GNU,
do support leb128 directives.  The current configuration test omits these
cases and only supports GNU assemblers with a version > 2.11.

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).

This change then assumes that a non-GNU assembler that passes the asm test
correctly supports the directives.

gcc/ChangeLog:

	* configure.ac (check leb128 support): Support non-GNU assemblers
	that pass the leb128 confgure test.  Add a test for uleb128 with
	the MSB set for a 64 bit value.
	* configure: Regenerated.
---
  gcc/configure    | 6 ++++++
  gcc/configure.ac | 6 ++++++
  2 files changed, 12 insertions(+)

diff --git a/gcc/configure.ac b/gcc/configure.ac
index b410428b4fc..8a1aa95f01b 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -3072,6 +3072,8 @@ 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.
+# Some assemblers based on older GAS have a bug when the MSB is set for
+# a 64b value and used in a uleb128.
  gcc_GAS_CHECK_FEATURE([.sleb128 and .uleb128], gcc_cv_as_leb128,
    [elf,2,11,0],,
  [	.data
@@ -3079,6 +3081,7 @@ gcc_GAS_CHECK_FEATURE([.sleb128 and .uleb128],  
gcc_cv_as_leb128,
  L1:
  	.uleb128 1280
  	.sleb128 -1010
+	.uleb128 0x8000000000000000
  L2:],
  [[# GAS versions before 2.11 do not support uleb128,
    # despite appearing to.
@@ -3095,6 +3098,9 @@ L2:],
      then :
      else gcc_cv_as_leb128=yes
      fi
+  else
+    # This is a non-GNU assembler, which passes the feature check.
+    gcc_cv_as_leb128=yes
    fi]],
    [AC_DEFINE(HAVE_AS_LEB128, 1,
      [Define if your assembler supports .sleb128 and .uleb128.])],
-- 
2.24.1


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

* Re: [PATCH] configury : Fix LEB128 support for non-GNU assemblers.
  2020-11-25  9:49 [PATCH] configury : Fix LEB128 support for non-GNU assemblers Iain Sandoe
@ 2020-11-25 10:37 ` Rainer Orth
  2020-11-26 13:08   ` Rainer Orth
  2020-11-25 17:49 ` Jeff Law
  1 sibling, 1 reply; 15+ messages in thread
From: Rainer Orth @ 2020-11-25 10:37 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: GCC Patches, David Edelsohn, Andrew Stubbs

Hi Iain,

> I’ve had this patch in my Darwin trees for (literally) years, it is
> relevant to
> the GCC ports that use a non-binutils assembler.
>
> At present, even if such an assembler supports LEB128,  the GCC config
> is setting HAVE_LEB128 = 0.
>
> The ports I know of that can benefit from a change here are:
[...]
> Solaris (bootstrapped and tests running on GCC211, but maybe Rainer would
>             want wider checks).

I've just manually tried the augmented test on Solaris 10-11.4, SPARC
and x86.  While the Solaris/SPARC assembler handles it just fine, the
x86 one chokes in a known way:

Assembler: 
	"/homes/ro/leb128.s", line 2 : Syntax error
	Near line: "	.uleb128 L2 - L1"

> I guess we could exclude specific ports that don’t want to use leb128 with
> a target elif in the configuration.

I'll include the patch in tonight's Solaris bootstraps for good measure,
but it seems fine to me as is.

Thanks.
        Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

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

* Re: [PATCH] configury : Fix LEB128 support for non-GNU assemblers.
  2020-11-25  9:49 [PATCH] configury : Fix LEB128 support for non-GNU assemblers Iain Sandoe
  2020-11-25 10:37 ` Rainer Orth
@ 2020-11-25 17:49 ` Jeff Law
  1 sibling, 0 replies; 15+ messages in thread
From: Jeff Law @ 2020-11-25 17:49 UTC (permalink / raw)
  To: Iain Sandoe, GCC Patches; +Cc: Andrew Stubbs, David Edelsohn



On 11/25/20 2:49 AM, Iain Sandoe wrote:
> Hi,
>
> I’ve had this patch in my Darwin trees for (literally) years, it is
> relevant to
> the GCC ports that use a non-binutils assembler.
>
> At present, even if such an assembler supports LEB128,  the GCC config
> is setting HAVE_LEB128 = 0.
>
> The ports I know of that can benefit from a change here are:
>
> AIX (unaffected by this change, since the assembler [at least on
> gcc119] does
>         not support leb128 directives)
>
> Darwin (checked the various different assemblers)
>
> GCN (I can’t test this, so Andrew, please could you say if the change
>           is OK for that)
>
> Solaris (bootstrapped and tests running on GCC211, but maybe Rainer would
>             want wider checks).
>
> I guess we could exclude specific ports that don’t want to use leb128
> with
> a target elif in the configuration.
>
> OK for master?
>
> thanks
> Iain
>
> ===== commit message
>
> Some assemblers that either do not respond to --version, or are non-GNU,
> do support leb128 directives.  The current configuration test omits these
> cases and only supports GNU assemblers with a version > 2.11.
>
> 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).
>
> This change then assumes that a non-GNU assembler that passes the asm
> test
> correctly supports the directives.
>
> gcc/ChangeLog:
>
>     * configure.ac (check leb128 support): Support non-GNU assemblers
>     that pass the leb128 confgure test.  Add a test for uleb128 with
>     the MSB set for a 64 bit value.
>     * configure: Regenerated.
OK.  Obviously if Rainer's tests show adjustment is necessary we'll have
to take care of that. 

Jeff


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

* Re: [PATCH] configury : Fix LEB128 support for non-GNU assemblers.
  2020-11-25 10:37 ` Rainer Orth
@ 2020-11-26 13:08   ` Rainer Orth
  2020-11-26 13:17     ` Iain Sandoe
  0 siblings, 1 reply; 15+ messages in thread
From: Rainer Orth @ 2020-11-26 13:08 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: GCC Patches

Hi Iain,

>> The ports I know of that can benefit from a change here are:
> [...]
>> Solaris (bootstrapped and tests running on GCC211, but maybe Rainer would
>>             want wider checks).
>
> I've just manually tried the augmented test on Solaris 10-11.4, SPARC
> and x86.  While the Solaris/SPARC assembler handles it just fine, the
> x86 one chokes in a known way:
>
> Assembler: 
> 	"/homes/ro/leb128.s", line 2 : Syntax error
> 	Near line: "	.uleb128 L2 - L1"
>
>> I guess we could exclude specific ports that don’t want to use leb128 with
>> a target elif in the configuration.
>
> I'll include the patch in tonight's Solaris bootstraps for good measure,
> but it seems fine to me as is.

unfortunately, Solaris/SPARC results are miserable:

* About 1600 Go tests FAIL, spread across go.*, libgo, and gotools, all
  in the same way, it seems:

+FAIL: go.go-torture/execute/array-1.go execution,  -O0 

fatal error: DWARF underflow in .debug_line at 3266879

goroutine 1 [running, locked to thread]:
fatal error: DWARF underflow in .debug_line at 3266879
panic during panic

goroutine 1 [running, locked to thread]:
fatal error: DWARF underflow in .debug_line at 3266879
stack trace unavailable

* On top of that, I get about 80 libstdc++ failures, again all of the
  same kind:

+FAIL: libstdc++-prettyprinters/80276.cc whatis p4

type = std::unique_ptr<std::vector<std::unique_ptr<std::list<std::basic_string<char, std::char_traits<char>, std::allocator<char> >>[]>>[99]>
got: type = std::unique_ptr<std::vector<std::unique_ptr<std::list<std::basic_string<char, std::char_traits<char>, std::allocator<char> >>[]>>[99]>

  which is confusing because the the found and expected types are
  identical.

All this happens for both 32 and 64-bit tests.

To ascertain that this is really the leb128 patch, I've reverted it in
my tree and reran the bootstrap: all those failures are gone.

So without further investigation, we cannot use the leb128 directives
with Solaris/SPARC as.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

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

* Re: [PATCH] configury : Fix LEB128 support for non-GNU assemblers.
  2020-11-26 13:08   ` Rainer Orth
@ 2020-11-26 13:17     ` Iain Sandoe
  2020-11-26 13:26       ` Rainer Orth
  0 siblings, 1 reply; 15+ messages in thread
From: Iain Sandoe @ 2020-11-26 13:17 UTC (permalink / raw)
  To: Rainer Orth; +Cc: GCC Patches

Hi Rainer,

Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote:

>>> The ports I know of that can benefit from a change here are:
>> [...]
>>> Solaris (bootstrapped and tests running on GCC211, but maybe Rainer would
>>>            want wider checks).
>>
>> I've just manually tried the augmented test on Solaris 10-11.4, SPARC
>> and x86.  While the Solaris/SPARC assembler handles it just fine, the
>> x86 one chokes in a known way:
>>
>> Assembler:
>> 	"/homes/ro/leb128.s", line 2 : Syntax error
>> 	Near line: "	.uleb128 L2 - L1"
>>
>>> I guess we could exclude specific ports that don’t want to use leb128  
>>> with
>>> a target elif in the configuration.
>>
>> I'll include the patch in tonight's Solaris bootstraps for good measure,
>> but it seems fine to me as is.
>
> unfortunately, Solaris/SPARC results are miserable:
>
> * About 1600 Go tests FAIL, spread across go.*, libgo, and gotools, all
>  in the same way, it seems:
>
> +FAIL: go.go-torture/execute/array-1.go execution,  -O0
>
> fatal error: DWARF underflow in .debug_line at 3266879
>
> goroutine 1 [running, locked to thread]:
> fatal error: DWARF underflow in .debug_line at 3266879
> panic during panic
>
> goroutine 1 [running, locked to thread]:
> fatal error: DWARF underflow in .debug_line at 3266879
> stack trace unavailable

go is stil not implemented for Darwin, so I have no comparison there.

> * On top of that, I get about 80 libstdc++ failures, again all of the
>  same kind:
>
> +FAIL: libstdc++-prettyprinters/80276.cc whatis p4
>
> type =  
> std::unique_ptr<std::vector<std::unique_ptr<std::list<std::basic_string<char,  
> std::char_traits<char>, std::allocator<char> >>[]>>[99]>
> got: type =  
> std::unique_ptr<std::vector<std::unique_ptr<std::list<std::basic_string<char,  
> std::char_traits<char>, std::allocator<char> >>[]>>[99]>
>
>  which is confusing because the the found and expected types are
>  identical.

so some breakage in RTTI .. I suppose.

> All this happens for both 32 and 64-bit tests.
>
> To ascertain that this is really the leb128 patch, I've reverted it in
> my tree and reran the bootstrap: all those failures are gone.
>
> So without further investigation, we cannot use the leb128 directives
> with Solaris/SPARC as.

I think Andrew was running GCN (not sure of the results there)

- but, I suppose that the simplest modification is to do

elif … target is darwin

and make it so that other (non-GNU-as) platforms have to opt in.

I’ll make a version that does this and test it locally.

thanks for checking,
Iain


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

* Re: [PATCH] configury : Fix LEB128 support for non-GNU assemblers.
  2020-11-26 13:17     ` Iain Sandoe
@ 2020-11-26 13:26       ` Rainer Orth
  2020-11-26 13:30         ` Jakub Jelinek
  2020-11-26 14:48         ` [PATCH] " Iain Sandoe
  0 siblings, 2 replies; 15+ messages in thread
From: Rainer Orth @ 2020-11-26 13:26 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: GCC Patches

Hi Iain,

>> unfortunately, Solaris/SPARC results are miserable:
>>
>> * About 1600 Go tests FAIL, spread across go.*, libgo, and gotools, all
>>  in the same way, it seems:
>>
>> +FAIL: go.go-torture/execute/array-1.go execution,  -O0
>>
>> fatal error: DWARF underflow in .debug_line at 3266879
>>
>> goroutine 1 [running, locked to thread]:
>> fatal error: DWARF underflow in .debug_line at 3266879
>> panic during panic
>>
>> goroutine 1 [running, locked to thread]:
>> fatal error: DWARF underflow in .debug_line at 3266879
>> stack trace unavailable
>
> go is stil not implemented for Darwin, so I have no comparison there.

I suspect this is not about Go in particular, but about a buggy
assembler ;-)  The Solaris as track record isn't really the best,
unfortunately.

I just had a quick look at the stacktrace at the failure point

Thread 2 hit Breakpoint 1, 0xfeb60cc8 in runtime.throw (s=...) at  /vol/gcc/src/hg/master/local/libgo/go/runtime/internal/sys/intrinsics_common.go:33837
33837	 /vol/gcc/src/hg/master/local/libgo/go/runtime/internal/sys/intrinsics_common.go: No such file or directory.
(gdb) where
#0  0xfeb60cc8 in runtime.throw (s=...) at  /vol/gcc/src/hg/master/local/libgo/go/runtime/internal/sys/intrinsics_common.go:33837
#1  0xfe6a1c0c in runtime_throw (s=0x64d100 "DWARF underflow in .debug_line at 3266879") at /vol/gcc/src/hg/master/local/libgo/runtime/panic.c:13
#2  0xfe69ea18 in error_callback (data=0x64d9e0, errnum=0, msg=0x64d100 "DWARF underflow in .debug_line at 3266879") at /vol/gcc/src/hg/master/local/libgo/runtime/go-callers.c:68
#3  error_callback (data=0x64d9e0, msg=0x64d100 "DWARF underflow in .debug_line at 3266879", errnum=0) at /vol/gcc/src/hg/master/local/libgo/runtime/go-callers.c:211
#4  0xfecd22d0 in dwarf_buf_error (msg=<optimized out>, buf=<optimized out>) at /vol/gcc/src/hg/master/local/libbacktrace/dwarf.c:16103
#5  require (count=<optimized out>, buf=<optimized out>) at /vol/gcc/src/hg/master/local/libbacktrace/dwarf.c:433
#6  advance (count=<optimized out>, buf=<optimized out>) at /vol/gcc/src/hg/master/local/libbacktrace/dwarf.c:446
#7  read_line_program (vec=<optimized out>, line_buf=<optimized out>, hdr=<optimized out>, u=<optimized out>, ddata=<optimized out>, state=<optimized out>) at /vol/gcc/src/hg/master/local/libbacktrace/dwarf.c:2819
#8  read_line_info (lines_count=<synthetic pointer>, lines=<synthetic pointer>, hdr=<error reading variable: DWARF expression error: ran off end of buffer reading sleb128 value>, u=<optimized out>, data=<optimized out>, error_callback=<optimized out>, ddata=0xed188510, state=0xff188000) at /vol/gcc/src/hg/master/local/libbacktrace/dwarf.c:2951
#9  dwarf_lookup_pc (state=0xff188000, ddata=0xed188510, pc=<optimized out>, callback=<optimized out>, error_callback=<optimized out>, data=<optimized out>, found=<optimized out>) at /vol/gcc/src/hg/master/local/libbacktrace/dwarf.c:3732
#10 0xfecd3300 in dwarf_fileline (state=0xff188000, pc=4273447611, callback=0xfe69ec04 <callback>, error_callback=0xfe69e9e4 <error_callback>, data=0x64d9e0) at /vol/gcc/src/hg/master/local/libbacktrace/dwarf.c:20031
#11 0xfecd4160 in backtrace_pcinfo (state=0xff188000, pc=4273447611, callback=0xfe69ec04 <callback>, error_callback=0xfe69e9e4 <error_callback>, data=0x64d9e0) at /vol/gcc/src/hg/master/local/libbacktrace/fileline.c:973
#12 0xfecd4714 in unwind (context=<optimized out>, vdata=0x64d964) at /vol/gcc/src/hg/master/local/libbacktrace/backtrace.c:91
#13 0xff13c8d4 in _Unwind_Backtrace (trace=0xfecd4688 <unwind>, trace_argument=0x64d964) at /builds2/ulhg/nightly_85/components/gcc10/gcc-10.2.0/libgcc/unwind.inc:307
#14 0xfecd479c in backtrace_full (state=0xff188000, skip=0, callback=0xfe69ec04 <callback>, error_callback=0xfe69e9e4 <error_callback>, data=0x64d9e0) at /vol/gcc/src/hg/master/local/libbacktrace/backtrace.c:127
[...]

  which shows that the problem is detected in the depths of
  libbacktrace's DWARF reader.  There's something completely off in
  places, like line numbers well beyond the end of dwarf.c.

  TBH, I don't really feel like diving into the innards of libbacktrace
  and DWARF at this point to investigate.

>> All this happens for both 32 and 64-bit tests.
>>
>> To ascertain that this is really the leb128 patch, I've reverted it in
>> my tree and reran the bootstrap: all those failures are gone.
>>
>> So without further investigation, we cannot use the leb128 directives
>> with Solaris/SPARC as.
>
> I think Andrew was running GCN (not sure of the results there)
>
> - but, I suppose that the simplest modification is to do
>
> elif … target is darwin
>
> and make it so that other (non-GNU-as) platforms have to opt in.

Agreed: that's certainly the safest option given that we're in stage3.
While it would be nice to be able to use the leb128 directives, I
wouldn't consider this crucial.

> I’ll make a version that does this and test it locally.

Great, thanks.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

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

* Re: [PATCH] configury : Fix LEB128 support for non-GNU assemblers.
  2020-11-26 13:26       ` Rainer Orth
@ 2020-11-26 13:30         ` Jakub Jelinek
  2020-11-26 15:07           ` Rainer Orth
  2020-11-26 14:48         ` [PATCH] " Iain Sandoe
  1 sibling, 1 reply; 15+ messages in thread
From: Jakub Jelinek @ 2020-11-26 13:30 UTC (permalink / raw)
  To: Rainer Orth; +Cc: Iain Sandoe, GCC Patches

On Thu, Nov 26, 2020 at 02:26:58PM +0100, Rainer Orth wrote:
>   which shows that the problem is detected in the depths of
>   libbacktrace's DWARF reader.  There's something completely off in
>   places, like line numbers well beyond the end of dwarf.c.
> 
>   TBH, I don't really feel like diving into the innards of libbacktrace
>   and DWARF at this point to investigate.

Perhaps try binutils readelf -wi or dwarflint etc. to find out where things
go wrong, and then just look at the assembly and whether it matches what as
made out of it?

	Jakub


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

* Re: [PATCH] configury : Fix LEB128 support for non-GNU assemblers.
  2020-11-26 13:26       ` Rainer Orth
  2020-11-26 13:30         ` Jakub Jelinek
@ 2020-11-26 14:48         ` Iain Sandoe
  2020-11-26 14:52           ` Andrew Stubbs
  1 sibling, 1 reply; 15+ messages in thread
From: Iain Sandoe @ 2020-11-26 14:48 UTC (permalink / raw)
  To: GCC Patches; +Cc: Rainer Orth, Andrew Stubbs

Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote:

>>> unfortunately, Solaris/SPARC results are miserable:
>>>

>>> So without further investigation, we cannot use the leb128 directives
>>> with Solaris/SPARC as.
>>
>> I think Andrew was running GCN (not sure of the results there)
>>
>> - but, I suppose that the simplest modification is to do
>>
>> elif … target is darwin
>>
>> and make it so that other (non-GNU-as) platforms have to opt in.
>
> Agreed: that's certainly the safest option given that we're in stage3.
> While it would be nice to be able to use the leb128 directives, I
> wouldn't consider this crucial.
>
>> I’ll make a version that does this and test it locally.
>
> Great, thanks.

testing this (which ought to be easy for GCN to opt into if wanted):

diff --git a/gcc/configure.ac b/gcc/configure.ac
index cc27d099f00..d9aa36d7f24 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -3114,6 +3114,8 @@ 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.
+# Some assemblers based on older GAS have a bug when the MSB is set for
+# a 64b value and used in a uleb128.
  gcc_GAS_CHECK_FEATURE([.sleb128 and .uleb128], gcc_cv_as_leb128,
    [elf,2,11,0],,
  [      .data
@@ -3121,6 +3123,7 @@ gcc_GAS_CHECK_FEATURE([.sleb128 and .uleb128],  
gcc_cv_as_leb128,
  L1:
         .uleb128 1280
         .sleb128 -1010
+       .uleb128 0x8000000000000000
  L2:],
  [[# GAS versions before 2.11 do not support uleb128,
    # despite appearing to.
@@ -3137,6 +3140,13 @@ L2:],
      then :
      else gcc_cv_as_leb128=yes
      fi
+  else
+    # Allow targets to opt in if the leb128 test above is adequate to
+    # indicate it may be used.
+    case "$target" in
+      *-*-darwin*) gcc_cv_as_leb128=yes ;;
+      *) ;;
+    esac
    fi]],
    [AC_DEFINE(HAVE_AS_LEB128, 1,
      [Define if your assembler supports .sleb128 and .uleb128.])],



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

* Re: [PATCH] configury : Fix LEB128 support for non-GNU assemblers.
  2020-11-26 14:48         ` [PATCH] " Iain Sandoe
@ 2020-11-26 14:52           ` Andrew Stubbs
  0 siblings, 0 replies; 15+ messages in thread
From: Andrew Stubbs @ 2020-11-26 14:52 UTC (permalink / raw)
  To: Iain Sandoe, GCC Patches

On 26/11/2020 14:48, Iain Sandoe wrote:
> Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote:
> 
>>>> unfortunately, Solaris/SPARC results are miserable:
>>>>
> 
>>>> So without further investigation, we cannot use the leb128 directives
>>>> with Solaris/SPARC as.
>>>
>>> I think Andrew was running GCN (not sure of the results there)
>>>
>>> - but, I suppose that the simplest modification is to do
>>>
>>> elif … target is darwin
>>>
>>> and make it so that other (non-GNU-as) platforms have to opt in.
>>
>> Agreed: that's certainly the safest option given that we're in stage3.
>> While it would be nice to be able to use the leb128 directives, I
>> wouldn't consider this crucial.
>>
>>> I’ll make a version that does this and test it locally.
>>
>> Great, thanks.
> 
> testing this (which ought to be easy for GCN to opt into if wanted):

I've tested the original patch, and there were no new failures for GCN.

Andrew

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

* Re: [PATCH] configury : Fix LEB128 support for non-GNU assemblers.
  2020-11-26 13:30         ` Jakub Jelinek
@ 2020-11-26 15:07           ` Rainer Orth
  2020-11-26 20:28             ` Iain Sandoe
  0 siblings, 1 reply; 15+ messages in thread
From: Rainer Orth @ 2020-11-26 15:07 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Iain Sandoe, GCC Patches

Hi Jakub,

> On Thu, Nov 26, 2020 at 02:26:58PM +0100, Rainer Orth wrote:
>>   which shows that the problem is detected in the depths of
>>   libbacktrace's DWARF reader.  There's something completely off in
>>   places, like line numbers well beyond the end of dwarf.c.
>> 
>>   TBH, I don't really feel like diving into the innards of libbacktrace
>>   and DWARF at this point to investigate.
>
> Perhaps try binutils readelf -wi or dwarflint etc. to find out where things
> go wrong, and then just look at the assembly and whether it matches what as
> made out of it?

that was certainly easier than getting into libbacktrace.  I've looked
for the smallest object linked into libgo that showed readelf -wi
differences.  errors.o had

--- build.no-leb128/sparc-sun-solaris2.11/libgo/errors.o.readelf-wi     2020-11-26 14:55:17.792533040 +0000
+++ build.leb128/sparc-sun-solaris2.11/libgo/errors.o.readelf-wi        2020-11-26 14:54:59.099964700 +0000
@@ -957,7 +957,7 @@
     <7b1>   DW_AT_decl_line   : 89
     <7b2>   DW_AT_decl_column : 2
     <7b3>   DW_AT_type        : <0x314>
-    <7b7>   DW_AT_location    : 3 byte block: 91 94 7f         (DW_OP_fbreg: -108)
+    <7b7>   DW_AT_location    : 3 byte block: 91 54 7f         (DW_OP_fbreg: -44; DW_OP_breg15 (r15): 0)
  <3><7bb>: Abbrev Number: 35 (DW_TAG_lexical_block)
     <7bc>   DW_AT_ranges      : 0x98
     <7c0>   DW_AT_sibling     : <0x806>

The corresponding assembler is

        .byte   0x3     ! uleb128 0x3; DW_AT_location
        .byte   0x91    ! DW_OP_fbreg
        .byte   0x94,0x7f       ! sleb128 -108

without leb128 directives and

        .uleb128 0x3    ! DW_AT_location
        .byte   0x91    ! DW_OP_fbreg
        .sleb128 -108

with them.  Solaris/SPARC as emits 54 7f for the .sleb128, not 94 7f.

No wonder things go astray from there...

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

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

* Re: [PATCH] configury : Fix LEB128 support for non-GNU assemblers.
  2020-11-26 15:07           ` Rainer Orth
@ 2020-11-26 20:28             ` Iain Sandoe
  2020-11-26 21:39               ` Rainer Orth
  0 siblings, 1 reply; 15+ messages in thread
From: Iain Sandoe @ 2020-11-26 20:28 UTC (permalink / raw)
  To: GCC Patches; +Cc: Jakub Jelinek, Rainer Orth, Andrew Stubbs

Hi folks,

Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote:

>> On Thu, Nov 26, 2020 at 02:26:58PM +0100, Rainer Orth wrote:
>>>  which shows that the problem is detected in the depths of
>>>  libbacktrace's DWARF reader.  There's something completely off in
>>>  places, like line numbers well beyond the end of dwarf.c.
>>>
>>>  TBH, I don't really feel like diving into the innards of libbacktrace
>>>  and DWARF at this point to investigate.
>>
>> Perhaps try binutils readelf -wi or dwarflint etc. to find out where  
>> things
>> go wrong, and then just look at the assembly and whether it matches what  
>> as
>> made out of it?
>
> that was certainly easier than getting into libbacktrace.  I've looked
> for the smallest object linked into libgo that showed readelf -wi
> differences.  errors.o had
>

> No wonder things go astray from there…

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?

Iain

-----

# Check if we have .[us]leb128, and support symbol arithmetic with it.
# Older versions of GAS and some tools based on those, have a bugs handling
# these directive, even when they appear to accept them.
gcc_GAS_CHECK_FEATURE([.sleb128 and .uleb128], gcc_cv_as_leb128,
   [elf,2,11,0],,
[	.data
	.uleb128 L2 - L1
L1:
	.uleb128 1280
	.sleb128 -1010
L2:
	.uleb128 0x8000000000000000
],
[[
if test "x$gcc_cv_objdump" != x; then
  if $gcc_cv_objdump --full-contents 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.])])


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

* Re: [PATCH] configury : Fix LEB128 support for non-GNU assemblers.
  2020-11-26 20:28             ` Iain Sandoe
@ 2020-11-26 21:39               ` Rainer Orth
  2020-11-26 21:43                 ` Iain Sandoe
  0 siblings, 1 reply; 15+ messages in thread
From: Rainer Orth @ 2020-11-26 21:39 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: GCC Patches, Jakub Jelinek, Andrew Stubbs

Hi Iain,

> 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.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

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

* Re: [PATCH] configury : Fix LEB128 support for non-GNU assemblers.
  2020-11-26 21:39               ` Rainer Orth
@ 2020-11-26 21:43                 ` Iain Sandoe
  2020-11-27 10:29                   ` [PATCH v2] " Iain Sandoe
  0 siblings, 1 reply; 15+ messages in thread
From: Iain Sandoe @ 2020-11-26 21:43 UTC (permalink / raw)
  To: Rainer Orth; +Cc: GCC Patches, Jakub Jelinek, Andrew Stubbs

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.

corollary: one should not assume that other invocations of objdump in  
configure
are working as expected if the objdump is the LLVM one.

cheers
Iain


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

* [PATCH v2] configury : Fix LEB128 support for non-GNU assemblers.
  2020-11-26 21:43                 ` Iain Sandoe
@ 2020-11-27 10:29                   ` Iain Sandoe
  2020-11-27 10:37                     ` Jakub Jelinek
  0 siblings, 1 reply; 15+ messages in thread
From: Iain Sandoe @ 2020-11-27 10:29 UTC (permalink / raw)
  To: GCC Patches; +Cc: Rainer Orth, Jakub Jelinek, Andrew Stubbs, Jeff Law

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(-)

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 () {
-- 
2.24.1


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

* Re: [PATCH v2] configury : Fix LEB128 support for non-GNU assemblers.
  2020-11-27 10:29                   ` [PATCH v2] " Iain Sandoe
@ 2020-11-27 10:37                     ` Jakub Jelinek
  0 siblings, 0 replies; 15+ messages in thread
From: Jakub Jelinek @ 2020-11-27 10:37 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: GCC Patches, Rainer Orth, Andrew Stubbs, Jeff Law

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


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

end of thread, other threads:[~2020-11-27 10:37 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-25  9:49 [PATCH] configury : Fix LEB128 support for non-GNU assemblers Iain Sandoe
2020-11-25 10:37 ` Rainer Orth
2020-11-26 13:08   ` Rainer Orth
2020-11-26 13:17     ` Iain Sandoe
2020-11-26 13:26       ` Rainer Orth
2020-11-26 13:30         ` Jakub Jelinek
2020-11-26 15:07           ` Rainer Orth
2020-11-26 20:28             ` Iain Sandoe
2020-11-26 21:39               ` Rainer Orth
2020-11-26 21:43                 ` Iain Sandoe
2020-11-27 10:29                   ` [PATCH v2] " Iain Sandoe
2020-11-27 10:37                     ` Jakub Jelinek
2020-11-26 14:48         ` [PATCH] " Iain Sandoe
2020-11-26 14:52           ` Andrew Stubbs
2020-11-25 17:49 ` Jeff Law

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).