ogle/probe: rewrite validRead and validWrite to use the new SetPanicOnFault function
LGTM=nigeltao
R=nigeltao
https://golang.org/cl/66700043
diff --git a/probe/addr_test.go b/probe/addr_test.go
index eca60c1..48ff2ac 100644
--- a/probe/addr_test.go
+++ b/probe/addr_test.go
@@ -9,6 +9,12 @@
"testing"
)
+// Defined in assembler.
+func base() uintptr
+func etext() uintptr
+func edata() uintptr
+func end() uintptr
+
var dataItem = 3
var bssItem [100]int
@@ -24,20 +30,12 @@
}
}
-func TestHeapRange(t *testing.T) {
- p := addr(new(int)) // On the heap.
- if p < heapStart() || heapUsed() <= p {
- t.Fatalf("%#x is not in [%#x, %#x)", p, heapStart(), heapUsed())
- }
-}
-
func TestGoodReadAddresses(t *testing.T) {
addrs := []uintptr{
base(),
addr(&t), // On the stack.
addr(&dataItem), // In data.
addr(&bssItem), // In bss.
- heapUsed() - 1,
}
for _, a := range addrs {
if !validRead(a, 1) {
@@ -49,8 +47,7 @@
func TestBadReadAddresses(t *testing.T) {
addrs := []uintptr{
0,
- base() - 1,
- heapUsed() + 1,
+ base()/2 - 1, // Pull well down below; the Mac only unmaps up to 0x1000.
^uintptr(0),
}
for _, a := range addrs {
@@ -65,7 +62,6 @@
addr(&t), // On the stack.
addr(&dataItem), // In data.
addr(&bssItem), // In bss.
- heapUsed() - 1,
}
for _, a := range addrs {
if !validWrite(a, 1) {
@@ -79,7 +75,6 @@
0,
base(), // In the text segment.
base() - 1,
- heapUsed(),
^uintptr(0),
}
for _, a := range addrs {
@@ -88,45 +83,3 @@
}
}
}
-
-type span struct {
- p uintptr
- size int
- ok bool
-}
-
-func TestReadAddressSpan(t *testing.T) {
- spans := []span{
- {base(), 0, true},
- {base(), 1, true},
- {base(), 4096, true},
- {base(), int(heapStart() - base()), false},
- {base(), 1e9, false},
- {heapStart(), 1, true},
- {heapStart(), 4096, true},
- {heapStart(), 1e9, false},
- }
- for _, s := range spans {
- if validRead(s.p, s.size) != s.ok {
- t.Errorf("(%#x,%d) should be %t; is %t", s.p, s.size, s.ok, !s.ok)
- }
- }
-}
-
-func TestWriteAddressSpan(t *testing.T) {
- spans := []span{
- {etext(), 0, true},
- {etext(), 1, true},
- {etext(), 4096, true},
- {etext(), int(heapStart() - base()), false},
- {etext(), 1e9, false},
- {heapStart(), 1, true},
- {heapStart(), 4096, true},
- {heapStart(), 1e9, false},
- }
- for _, s := range spans {
- if validWrite(s.p, s.size) != s.ok {
- t.Errorf("(%#x,%d) should be %t; is %t", s.p, s.size, s.ok, !s.ok)
- }
- }
-}
diff --git a/probe/net_test.go b/probe/net_test.go
index 7fbb4ad..fbee543 100644
--- a/probe/net_test.go
+++ b/probe/net_test.go
@@ -73,7 +73,7 @@
// Request a read of a bad address.
conn.output.WriteByte('r')
// Address.
- n := putUvarint(tmp[:], uint64(base()-8))
+ n := putUvarint(tmp[:], 0)
conn.output.Write(tmp[:n])
// Length. Any length will do.
n = putUvarint(tmp[:], 8)
diff --git a/probe/probe.go b/probe/probe.go
index c3fdc3d..24b75af 100644
--- a/probe/probe.go
+++ b/probe/probe.go
@@ -7,63 +7,58 @@
package probe
import (
+ "runtime/debug"
"unsafe"
)
-// Defined in assembler.
-func base() uintptr
-func etext() uintptr
-func edata() uintptr
-func end() uintptr
-
-func heapStart() uintptr
-func heapUsed() uintptr
-func heapEnd() uintptr
+// catchFault is used by the read and write routines to turn a panic into an error return.
+func catchFault(ok *bool) {
+ if e := recover(); e != nil {
+ *ok = false
+ }
+}
// validRead reports whether a read of the specified size can be done at address p.
-func validRead(p uintptr, size int) bool {
+// TODO: It does this by actually doing the read and seeing if it succeeds. Do better.
+func validRead(p uintptr, size int) (ok bool) {
// Check for negative size and for (p + size) overflow.
if size < 0 || uint64(^uintptr(0)-p) < uint64(size) {
return false
}
- // The read must be in a single contiguous valid region.
- switch {
- case base() <= p && p < end():
- // Assumes text is before data, but ld's binaries always satisfy that constraint.
- p += uintptr(size)
- return base() <= p && p <= end()
- case heapStart() <= p && p < heapUsed(): // Don't allow reads past the used part of the heap.
- p += uintptr(size)
- return heapStart() <= p && p <= heapUsed()
+ defer catchFault(&ok)
+ defer debug.SetPanicOnFault(debug.SetPanicOnFault(true))
+ ep := p + uintptr(size)
+ var b byte
+ for p < ep {
+ b = *(*byte)(unsafe.Pointer(p))
+ _ = b
+ p++
}
- return false
+ return true
}
// validWrite reports whether a write of the specified size can be done at address p.
-func validWrite(p uintptr, size int) bool {
+// TODO: It does this by actually doing a write and seeing if it succeeds. Do better.
+func validWrite(p uintptr, size int) (ok bool) {
// Check for negative size and for (p + size) overflow.
if size < 0 || uint64(^uintptr(0)-p) < uint64(size) {
return false
}
- // The write must be in a single contiguous valid region.
- switch {
- case etext() <= p && p < end():
- // Assumes text is before data, but ld's binaries always satisfy that constraint.
- p += uintptr(size)
- return etext() <= p && p <= end()
- case heapStart() <= p && p < heapUsed(): // Don't allow writes past the used part of the heap.
- p += uintptr(size)
- return heapStart() <= p && p <= heapUsed()
+ defer catchFault(&ok)
+ defer debug.SetPanicOnFault(debug.SetPanicOnFault(true))
+ ep := p + uintptr(size)
+ for p < ep {
+ *(*byte)(unsafe.Pointer(p)) = *(*byte)(unsafe.Pointer(p))
+ p++
}
- return false
+ return true
}
// read copies into the argument buffer the contents of memory starting at address p.
// Its boolean return tells whether it succeeded. If it fails, no bytes were copied.
-func read(p uintptr, buf []byte) bool {
- if !validRead(p, len(buf)) {
- return false
- }
+func read(p uintptr, buf []byte) (ok bool) {
+ defer catchFault(&ok)
+ defer debug.SetPanicOnFault(debug.SetPanicOnFault(true))
for i := range buf {
buf[i] = *(*byte)(unsafe.Pointer(p))
p++
@@ -73,10 +68,9 @@
// write copies the argument buffer to memory starting at address p.
// Its boolean return tells whether it succeeded. If it fails, no bytes were copied.
-func write(p uintptr, buf []byte) bool {
- if !validWrite(p, len(buf)) {
- return false
- }
+func write(p uintptr, buf []byte) (ok bool) {
+ defer catchFault(&ok)
+ defer debug.SetPanicOnFault(debug.SetPanicOnFault(true))
for i := range buf {
*(*byte)(unsafe.Pointer(p)) = buf[i]
p++
diff --git a/probe/probe_darwin_amd64.s b/probe/probe_darwin_amd64.s
index a29a792..90ec8cc 100644
--- a/probe/probe_darwin_amd64.s
+++ b/probe/probe_darwin_amd64.s
@@ -36,27 +36,3 @@
LEAQ end+0(SB), BX
MOVQ BX, ret+0(FP)
RET
-
-// These are offsets of critical fields of runtime.mheap.
-// TODO: Very susceptible to change! They (or something equivalent) need to be published by runtime.
-#define arena_start_offset 14504
-#define arena_used_offset 14512
-#define arena_end_offset 14520
-
-// start of heap.
-TEXT ·heapStart(SB), NOSPLIT, $0
- MOVQ runtime·mheap+arena_start_offset(SB), BX
- MOVQ BX, ret+0(FP)
- RET
-
-// end of active region of heap.
-TEXT ·heapUsed(SB), NOSPLIT, $0
- MOVQ runtime·mheap+arena_used_offset(SB), BX
- MOVQ BX, ret+0(FP)
- RET
-
-// end of all of heap.
-TEXT ·heapEnd(SB), NOSPLIT, $0
- MOVQ runtime·mheap+arena_end_offset(SB), BX
- MOVQ BX, ret+0(FP)
- RET
diff --git a/probe/probe_linux_amd64.s b/probe/probe_linux_amd64.s
index 9922332..37999a2 100644
--- a/probe/probe_linux_amd64.s
+++ b/probe/probe_linux_amd64.s
@@ -37,27 +37,3 @@
LEAQ end+0(SB), BX
MOVQ BX, ret+0(FP)
RET
-
-// These are offsets of critical fields of runtime.mheap.
-// TODO: Very susceptible to change! They (or something equivalent) need to be published by runtime.
-#define arena_start_offset 14504
-#define arena_used_offset 14512
-#define arena_end_offset 14520
-
-// start of heap.
-TEXT ·heapStart(SB), NOSPLIT, $0
- MOVQ runtime·mheap+arena_start_offset(SB), BX
- MOVQ BX, ret+0(FP)
- RET
-
-// end of active region of heap.
-TEXT ·heapUsed(SB), NOSPLIT, $0
- MOVQ runtime·mheap+arena_used_offset(SB), BX
- MOVQ BX, ret+0(FP)
- RET
-
-// end of all of heap.
-TEXT ·heapEnd(SB), NOSPLIT, $0
- MOVQ runtime·mheap+arena_end_offset(SB), BX
- MOVQ BX, ret+0(FP)
- RET
