public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Ian Lance Taylor <iant@golang.org>
To: Cherry Zhang <cherryyz@google.com>
Cc: Matthias Klose <doko@ubuntu.com>,
	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, 12 Dec 2018 23:27:00 -0000	[thread overview]
Message-ID: <CAOyqgcWvHNRnKEjFkivpp-OvdJ=bkCd537Ot2t3VozuNfW2+EA@mail.gmail.com> (raw)
In-Reply-To: <CAHpjNSJaexNsKPhsdZj=foir7NXb_tu8XbMYjQjFFsXCBWtEMg@mail.gmail.com>

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-12 23:27 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 [this message]
2018-12-19  3:24                 ` Matthias Klose
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='CAOyqgcWvHNRnKEjFkivpp-OvdJ=bkCd537Ot2t3VozuNfW2+EA@mail.gmail.com' \
    --to=iant@golang.org \
    --cc=cherryyz@google.com \
    --cc=doko@ubuntu.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=gofrontend-dev@googlegroups.com \
    /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).