code review 145570043: runtime: initialize traceback variables earlier

runtime: initialize traceback variables earlier

Our traceback code needs to know the PC of several special
functions, including goexit, mcall, etc.  Make sure that
these PCs are initialized before any traceback occurs.

Fixes issue 8766

[khr, 2014-09-29 20:42:13]

Hello golang-codereviews@googlegroups.com,

I'd like you to review this change to
https://khr%40golang.org@code.google.com/p/go/

[rsc, 2014-09-29 22:59:45]

LGTM but please add a test

[khr1, 2014-09-29 23:54:25]

That's going to be a pain to test.  Everything is correctly initialized by
the time any test code is run.  It is only during runtime's own
initialization that these failures happen.
I could make runtime call traceback in a few init files, but I don't think
we'd want to leave that in a release.

On Mon, Sep 29, 2014 at 3:59 PM, <rsc@golang.org> wrote:

> LGTM but please add a test
>
>
> https://codereview.appspot.com/145570043/
>

[bradfitz, 2014-09-30 00:34:39]

Could make an environment variable (GO_TEST_TRACEBACK=1 ?) trigger an
early-init traceback and then have a foo_test.go that runs a go child
process with that env set?

That does mean we'd have test hook code in every Go binary, potentially,
without extra work to modify the compiler to, say, make it conditional on a
build tag or something. But then that's more magic.


On Mon, Sep 29, 2014 at 4:54 PM, 'Keith Randall' via golang-codereviews <
golang-codereviews@googlegroups.com> wrote:

> That's going to be a pain to test.  Everything is correctly initialized by
> the time any test code is run.  It is only during runtime's own
> initialization that these failures happen.
> I could make runtime call traceback in a few init files, but I don't think
> we'd want to leave that in a release.
>
> On Mon, Sep 29, 2014 at 3:59 PM, <rsc@golang.org> wrote:
>
>> LGTM but please add a test
>>
>>
>> https://codereview.appspot.com/145570043/
>>
>
>  --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

[rsc, 2014-09-30 01:38:56]

LGTM

https://codereview.appspot.com/145570043/diff/60001/src/runtime/traceback.go
File src/runtime/traceback.go (right):

https://codereview.appspot.com/145570043/diff/60001/src/runtime/traceback.go#...
src/runtime/traceback.go:48: func tracebackinit() {
In lieu of a test, add a comment:

// Go variable initialization happens late during runtime startup.
// Instead of initializing the variables above in the declarations,
// define tracebackinit and call it from schedinit, so that they are
// available earlier in the startup sequence.

https://codereview.appspot.com/145570043/diff/60001/src/runtime/traceback.go#...
src/runtime/traceback.go:97: gothrow("goexitPC not set up yet")
gothrow("gentraceback before goexitPC initialization")

[khr, 2014-09-30 04:21:41]

*** Submitted as https://code.google.com/p/go/source/detail?r=a46889342ac8 ***

runtime: initialize traceback variables earlier

Our traceback code needs to know the PC of several special
functions, including goexit, mcall, etc.  Make sure that
these PCs are initialized before any traceback occurs.

Fixes issue 8766

LGTM=rsc
R=golang-codereviews, rsc, khr, bradfitz
CC=golang-codereviews
https://codereview.appspot.com/145570043