runtime/checkptr：Pro 8000 posed some changes to pointer detection #68220

ruyi789 · 2024-06-27T14:42:23Z

Proposal Details

build-debug runs the following code, and when it reaches stack_expansion it reports an error: (invalid pointer found on stack)
You can run it under this site:https://onlinegdb.com/LekF0Chbo

func main() {
	var v = unsafe.Pointer(uintptr(1))
	println("addr", &v, v)
	println(stack_expansion(10000))
	println("addr", &v, v)
}

For this kind of code, you consider it an invalid pointer. If it is valid for me, then it is valid. Whether it's valid or not, we'll leave that aside for now.

var v = unsafe.Pointer(uintptr(1))

For the behaviour that reported the error, I found two places

runtime/stack.go:adjustpointers
runtime/checkptr.go:checkptrArithmetic

I recommend that they only run under build-race, so add the (raceenabled) condition.

func adjustpointers(scanp unsafe.Pointer, bv *bitvector, adjinfo *adjustinfo, f funcInfo) {
   ...
        if raceenabled {
		if f.valid() && 0 < p && p < minLegalPointer && debug.invalidptr != 0 {
			// Looks like a junk value in a pointer slot.
			// Live analysis wrong?
			getg().m.traceback = 2
			print("runtime: bad pointer in frame ", funcname(f), " at ", pp, ": ", hex(p), "\n")
			throw("invalid pointer found on stack")
		}
	}
  ...
}

func checkptrArithmetic(p unsafe.Pointer, originals []unsafe.Pointer) {
	if raceenabled {
		if 0 < uintptr(p) && uintptr(p) < minLegalPointer {
			throw("checkptr: pointer arithmetic computed bad pointer value")
		}
	}
    ....
}

Don't say there are lots of alternatives and that's all we'll discuss. If you say you haven't encountered this situation yet, you just haven't. It's natural to report an error when a pointer read or write is invalid, there's no need to actively report an error, that's my option.

If it is valid for me, then it is valid: https://onlinegdb.com/iW2ppsMTS

The text was updated successfully, but these errors were encountered:

gabyhelp · 2024-06-27T14:45:49Z

Related Issues

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

randall77 · 2024-06-27T14:53:02Z

Closing as a dup of #68170.

ruyi789 · 2024-06-27T14:53:35Z

@mvdan

timothy-king · 2024-06-27T17:28:57Z

Here is a minimized example https://go.dev/play/p/zUPGPmUqkzq :

func stack_expansion(i uint) {
	if i > 0 {
		stack_expansion(i - 1)
	}
}

func main() {
	var v = unsafe.Pointer(uintptr(1))
	stack_expansion(100)

	println(&v)
}

This panics on the playground.

One issue is that unsafe.Pointer(uintptr(1)) does not follow the any of the patterns listed as valid in the documentation https://pkg.go.dev/unsafe#Pointer . This section starts with: "The following patterns involving Pointer are valid." and continues "Code not using these patterns is likely to be invalid today or to become invalid in the future." So this code is considered invalid. A panic is an acceptable reason from the runtime and compiler in the face of invalid code. At least according to the documentation. Simply asserting it is valid does not change the language or what it promises to support.

If you would like the situation to change, I recommend filing a proposal that proposes changes to the valid patterns of unsafe.Pointer. The proposal process is documented here https://github.com/golang/proposal .

(I didn't really follow what your are saying about adjustpointers, but I kinda doubt this matters.)

ruyi789 · 2024-06-27T18:19:36Z

Here is a minimized example https://go.dev/play/p/zUPGPmUqkzq :
func stack_expansion(i uint) {
	if i > 0 {
		stack_expansion(i - 1)
	}
}

func main() {
	var v = unsafe.Pointer(uintptr(1))
	stack_expansion(100)

	println(&v)
}
This panics on the playground.

One issue is that unsafe.Pointer(uintptr(1)) does not follow the any of the patterns listed as valid in the documentation https://pkg.go.dev/unsafe#Pointer . This section starts with: "The following patterns involving Pointer are valid." and continues "Code not using these patterns is likely to be invalid today or to become invalid in the future." So this code is considered invalid. A panic is an acceptable reason from the runtime and compiler in the face of invalid code. At least according to the documentation. Simply asserting it is valid does not change the language or what it promises to support.

If you would like the situation to change, I recommend filing a proposal that proposes changes to the valid patterns of unsafe.Pointer. The proposal process is documented here https://github.com/golang/proposal .

(I didn't really follow what your are saying about adjustpointers, but I kinda doubt this matters.)

Pointer safety discussion I would suggest discussing pointers in the range 0x1-0xFFFF, a range (GO) that is unreadable and unwritable, and theoretically safe, just not for reading and writing.

Regarding writing the value of the code (unsafe.Pointer), a setting in the 0x1-0xFFFF range (or there can be a smaller range) is safe in my hunch. So you won't see a GC error triggered by a savage write like this unsafe.Pointer(0x34534632), as long as the GC excludes the low value range it has nothing going on, and the actual system doesn't allocate low-address memory.

ruyi789 · 2024-06-27T18:37:06Z

If the GC requests memory in the 0x1-0xFFFF range, just discard it, 64k memory is not much of a problem

ruyi789 · 2024-06-27T20:37:39Z

@timothy-king adjustpointers is a highly frequently called function, and if it's not running correctly, it will be detected early on. Pointer detection is not its job, and can be done by moving to build -race if you need a hint. Use raceenabled to mask it, so it won't compile in at compile time, and if I don't mask it, it's featured where it is and doesn't look secure

ruyi789 added the Proposal label Jun 27, 2024

randall77 closed this as completed Jun 27, 2024

gabyhelp mentioned this issue May 20, 2025

runtime: valgrind thinks adjustpointer sometimes operates on uninitialized values #73801

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

runtime/checkptr：Pro 8000 posed some changes to pointer detection #68220

runtime/checkptr：Proposed some changes to pointer detection #68220

runtime/checkptr：Pro 8000 posed some changes to pointer detection #68220

runtime/checkptr：Proposed some changes to pointer detection #68220

Comments

Proposal Details