code review 60070045: ogle/probe: start of package

ogle/probe: start of package
Implement functions to test validity of addresses for read and write.
Needs some very hacky assembly support; provide this for amd64
only, darwin and linux only.

[r, 2014-02-06 01:00:25]

Hello nigeltao,

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

[nigeltao, 2014-02-06 04:33:25]

LGTM.

https://codereview.appspot.com/60070045/diff/1/probe/addr_test.go
File probe/addr_test.go (right):

https://codereview.appspot.com/60070045/diff/1/probe/addr_test.go#newcode33
probe/addr_test.go:33: }
Add "heapUsed() - 1"?

https://codereview.appspot.com/60070045/diff/1/probe/addr_test.go#newcode46
probe/addr_test.go:46: 1<<64 - 1,
Strictly speaking, should this be "^uintptr(0)", for 32-bit programs?

https://codereview.appspot.com/60070045/diff/1/probe/addr_test.go#newcode75
probe/addr_test.go:75: 1<<64 - 1,
Ditto.

https://codereview.appspot.com/60070045/diff/1/probe/probe.go
File probe/probe.go (right):

https://codereview.appspot.com/60070045/diff/1/probe/probe.go#newcode9
probe/probe.go:9: func base() uintptr
Perhaps order these four functions as base, etext, edata and end, if that's the
order you expect their values to happen in? This would match the order in the .s
files.

https://codereview.appspot.com/60070045/diff/1/probe/probe.go#newcode42
probe/probe.go:42: case etext() <= p && p < end():
Is the text guaranteed to be placed before data? I don't know enough about ELF
or Mach-O to say in general. Or are we assuming that the Go toolchain will
always do this?

https://codereview.appspot.com/60070045/diff/1/probe/probe.go#newcode45
probe/probe.go:45: case heapStart() <= p && p < heapUsed(): // Don't allow reads
past the used part of the heap.
s/reads/writes/

https://codereview.appspot.com/60070045/diff/1/probe/probe_darwin_amd64.s
File probe/probe_darwin_amd64.s (right):

https://codereview.appspot.com/60070045/diff/1/probe/probe_darwin_amd64.s#new...
probe/probe_darwin_amd64.s:15: LEAQ	main·main+0(SB), BX
This will probably work as a placeholder, but is main.main guaranteed to be the
base of the address space??

$ cat main.go 
package main

func maininc(x int) int {
	return x + 1
}

func main() {
	println(maininc(42))
}
$ go build -gcflags '-l' .
$ readelf --symbols scratch | grep main.main
  1180: 0000000000400c00    16 FUNC    GLOBAL DEFAULT    1 main.maininc
  1181: 0000000000400c10    64 FUNC    GLOBAL DEFAULT    1 main.main

[r, 2014-02-06 06:18:50]

No, main is not guaranteed to be first but i don't have a better idea
at the moment. This is just to get us started. Also: remember this
program is dealing with Go binaries. They are linked by our linker,
and its properties are well-defined. We'll need to get gccgo support
working some day, but not yet and it shouldn't be hard.

I'd be very surprised if our toolchain puts text above heap. It always
puts bss after data after text.

But again, this is just to get us started.

-rob


On Wed, Feb 5, 2014 at 8:33 PM,  <nigeltao@golang.org> wrote:
> LGTM.
>
>
> https://codereview.appspot.com/60070045/diff/1/probe/addr_test.go
> File probe/addr_test.go (right):
>
> https://codereview.appspot.com/60070045/diff/1/probe/addr_test.go#newcode33
> probe/addr_test.go:33: }
> Add "heapUsed() - 1"?
>
> https://codereview.appspot.com/60070045/diff/1/probe/addr_test.go#newcode46
> probe/addr_test.go:46: 1<<64 - 1,
> Strictly speaking, should this be "^uintptr(0)", for 32-bit programs?
>
> https://codereview.appspot.com/60070045/diff/1/probe/addr_test.go#newcode75
> probe/addr_test.go:75: 1<<64 - 1,
> Ditto.
>
> https://codereview.appspot.com/60070045/diff/1/probe/probe.go
> File probe/probe.go (right):
>
> https://codereview.appspot.com/60070045/diff/1/probe/probe.go#newcode9
> probe/probe.go:9: func base() uintptr
> Perhaps order these four functions as base, etext, edata and end, if
> that's the order you expect their values to happen in? This would match
> the order in the .s files.
>
> https://codereview.appspot.com/60070045/diff/1/probe/probe.go#newcode42
> probe/probe.go:42: case etext() <= p && p < end():
> Is the text guaranteed to be placed before data? I don't know enough
> about ELF or Mach-O to say in general. Or are we assuming that the Go
> toolchain will always do this?
>
> https://codereview.appspot.com/60070045/diff/1/probe/probe.go#newcode45
> probe/probe.go:45: case heapStart() <= p && p < heapUsed(): // Don't
> allow reads past the used part of the heap.
> s/reads/writes/
>
> https://codereview.appspot.com/60070045/diff/1/probe/probe_darwin_amd64.s
> File probe/probe_darwin_amd64.s (right):
>
>
https://codereview.appspot.com/60070045/diff/1/probe/probe_darwin_amd64.s#new...
> probe/probe_darwin_amd64.s:15: LEAQ     main·main+0(SB), BX
> This will probably work as a placeholder, but is main.main guaranteed to
> be the base of the address space??
>
> $ cat main.go
> package main
>
> func maininc(x int) int {
>         return x + 1
> }
>
> func main() {
>         println(maininc(42))
> }
> $ go build -gcflags '-l' .
> $ readelf --symbols scratch | grep main.main
>   1180: 0000000000400c00    16 FUNC    GLOBAL DEFAULT    1 main.maininc
>   1181: 0000000000400c10    64 FUNC    GLOBAL DEFAULT    1 main.main
>
> https://codereview.appspot.com/60070045/

[r, 2014-02-06 19:09:41]

*** Submitted as https://code.google.com/p/ogle/source/detail?r=491eb7565e88 ***

ogle/probe: start of package
Implement functions to test validity of addresses for read and write.
Needs some very hacky assembly support; provide this for amd64
only, darwin and linux only.

LGTM=nigeltao
R=nigeltao
https://codereview.appspot.com/60070045

[nigeltao, 2014-02-06 22:53:48]

On Thu, Feb 6, 2014 at 5:18 PM, Rob Pike <r@golang.org> wrote:
> No, main is not guaranteed to be first but i don't have a better idea
> at the moment. This is just to get us started. Also: remember this
> program is dealing with Go binaries. They are linked by our linker,
> and its properties are well-defined. We'll need to get gccgo support
> working some day, but not yet and it shouldn't be hard.

Well, if it's our linker, I suspect that base will effectively always
be 0x400000 on linux. We could hard-code that.

[nigeltao, 2014-02-06 22:57:04]

On Fri, Feb 7, 2014 at 9:53 AM, Nigel Tao <nigeltao@golang.org> wrote:
> Well, if it's our linker, I suspect that base will effectively always
> be 0x400000 on linux. We could hard-code that.

Never mind, I see you've already done:
#define INITTEXT ((1<<22)+ELFRESERVE)