code review 134200044: runtime: convert traceback*.c to Go

runtime: convert traceback*.c to Go

The two converted files were nearly identical.
Instead of continuing that duplication, I merged them
into a single traceback.go.

Tested on arm, amd64, amd64p32, and 386.

[rsc, 2014-09-02 03:58:34]

Hello golang-codereviews@googlegroups.com (cc: dvyukov, iant, khr, r),

I'd like you to review this change to
https://code.google.com/p/go/

[remyoudompheng, 2014-09-02 05:24:05]

https://codereview.appspot.com/134200044/diff/140001/src/pkg/runtime/extern.go
File src/pkg/runtime/extern.go (right):

https://codereview.appspot.com/134200044/diff/140001/src/pkg/runtime/extern.g...
src/pkg/runtime/extern.go:146: return int(callers(skip, &pc[0], len(pc)))
callers now returns int

[dave_cheney.net, 2014-09-02 06:04:54]

Is it safe to use the address of the empty struct as a signal to the trace
back code? Don't all empty structs share the same address?
On 2 Sep 2014 13:58, <rsc@golang.org> wrote:

> Reviewers: golang-codereviews,
>
> Message:
> Hello golang-codereviews@googlegroups.com (cc: dvyukov, iant, khr, r),
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> runtime: convert traceback*.c to Go
>
> The two converted files were nearly identical.
> Instead of continuing that duplication, I merged them
> into a single traceback.go.
>
> Tested on arm, amd64, amd64p32, and 386.
>
> Please review this at https://codereview.appspot.com/134200044/
>
> Affected files (+677, -46 lines):
>   M src/cmd/api/goapi.go
>   M src/pkg/runtime/extern.go
>   M src/pkg/runtime/heapdump.c
>   M src/pkg/runtime/mgc0.c
>   M src/pkg/runtime/mprof.go
>   A src/pkg/runtime/os_windows_386.go
>   A src/pkg/runtime/os_windows_amd64.go
>   M src/pkg/runtime/runtime.h
>   M src/pkg/runtime/stack.c
>   M src/pkg/runtime/stubs.go
>   A src/pkg/runtime/traceback.go
>   M src/pkg/runtime/traceback_arm.c
>   A src/pkg/runtime/traceback_windows.go
>   M src/pkg/runtime/traceback_x86.c
>
>
> --
> 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-02 18:09:30]

>
https://codereview.appspot.com/134200044/diff/140001/src/pkg/runtime/extern.g...
> src/pkg/runtime/extern.go:146: return int(callers(skip, &pc[0], len(pc)))
> callers now returns int

thanks

[rsc, 2014-09-02 18:10:55]

On 2014/09/02 06:04:54, dfc wrote:
> Is it safe to use the address of the empty struct as a signal to the trace
> back code? Don't all empty structs share the same address?

yes, it is safe. new(struct{}) and &struct{} use the same address.
global variables are global variables.
the struct{} is overridden by the real text symbol.

[rsc, 2014-09-02 18:38:24]

fyi, now with side-by-side diffs in traceback.go

[r, 2014-09-02 18:49:17]

LGTM

https://codereview.appspot.com/134200044/diff/160001/src/pkg/runtime/tracebac...
File src/pkg/runtime/traceback.go (right):

https://codereview.appspot.com/134200044/diff/160001/src/pkg/runtime/tracebac...
src/pkg/runtime/traceback.go:31: const usesLR = GOARCH == "arm"
this definition is kind of the inverse of what the code says. i would write

const usesLR = GOARCH!="386" && GOARCH!="amd64" && GOARCH!="amd64p32"

https://codereview.appspot.com/134200044/diff/160001/src/pkg/runtime/tracebac...
src/pkg/runtime/traceback.go:330: print(hex(argp[i]))
is i guaranteed never to go past 100?

[rsc, 2014-09-02 19:07:09]

https://codereview.appspot.com/134200044/diff/160001/src/pkg/runtime/tracebac...
File src/pkg/runtime/traceback.go (right):

https://codereview.appspot.com/134200044/diff/160001/src/pkg/runtime/tracebac...
src/pkg/runtime/traceback.go:330: print(hex(argp[i]))
On 2014/09/02 18:49:17, r wrote:
> is i guaranteed never to go past 100?

yes: if i >= 10 break above. 100 was overkill.

[rsc, 2014-09-02 19:13:00]

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

runtime: convert traceback*.c to Go

The two converted files were nearly identical.
Instead of continuing that duplication, I merged them
into a single traceback.go.

Tested on arm, amd64, amd64p32, and 386.

LGTM=r
R=golang-codereviews, remyoudompheng, dave, r
CC=dvyukov, golang-codereviews, iant, khr
https://codereview.appspot.com/134200044