{"action":"created","issue":{"url":"https://api.github.com/repos/golang/go/issues/23391","repository_url":"https://api.github.com/repos/golang/go","labels_url":"https://api.github.com/repos/golang/go/issues/23391/labels{/name}","comments_url":"https://api.github.com/repos/golang/go/issues/23391/comments","events_url":"https://api.github.com/repos/golang/go/issues/23391/events","html_url":"https://github.com/golang/go/issues/23391","id":287229980,"number":23391,"title":"gccgo: cgo: false positive cgo argument has Go pointer to Go pointer","user":{"login":"tamird","id":1535036,"avatar_url":"https://avatars0.githubusercontent.com/u/1535036?v=4","gravatar_id":"","url":"https://api.github.com/users/tamird","html_url":"https://github.com/tamird","followers_url":"https://api.github.com/users/tamird/followers","following_url":"https://api.github.com/users/tamird/following{/other_user}","gists_url":"https://api.github.com/users/tamird/gists{/gist_id}","starred_url":"https://api.github.com/users/tamird/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tamird/subscriptions","organizations_url":"https://api.github.com/users/tamird/orgs","repos_url":"https://api.github.com/users/tamird/repos","events_url":"https://api.github.com/users/tamird/events{/privacy}","received_events_url":"https://api.github.com/users/tamird/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/golang/go/milestones/23","html_url":"https://github.com/golang/go/milestone/23","labels_url":"https://api.github.com/repos/golang/go/milestones/23/labels","id":1069171,"number":23,"title":"Gccgo","description":"Issues to be fixed in the gccgo distribution (as opposed to the main Go repos).\r\n","creator":{"login":"rsc","id":104030,"avatar_url":"https://avatars1.githubusercontent.com/u/104030?v=4","gravatar_id":"","url":"https://api.github.com/users/rsc","html_url":"https://github.com/rsc","followers_url":"https://api.github.com/users/rsc/followers","following_url":"https://api.github.com/users/rsc/following{/other_user}","gists_url":"https://api.github.com/users/rsc/gists{/gist_id}","starred_url":"https://api.github.com/users/rsc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rsc/subscriptions","organizations_url":"https://api.github.com/users/rsc/orgs","repos_url":"https://api.github.com/users/rsc/repos","events_url":"https://api.github.com/users/rsc/events{/privacy}","received_events_url":"https://api.github.com/users/rsc/received_events","type":"User","site_admin":false},"open_issues":45,"closed_issues":127,"state":"open","created_at":"2015-04-15T17:34:15Z","updated_at":"2018-02-01T21:04:11Z","due_on":null,"closed_at":null},"comments":13,"created_at":"2018-01-09T21:05:08Z","updated_at":"2018-02-01T21:21:38Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"### What version of Go are you using (`go version`)?\r\n`go version go1.9 gccgo (GCC) 8.0.0 20180108 (experimental) linux/amd64`\r\n\r\n### Description\r\n\r\nThe following code exists in CockroachDB:\r\nhttps://github.com/cockroachdb/cockroach/blob/4e6ec545cc65dc0051a797996e9642b4b5e8bdc2/pkg/storage/engine/rocksdb.go#L447-L450\r\n\r\nhttps://github.com/cockroachdb/cockroach/blob/4e6ec545cc65dc0051a797996e9642b4b5e8bdc2/pkg/storage/engine/rocksdb.go#L584\r\n\r\nhttps://github.com/cockroachdb/cockroach/blob/4e6ec545cc65dc0051a797996e9642b4b5e8bdc2/c-deps/libroach/db.cc#L1616\r\n\r\nSaid plainly, initialization across cgo is done by passing a Go `**C.DBEngine` which points to `nil` to a C function that allocates and sets the pointed-to `*C.DBEngine`. This code works perfectly when compiled with gc, but produces this error when compiled with gccgo:\r\n```\r\npanic: runtime error: cgo argument has Go pointer to Go pointer\r\n\r\ngoroutine 24 [running]:\r\npanic\r\n        ../../../src/libgo/go/runtime/panic.go:543\r\ngithub_com_cockroachdb_cockroach_pkg_storage_engine._cgoCheckPointer\r\n        /usr/local/home/tduberstein/go/src/github.com/cockroachdb/cockroach/pkg/storage/engine/cgo-c-prolog-gccgo:74\r\nengine.$nested21\r\n        /usr/local/home/tduberstein/go/src/github.com/cockroachdb/cockroach/pkg/storage/engine/rocksdb.go:584\r\ngithub_com_cockroachdb_cockroach_pkg_storage_engine.open.pN59_github_com_cockroachdb_cockroach_pkg_storage_engine.RocksDB\r\n        /usr/local/home/tduberstein/go/src/github.com/cockroachdb/cockroach/pkg/storage/engine/rocksdb.go:584\r\nengine.newMemRocksDB\r\n        /usr/local/home/tduberstein/go/src/github.com/cockroachdb/cockroach/pkg/storage/engine/rocksdb.go:518\r\ngithub_com_cockroachdb_cockroach_pkg_storage_engine.NewInMem\r\n        /usr/local/home/tduberstein/go/src/github.com/cockroachdb/cockroach/pkg/storage/engine/in_mem.go:37\r\ngithub_com_cockroachdb_cockroach_pkg_server.CreateEngines.pN50_github_com_cockroachdb_cockroach_pkg_server.Config\r\n        /usr/local/home/tduberstein/go/src/github.com/cockroachdb/cockroach/pkg/server/config.go:483\r\ngithub_com_cockroachdb_cockroach_pkg_server.Start.pN50_github_com_cockroachdb_cockroach_pkg_server.Server\r\n        /usr/local/home/tduberstein/go/src/github.com/cockroachdb/cockroach/pkg/server/server.go:858\r\ncli.$nested72\r\n        /usr/local/home/tduberstein/go/src/github.com/cockroachdb/cockroach/pkg/cli/start.go:518\r\ncli.$nested70\r\n        /usr/local/home/tduberstein/go/src/github.com/cockroachdb/cockroach/pkg/cli/start.go:500\r\ncreated by cli.runStart\r\n        /usr/local/home/tduberstein/go/src/github.com/cockroachdb/cockroach/pkg/cli/start.go:474 +3255\r\n```\r\n\r\nAccording to https://golang.org/cmd/cgo/#hdr-Passing_pointers:\r\n> Go code may pass a Go pointer to C provided the Go memory to which it points does not contain any Go pointers.\r\n\r\nIt's not exactly clear if nil Go pointers are permitted, but the gc implementation certainly permits it. Perhaps gccgo is missing a null check?\r\n\r\n@ianlancetaylor \r\n  "},"comment":{"url":"https://api.github.com/repos/golang/go/issues/comments/362405599","html_url":"https://github.com/golang/go/issues/23391#issuecomment-362405599","issue_url":"https://api.github.com/repos/golang/go/issues/23391","id":362405599,"user":{"login":"tamird","id":1535036,"avatar_url":"https://avatars0.githubusercontent.com/u/1535036?v=4","gravatar_id":"","url":"https://api.github.com/users/tamird","html_url":"https://github.com/tamird","followers_url":"https://api.github.com/users/tamird/followers","following_url":"https://api.github.com/users/tamird/following{/other_user}","gists_url":"https://api.github.com/users/tamird/gists{/gist_id}","starred_url":"https://api.github.com/users/tamird/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tamird/subscriptions","organizations_url":"https://api.github.com/users/tamird/orgs","repos_url":"https://api.github.com/users/tamird/repos","events_url":"https://api.github.com/users/tamird/events{/privacy}","received_events_url":"https://api.github.com/users/tamird/received_events","type":"User","site_admin":false},"created_at":"2018-02-01T21:21:38Z","updated_at":"2018-02-01T21:21:38Z","author_association":"CONTRIBUTOR","body":"OK, here's the actual panic backtrace:\r\n```\r\n#0  runtime.gopanic (e=...) at ../../../src/libgo/go/runtime/panic.go:498\r\n#1  0x00007ffff6d0114b in runtime.cgoCheckUnknownPointer (msg=..., p=<optimized out>) at ../../../src/libgo/go/runtime/cgocall.go:238\r\n#2  runtime.cgoCheckArg (t=<optimized out>, p=<optimized out>, indir=indir@entry=true, top=top@entry=true, msg=...)\r\n    at ../../../src/libgo/go/runtime/cgocall.go:206\r\n#3  0x00007ffff6d0124c in runtime.cgoCheckArg (t=<optimized out>, p=<optimized out>, indir=indir@entry=true, top=top@entry=true, msg=...)\r\n    at ../../../src/libgo/go/runtime/cgocall.go:192\r\n#4  0x00007ffff6d0124c in runtime.cgoCheckArg (\r\n    t=t@entry=0x1e63960 <github_com_cockroachdb_cockroach_pkg_storage_engine..github_com_cockroachdb_cockroach_pkg_storage_engine._Ctype_struct___7..d>, p=<optimized out>, indir=<optimized out>, top=<optimized out>, msg=...) at ../../../src/libgo/go/runtime/cgocall.go:192\r\n#5  0x00007ffff6d015bb in runtime.cgoCheckPointer (ptr=..., args=...) at ../../../src/libgo/go/runtime/cgocall.go:93\r\n#6  0x0000000000f782d7 in github_com_cockroachdb_cockroach_pkg_storage_engine._cgoCheckPointer (ptr=..., args=...) at cgo-c-prolog-gccgo:74\r\n#7  0x0000000000f3c4d4 in engine.func1 (_cgo0=0xc420a3c5f0, _cgo1=..., _cgo2=...)\r\n    at /home/tduberstein/local/go/src/github.com/cockroachdb/cockroach/pkg/storage/engine/rocksdb.go:584\r\n#8  0x0000000000f3c040 in github_com_cockroachdb_cockroach_pkg_storage_engine.RocksDB.open (r=0xc420a3c580)\r\n    at /home/tduberstein/local/go/src/github.com/cockroachdb/cockroach/pkg/storage/engine/rocksdb.go:584\r\n#9  0x0000000000f3b57b in engine.newMemRocksDB (attrs=..., param=..., MaxSizeBytes=536870912)\r\n    at /home/tduberstein/local/go/src/github.com/cockroachdb/cockroach/pkg/storage/engine/rocksdb.go:518\r\n#10 0x0000000000e2f30e in github_com_cockroachdb_cockroach_pkg_storage_engine.NewInMem (attrs=..., cacheSize=1048576)\r\n    at /home/tduberstein/local/go/src/github.com/cockroachdb/cockroach/pkg/storage/engine/in_mem.go:37\r\n#11 0x0000000000e4d84f in engine.testBatchBasics (t=0xc420b180f0, writeOnly=false, \r\n    commit=0x1e521a0 <github_com_cockroachdb_cockroach_pkg_storage_engine.TestBatchBasics..func1..f>)\r\n    at /home/tduberstein/local/go/src/github.com/cockroachdb/cockroach/pkg/storage/engine/batch_test.go:56\r\n#12 0x0000000000e5000f in github_com_cockroachdb_cockroach_pkg_storage_engine.TestBatchBasics (t=0xc420b180f0)\r\n    at /home/tduberstein/local/go/src/github.com/cockroachdb/cockroach/pkg/storage/engine/batch_test.go:132\r\n#13 0x00007ffff6d900d0 in testing.tRunner (param=<optimized out>, fn=<optimized out>) at ../../../src/libgo/go/testing/testing.go:777\r\n#14 0x00007ffff6e20975 in __morestack () at ../../../src/libgcc/config/i386/morestack.S:546\r\n#15 0x00007ffff6d901cb in testing.testing..thunk24 (__go_thunk_parameter=<optimized out>) at ../../../src/libgo/go/testing/testing.go:824\r\n#16 0x00007ffff6d18a79 in runtime.kickoff () at ../../../src/libgo/go/runtime/proc.go:1161\r\n#17 0x00007ffff59f0d40 in ?? () from /lib64/libc.so.6\r\n```\r\nconfirming that the passed pointer points to a null pointer:\r\n```\r\n(gdb) frame 8\r\n#8  0x0000000000f3c040 in github_com_cockroachdb_cockroach_pkg_storage_engine.RocksDB.open (r=0xc420a3c580)\r\n    at /home/tduberstein/local/go/src/github.com/cockroachdb/cockroach/pkg/storage/engine/rocksdb.go:584\r\n584             status := C.DBOpen(&r.rdb, goToCSlice([]byte(r.cfg.Dir)),\r\n(gdb) print r.rdb\r\n$4 = (.github.com/cockroachdb/cockroach/pkg/storage/engine._Ctype_struct_DBEngine *) 0x0\r\n```\r\ninteresting stuff starts at frame 5:\r\n```\r\n(gdb) frame 5\r\n#5  0x00007ffff6d015bb in runtime.cgoCheckPointer (ptr=..., args=...) at ../../../src/libgo/go/runtime/cgocall.go:93\r\n93              cgoCheckArg(t, ep.data, t.kind&kindDirectIface == 0, top, cgoCheckPointerFail)\r\n(gdb) list\r\n88                      default:\r\n89                              throw(\"can't happen\")\r\n90                      }\r\n91              }\r\n92\r\n93              cgoCheckArg(t, ep.data, t.kind&kindDirectIface == 0, top, cgoCheckPointerFail)\r\n94      }\r\n95\r\n96      const cgoCheckPointerFail = \"cgo argument has Go pointer to Go pointer\"\r\n97      const cgoResultFail = \"cgo result has Go pointer\"\r\n```\r\nthings we care about are arguments to `cgoCheckArg`:\r\n```\r\n(gdb) print t\r\n$9 = (.runtime._type *) 0x1e645a0 <github_com_cockroachdb_cockroach_pkg_storage_engine..github_com_cockroachdb_cockroach_pkg_storage_engine._Ctype_struct___7..d>\r\n(gdb) print ep.data\r\nvalue has been optimized out\r\n(gdb) print t.kind&(1<<5)\r\n$10 = 0\r\n(gdb) print top\r\n$11 = <optimized out>\r\n```\r\nso we don't know the value of `top`, but it's always true if `len(args) == 0`:\r\n```\r\n(gdb) print args\r\n$12 = {__values = <optimized out>, __count = 0, __capacity = <optimized out>}\r\n```\r\ninterestingly, `t` is different than what I mistakenly thought it was previously:\r\n```\r\n(gdb) print *t.string\r\n$13 = 0x1e34470 \"\\tgithub_com_cockroachdb_cockroach_pkg_storage_engine\\tengine._Ctype_struct___7\"\r\n```\r\nnext frame:\r\n```\r\n(gdb) frame 4\r\n#4  0x00007ffff6d0124c in runtime.cgoCheckArg (\r\n    t=t@entry=0x1e645a0 <github_com_cockroachdb_cockroach_pkg_storage_engine..github_com_cockroachdb_cockroach_pkg_storage_engine._Ctype_struct___7..d>, p=<optimized out>, indir=<optimized out>, top=<optimized out>, msg=...) at ../../../src/libgo/go/runtime/cgocall.go:192\r\n192                             cgoCheckArg(f.typ, add(p, f.offset), true, top, msg)\r\n(gdb) list\r\n187                             }\r\n188                             cgoCheckArg(st.fields[0].typ, p, st.fields[0].typ.kind&kindDirectIface == 0, top, msg)\r\n189                             return\r\n190                     }\r\n191                     for _, f := range st.fields {\r\n192                             cgoCheckArg(f.typ, add(p, f.offset), true, top, msg)\r\n193                     }\r\n194             case kindPtr, kindUnsafePointer:\r\n195                     if indir {\r\n196                             p = *(*unsafe.Pointer)(p)\r\n```\r\nso this type is a struct (that's where we are in the type switch). we know `indir` is true from the previous frame, so we go straight into this `range`. unfortunately, `f` is optimized out and the next few frames have been inlined. frame 2:\r\n```\r\n(gdb) frame 2\r\n#2  runtime.cgoCheckArg (t=<optimized out>, p=<optimized out>, indir=indir@entry=true, top=top@entry=true, msg=...)\r\n    at ../../../src/libgo/go/runtime/cgocall.go:206\r\n206                     cgoCheckUnknownPointer(p, msg)\r\n(gdb) list\r\n201                     }\r\n202                     if !top {\r\n203                             panic(errorString(msg))\r\n204                     }\r\n205\r\n206                     cgoCheckUnknownPointer(p, msg)\r\n207             }\r\n208     }\r\n209\r\n210     // cgoCheckUnknownPointer is called for an arbitrary pointer into Go\r\n```\r\nthis time we land in the `kindPtr, kindUnsafePointer` case, and:\r\n```\r\n(gdb) print indir\r\n$27 = true\r\n(gdb) print top\r\n$28 = true\r\n```\r\nso we execute `p = *(*unsafe.Pointer)(p)` (it's just above the code pasted above) and proceed into `cgoCheckUnknownPointer`:\r\n```\r\n(gdb) frame 1\r\n#1  0x00007ffff6d0114b in runtime.cgoCheckUnknownPointer (msg=..., p=<optimized out>) at ../../../src/libgo/go/runtime/cgocall.go:238\r\n238                                     panic(errorString(msg))\r\n(gdb) list\r\n233                             if i != 1*sys.PtrSize && !hbits.morePointers() {\r\n234                                     // No more possible pointers.\r\n235                                     break\r\n236                             }\r\n237                             if hbits.isPointer() && cgoIsGoPointer(*(*unsafe.Pointer)(unsafe.Pointer(base + i))) {\r\n238                                     panic(errorString(msg))\r\n239                             }\r\n240                             hbits = hbits.next()\r\n241                     }\r\n242\r\n```\r\nthis is what's surpring. at this point, after the dereference, I would expect to end up with the null pointer we started with, but that's obviously not the case. the actual value of `p` is optimized out, but obviously `cgoInRange` returned true for it. there's more dereferencing going on here which I don't understand. Here's what information remains in this stack frame:\r\n```\r\n(gdb) info locals\r\nspan = 0x7ffff5658c88\r\nb = 842351250160\r\nhbits = {bitp = 0xc41fff9fa8 \"Y\\360\\363\\322t\\360\\374Y\\360\\363\", <incomplete sequence \\322>, shift = 3}\r\nsink$0 = 25\r\nn = 112\r\nroots = <optimized out>\r\nbase = 842351250160\r\ni = <optimized out>\r\n```\r\nlet me know if there's more detail I can provide."}}
{"actor":{"display_login":"tamird"}}