public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Matthias Klose <doko@ubuntu.com>
To: Ian Lance Taylor <iant@golang.org>, Cherry Zhang <cherryyz@google.com>
Cc: gcc-patches <gcc-patches@gcc.gnu.org>, gofrontend-dev@googlegroups.com
Subject: Re: [gofrontend-dev] Re: libgo patch committed: Add precise stack scan support
Date: Wed, 19 Dec 2018 03:24:00 -0000	[thread overview]
Message-ID: <bc93b1ba-5ee4-71c6-24a2-0afe349c8e4a@ubuntu.com> (raw)
In-Reply-To: <CAOyqgcWvHNRnKEjFkivpp-OvdJ=bkCd537Ot2t3VozuNfW2+EA@mail.gmail.com>

Cherry, see
https://gcc.gnu.org/ml/gcc-testresults/2018-12/msg02241.html
https://gcc.gnu.org/ml/gcc-testresults/2018-12/msg02240.html

still shows ~180 test failures on ARM32.  Sorry, no access to an ARM32 box until
next year.

Matthias

On 13.12.18 00:27, Ian Lance Taylor wrote:
> On Wed, Dec 12, 2018 at 8:10 AM Cherry Zhang <cherryyz@google.com> wrote:
>>
>> Thank you, Matthias!
>>
>> From the log, essentially all the tests aborted. The only place the new code can cause abort on all programs that I can think of is in the runtime startup code, probestackmaps, which calls value_size, which aborts due to an unhandled case. I haven't been able to try out on an ARM machine, but I managed to cross-compile a Go program and visually inspect the exception table. The type table's encoding is DW_EH_PE_absptr, which is indeed not handled, which will cause abort.
>>
>> I send https://go-review.googlesource.com/c/gofrontend/+/153857 (also as below). Hopefully this will fix the problem.
>>
>> Thanks,
>> Cherry
>>
>> diff --git a/libgo/runtime/go-unwind.c b/libgo/runtime/go-unwind.c
>> index c44755f9..f4bbfb60 100644
>> --- a/libgo/runtime/go-unwind.c
>> +++ b/libgo/runtime/go-unwind.c
>> @@ -318,6 +318,8 @@ value_size (uint8_t encoding)
>>        case DW_EH_PE_sdata8:
>>        case DW_EH_PE_udata8:
>>          return 8;
>> +      case DW_EH_PE_absptr:
>> +        return sizeof(uintptr);
>>        default:
>>          break;
>>      }
> 
> 
> Thanks.
> 
> Committed to mainline.
> 
> Ian
> 
> 
> 
>> On Tue, Dec 11, 2018 at 7:03 PM Matthias Klose <doko@ubuntu.com> wrote:
>>>
>>> On 11.12.18 22:01, Cherry Zhang wrote:
>>>> On Tue, Dec 11, 2018 at 3:51 PM Ian Lance Taylor <iant@golang.org> wrote:
>>>>
>>>>> On Tue, Dec 11, 2018 at 6:52 AM Matthias Klose <doko@ubuntu.com> wrote:
>>>>>>
>>>>>> On 10.12.18 16:54, Cherry Zhang wrote:
>>>>>>> On Mon, Dec 10, 2018 at 1:41 AM Matthias Klose <doko@ubuntu.com>
>>>>> wrote:
>>>>>>>
>>>>>>>> On 06.12.18 00:09, Ian Lance Taylor wrote:
>>>>>>>>> This libgo patch by Cherry Zhang adds support for precise stack
>>>>>>>>> scanning to the Go runtime.  This uses per-function stack maps stored
>>>>>>>>> in the exception tables in the language-specific data area.  The
>>>>>>>>> compiler needs to generate these stack maps; currently this is only
>>>>>>>>> done by a version of LLVM, not by GCC.  Each safepoint in a function
>>>>>>>>> is associated with a (real or dummy) landing pad, and its "type info"
>>>>>>>>> in the exception table is a pointer to the stack map. When a stack is
>>>>>>>>> scanned, the stack map is found by the stack unwinding code.
>>>>>>>>>
>>>>>>>>> For precise stack scan we need to unwind the stack. There are three
>>>>>>>> cases:
>>>>>>>>>
>>>>>>>>> - If a goroutine is scanning its own stack, it can unwind the stack
>>>>>>>>> and scan the frames.
>>>>>>>>>
>>>>>>>>> - If a goroutine is scanning another, stopped, goroutine, it cannot
>>>>>>>>> directly unwind the target stack. We handle this by switching
>>>>>>>>> (runtime.gogo) to the target g, letting it unwind and scan the stack,
>>>>>>>>> and switch back.
>>>>>>>>>
>>>>>>>>> - If we are scanning a goroutine that is blocked in a syscall, we
>>>>> send
>>>>>>>>> a signal to the target goroutine's thread, and let the signal handler
>>>>>>>>> unwind and scan the stack. Extra care is needed as this races with
>>>>>>>>> enter/exit syscall.
>>>>>>>>>
>>>>>>>>> Currently this is only implemented on GNU/Linux.
>>>>>>>>>
>>>>>>>>> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
>>>>>>>>> to mainline.
>>>>>>>>
>>>>>>>> this broke the libgo build on ARM32:
>>>>>>>>
>>>>>>>> ../../../src/libgo/runtime/go-unwind.c: In function
>>>>>>>> 'scanstackwithmap_callback':
>>>>>>>> ../../../src/libgo/runtime/go-unwind.c:754:18: error:
>>>>> '_URC_NORMAL_STOP'
>>>>>>>> undeclared (first use in this function)
>>>>>>>>   754 |           return _URC_NORMAL_STOP;
>>>>>>>>       |                  ^~~~~~~~~~~~~~~~
>>>>>>>> ../../../src/libgo/runtime/go-unwind.c:754:18: note: each undeclared
>>>>>>>> identifier
>>>>>>>> is reported only once for each function i
>>>>>>>> t appears in
>>>>>>>> ../../../src/libgo/runtime/go-unwind.c: In function
>>>>>>>> 'probestackmaps_callback':
>>>>>>>> ../../../src/libgo/runtime/go-unwind.c:802:10: error:
>>>>> '_URC_NORMAL_STOP'
>>>>>>>> undeclared (first use in this function)
>>>>>>>>   802 |   return _URC_NORMAL_STOP;
>>>>>>>>       |          ^~~~~~~~~~~~~~~~
>>>>>>>> ../../../src/libgo/runtime/go-unwind.c:803:1: warning: control
>>>>> reaches end
>>>>>>>> of
>>>>>>>> non-void function [-Wreturn-type]
>>>>>>>>   803 | }
>>>>>>>>       | ^
>>>>>>>> make[6]: *** [Makefile:1474: runtime/go-unwind.lo] Error 1
>>>>>>>> make[6]: Leaving directory
>>>>>>>> '/<<PKGBUILDDIR>>/build/arm-linux-gnueabihf/libgo'
>>>>>>>>
>>>>>>>>
>>>>>>> Hell Matthias,
>>>>>>>
>>>>>>> Thank you for the report. And sorry about the breakage. Does
>>>>>>> https://go-review.googlesource.com/c/gofrontend/+/153417 (or the patch
>>>>>>> below) fix ARM32 build? I don't have an ARM32 machine at hand to test.
>>>>>>
>>>>>> this fixes the build.
>>>>>>
>>>>>> currently running the testsuite, almost every test case core dumps on
>>>>>> arm-linux-gnueabihf
>>>>>
>>>>> I committed Cherry's patch to trunk, since it looks reasonable to me.
>>>>> Cherry, Matthias, let me know if you figure out why programs are
>>>>> failing.
>>>>>
>>>>>
>>>> Thank you.
>>>>
>>>> I don't know for the moment. I'm trying to find an ARM32 machine so I can
>>>> test.
>>>>
>>>> Matthias, is it convenient for you to share a stack trace for the failing
>>>> programs? That would be very helpful. Thanks!
>>>
>>> I'll do a local build this weekend. For now you find the build log at
>>> https://launchpad.net/ubuntu/+source/gcc-snapshot/1:20181210-0ubuntu1/+build/15759748

  reply	other threads:[~2018-12-19  3:24 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-05 23:10 Ian Lance Taylor
2018-12-07 10:07 ` Rainer Orth
2018-12-07 14:23   ` Ian Lance Taylor
2018-12-07 15:15     ` [gofrontend-dev] " Cherry Zhang via gcc-patches
2018-12-10  6:41 ` Matthias Klose
2018-12-10 15:54   ` [gofrontend-dev] " Cherry Zhang via gcc-patches
2018-12-11 14:52     ` Matthias Klose
2018-12-11 20:51       ` Ian Lance Taylor
2018-12-11 21:02         ` Cherry Zhang via gcc-patches
2018-12-12  0:03           ` Matthias Klose
2018-12-12 16:10             ` Cherry Zhang via gcc-patches
2018-12-12 23:27               ` Ian Lance Taylor
2018-12-19  3:24                 ` Matthias Klose [this message]
2018-12-26 20:42                   ` Cherry Zhang via gcc-patches
2018-12-27  9:42                     ` Ian Lance Taylor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bc93b1ba-5ee4-71c6-24a2-0afe349c8e4a@ubuntu.com \
    --to=doko@ubuntu.com \
    --cc=cherryyz@google.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=gofrontend-dev@googlegroups.com \
    --cc=iant@golang.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).