[dev.cc] code review 171660043: [dev.cc] runtime: convert openbsd/amd64 runtime from C to Go

runtime: convert openbsd/amd64 port to Go

[jsing, 2014-11-13 16:11:59]

Hello rsc@golang.org (cc: golang-codereviews@googlegroups.com),

I'd like you to review this change to the dev.cc branch of
https://go.googlecode.com/hg/

[bradfitz, 2014-11-13 16:19:43]

https://codereview.appspot.com/171660043/diff/60001/src/runtime/defs_openbsd_...
File src/runtime/defs_openbsd_amd64.go (right):

https://codereview.appspot.com/171660043/diff/60001/src/runtime/defs_openbsd_...
src/runtime/defs_openbsd_amd64.go:90: tf_stack uintptr
this file changes 4 pointers to uintptr... is something else keeping these
alive?

[jsing, 2014-11-13 16:33:51]

https://codereview.appspot.com/171660043/diff/60001/src/runtime/defs_openbsd_...
File src/runtime/defs_openbsd_amd64.go (right):

https://codereview.appspot.com/171660043/diff/60001/src/runtime/defs_openbsd_...
src/runtime/defs_openbsd_amd64.go:90: tf_stack uintptr
On 2014/11/13 16:19:42, bradfitz wrote:
> this file changes 4 pointers to uintptr... is something else keeping these
> alive?

For sigaltstackt and stackt, I presume the stack handling code is taking care of
this (at least stackt.ss_sp was changed for FreeBSD, interestingly
sigaltstackt.ss_sp was not). The tforkt tf_stack should be the same - however,
tf_tcb probably should be an unsafe.Pointer (although I'd hope m.tls does not go
away...).

[rsc, 2014-11-13 18:32:12]

https://codereview.appspot.com/171660043/diff/60001/src/runtime/defs_openbsd_...
File src/runtime/defs_openbsd_amd64.go (right):

https://codereview.appspot.com/171660043/diff/60001/src/runtime/defs_openbsd_...
src/runtime/defs_openbsd_amd64.go:90: tf_stack uintptr
On 2014/11/13 16:19:42, bradfitz wrote:
> this file changes 4 pointers to uintptr... is something else keeping these
> alive?

These are just the args to a system call. They go away when it's done, so
something had better be keeping them alive.

The reason I changed them to uintptr in the other files is that if the collector
sees a *byte pointing to the base of the stack (or worse, the top, which is
really another stack) it might report finding a pointer it should not have.
Better not to pretend they are pointers. They're just memory addresses.

[rsc, 2014-11-13 18:33:55]

LGTM

https://codereview.appspot.com/171660043/diff/80001/src/runtime/defs_openbsd_...
File src/runtime/defs_openbsd_amd64.go (right):

https://codereview.appspot.com/171660043/diff/80001/src/runtime/defs_openbsd_...
src/runtime/defs_openbsd_amd64.go:156: func (ts *timespec) set_nsec(x int32) {
This is fine but I don't think it's necessary.

https://codereview.appspot.com/171660043/diff/80001/src/runtime/signal_openbs...
File src/runtime/signal_openbsd_amd64.go (right):

https://codereview.appspot.com/171660043/diff/80001/src/runtime/signal_openbs...
src/runtime/signal_openbsd_amd64.go:41: return
*(*uint64)(unsafe.Pointer(uintptr(unsafe.Pointer(c.info)) + 16))
*(*uint64)(add(unsafe.Pointer(c.info), 16))

[jsing, 2014-11-14 02:00:19]

https://codereview.appspot.com/171660043/diff/80001/src/runtime/defs_openbsd_...
File src/runtime/defs_openbsd_amd64.go (right):

https://codereview.appspot.com/171660043/diff/80001/src/runtime/defs_openbsd_...
src/runtime/defs_openbsd_amd64.go:156: func (ts *timespec) set_nsec(x int32) {
On 2014/11/13 18:33:54, rsc wrote:
> This is fine but I don't think it's necessary.

tv_nsec is int32 on openbsd/386 and int64 on openbsd/amd64 - this seemed cleaner
than using unsafe.Pointer. If you'd prefer unsafe.Pointer I'll change it...

https://codereview.appspot.com/171660043/diff/80001/src/runtime/signal_openbs...
File src/runtime/signal_openbsd_amd64.go (right):

https://codereview.appspot.com/171660043/diff/80001/src/runtime/signal_openbs...
src/runtime/signal_openbsd_amd64.go:41: return
*(*uint64)(unsafe.Pointer(uintptr(unsafe.Pointer(c.info)) + 16))
On 2014/11/13 18:33:54, rsc wrote:
> *(*uint64)(add(unsafe.Pointer(c.info), 16))

Done (here and below) -thanks.

[jsing, 2014-11-14 02:01:25]

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

[dev.cc] runtime: convert openbsd/amd64 port to Go

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