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

Comment From: gabyhelp

Related Issues

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

Comment From: randall77

Closing as a dup of #68170.

Comment From: ruyi789

@mvdan

Comment From: timothy-king

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.)

Comment From: ruyi789

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.

Comment From: ruyi789

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

Comment From: ruyi789

@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