Proposal Details

Background

Recently, I noticed frequent panics in our internal Go code with the error message "cgo argument has Go pointer to Go pointer." After investigation, the issue was traced to a particular CGO call:

C.somefunction(xxx, (unsafe.Pointer(somexp)), xxx)

Interestingly, when I removed the parentheses around unsafe.Pointer, changing it to the following form, the panic no longer occurred:

C.somefunction(xxx, unsafe.Pointer(somexp), xxx)

Theoretically, these two forms should be equivalent, and there should not be a situation where one runs normally, and the other potentially causes a panic.

Issue Analysis

To investigate further, I used go tool cgo to inspect the intermediate code generated by CGO and found that _cgoCheckPointer was inserted for the first code snippet, but not for the second. Additionally, we ruled out the possibility that the parameters passed in contained Go pointers to Go pointers. Therefore, we believe the first panic is a false positive, and the insertion of _cgoCheckPointer might be unnecessary.

By examining the source code of cmd/cgo (source link), I found that it only applies special handling to the unsafe.Pointer used in the second form (i.e., removing the outer unsafe.Pointer wrap). For other uses of unsafe.Pointer, such as the first form or the example below, no special handling is applied, and pointer checking is conservatively assumed:

ptr = unsafe.Pointer(somexp)
C.somfunction(ptr)

This approach seems like a temporary measure rather than a long-term solution, and, in my opinion, is not very elegant. It resembles a "hard-coded" approach that can result in false positives in cases that aren't explicitly considered (like the ones I mentioned initially and the issue below).

Similar Issues

I searched the community issues and found many mentions of this problem, indicating that false positives caused by _cgoCheckPointer are not isolated cases. Examples include:

cmd/cgo: sometimes when passing a struct to C containing a go pointer to a struct field, the entire go struct is checked for go pointers instead of just the field · Issue #63460 · golang/go (github.com)

cmd/cgo: recognize that unsafe.StringData only points to a string · Issue #59954 · golang/go

runtime: cgoCheckUnknownPointer triggering incorrectly · Issue #54558 · golang/go

cmd/cgo: pointer check is too aggressive with bytes.Buffer · Issue #22788 · golang/go

gccgo: cgo: false positive cgo argument has Go pointer to Go pointer · Issue #23391 · golang/go

These false positives are challenging to debug, often requiring a long time to locate. Additionally, the current "hard-coded" method seems insufficient for handling new features as they are introduced. Fixes can only be applied when new features trigger specific bugs (such as cmd/cgo: recognize that unsafe.StringData only points to a string · Issue #59954 · golang/go). This approach also lacks elegance and, as more new features are added, the patch code will increase, making it eventually unmaintainable.

Summary of Current Approach

To sum up, in my opinion, current approach is a temporary, "hard-coded" solution that lacks elegance and can lead to false positives in cases that aren't explicitly considered. What's more, it is insufficient for handling new features, as fixes are only applied when specific bugs are triggered.

My Preliminary Thoughts

Therefore, I propose:

1. Integrating CGO as part of the Go compiler, rather than as a standalone tool.
2. Allowing CGO to reuse the compiler's analysis pass results, such as type inference and pointer analysis results. By reusing these results, it can more accurately determine the actual type that unsafe.Pointer points to, thereby inserting _cgoCheckPointer more precisely and avoiding the aforementioned false positives.

The above are my preliminary thoughts; I welcome any criticism and corrections.