go/internal/srcimporter: TestImportStdLib failing on NetBSD

[ianlancetaylor]

TestImportStdLib is failing on NetBSD on tip. As seen on

https://build.golang.org/log/fad56ee11c8b5522a96342ee2a936845758f1ab0

--- FAIL: TestImportStdLib (2.10s)
    srcimporter_test.go:30: import "archive/tar" failed (type-checking package "archive/tar" failed (/tmp/workdir/go/src/archive/tar/stat_unix.go:11:2: could not import os/user (type-checking package "os/user" failed (/tmp/workdir/go/src/os/user/getgrouplist_unix.go:25:43: gid_t not declared by package C))))
    srcimporter_test.go:43: testing time used up
    srcimporter_test.go:89: tested 1 imports
FAIL
FAIL	go/internal/srcimporter	6.949s

CC @mdempsky @griesemer

[ianlancetaylor]

Based on the build dashboard, this appears to have been broken by https://golang.org/cl/33677.

[mdempsky]

Thanks for the heads up. I don't immediately see why that wouldn't work.

If someone can run go build -work os/user on NetBSD and then post os/user's _cgo_gotypes.go file, that would be handy.

[ianlancetaylor]

On NetBSD, <sys/types.h> says

#ifndef gid_t
typedef __gid_t         gid_t;          /* group id */
#define gid_t           __gid_t
#endif

The type __gid_t is defined in <sys/ansi.h>, included by <sys/types.h>, as __uint32_t.

Because of the macro definition in <sys/types.h>, the generated _cgo_gotypes.go file defines _Ctype__gid_t and uses that wherever the original Go code uses C.gid_t. I assume that is confusing the rest of the code into thinking that there is no definition for C.gid_t. In the general case I think this code is going to need to be aware of preprocessor macros.

[mdempsky]

@ianlancetaylor I see. That's frustrating.

Does cgo emit any information about preprocessor macros that go/types can make use of?

Could cgo emit a type alias for type _Ctype_gid_t = _Ctype___gid_t in this scenario?

[ianlancetaylor]

_cgo_gotypes.go on amd64 NetBSD 9.0:

//go:cgo_ldflag "-g"
//go:cgo_ldflag "-O2"
// Code generated by cmd/cgo; DO NOT EDIT.

package user

import "unsafe"

import _ "runtime/cgo"

import "syscall"

var _ syscall.Errno
func _Cgo_ptr(ptr unsafe.Pointer) unsafe.Pointer { return ptr }

//go:linkname _Cgo_always_false runtime.cgoAlwaysFalse
var _Cgo_always_false bool
//go:linkname _Cgo_use runtime.cgoUse
func _Cgo_use(interface{})
type _Ctype___gid_t = _Ctype___uint32_t

type _Ctype___int64_t = _Ctype_long

type _Ctype___uid_t = _Ctype___uint32_t

type _Ctype___uint32_t = _Ctype_uint

type _Ctype_char int8

type _Ctype_int int32

type _Ctype_intgo = _Ctype_ptrdiff_t

type _Ctype_long int64

type _Ctype_ptrdiff_t = _Ctype_long

type _Ctype_size_t = _Ctype_ulong

type _Ctype_struct_group struct {
        gr_name         *_Ctype_char
        gr_passwd       *_Ctype_char
        gr_gid          _Ctype___gid_t
        gr_mem          **_Ctype_char
}

type _Ctype_struct_passwd struct {
        pw_name         *_Ctype_char
        pw_passwd       *_Ctype_char
        pw_uid          _Ctype___uid_t
        pw_gid          _Ctype___gid_t
        pw_change       _Ctype_time_t
        pw_class        *_Ctype_char
        pw_gecos        *_Ctype_char
        pw_dir          *_Ctype_char
        pw_shell        *_Ctype_char
        pw_expire       _Ctype_time_t
}

type _Ctype_time_t = _Ctype___int64_t

type _Ctype_uint uint32

type _Ctype_ulong uint64

type _Ctype_void [0]byte

//go:linkname _cgo_runtime_cgocall runtime.cgocall
func _cgo_runtime_cgocall(unsafe.Pointer, uintptr) int32

//go:linkname _cgo_runtime_cgocallback runtime.cgocallback
func _cgo_runtime_cgocallback(unsafe.Pointer, unsafe.Pointer, uintptr, uintptr)

//go:linkname _cgoCheckPointer runtime.cgoCheckPointer
func _cgoCheckPointer(interface{}, interface{})

//go:linkname _cgoCheckResult runtime.cgoCheckResult
func _cgoCheckResult(interface{})
const _Ciconst__SC_GETGR_R_SIZE_MAX = 0x2f
const _Ciconst__SC_GETPW_R_SIZE_MAX = 0x30


//go:linkname _cgo_runtime_gostring runtime.gostring
func _cgo_runtime_gostring(*_Ctype_char) string

func _Cfunc_GoString(p *_Ctype_char) string {
        return _cgo_runtime_gostring(p)
}

func _Cfunc__CMalloc(n _Ctype_size_t) unsafe.Pointer {
        return _cgo_cmalloc(uint64(n))
}
//go:cgo_import_static _cgo_bbce630bddcc_Cfunc_free
//go:linkname __cgofn__cgo_bbce630bddcc_Cfunc_free _cgo_bbce630bddcc_Cfunc_free
var __cgofn__cgo_bbce630bddcc_Cfunc_free byte
var _cgo_bbce630bddcc_Cfunc_free = unsafe.Pointer(&__cgofn__cgo_bbce630bddcc_Cfunc_free)

//go:cgo_unsafe_args
func _Cfunc_free(p0 unsafe.Pointer) (r1 _Ctype_void) {
        _cgo_runtime_cgocall(_cgo_bbce630bddcc_Cfunc_free, uintptr(unsafe.Pointer(&p0)))
        if _Cgo_always_false {
                _Cgo_use(p0)
        }
        return
}
//go:cgo_import_static _cgo_bbce630bddcc_Cfunc_mygetgrgid_r
//go:linkname __cgofn__cgo_bbce630bddcc_Cfunc_mygetgrgid_r _cgo_bbce630bddcc_Cfunc_mygetgrgid_r
var __cgofn__cgo_bbce630bddcc_Cfunc_mygetgrgid_r byte
var _cgo_bbce630bddcc_Cfunc_mygetgrgid_r = unsafe.Pointer(&__cgofn__cgo_bbce630bddcc_Cfunc_mygetgrgid_r)

//go:cgo_unsafe_args
func _Cfunc_mygetgrgid_r(p0 _Ctype_int, p1 *_Ctype_struct_group, p2 *_Ctype_char, p3 _Ctype_size_t, p4 **_Ctype_struct_group) (r1 _Ctype_int) {
        _cgo_runtime_cgocall(_cgo_bbce630bddcc_Cfunc_mygetgrgid_r, uintptr(unsafe.Pointer(&p0)))
        if _Cgo_always_false {
                _Cgo_use(p0)
                _Cgo_use(p1)
                _Cgo_use(p2)
                _Cgo_use(p3)
                _Cgo_use(p4)
        }
        return
}
//go:cgo_import_static _cgo_bbce630bddcc_Cfunc_mygetgrnam_r
//go:linkname __cgofn__cgo_bbce630bddcc_Cfunc_mygetgrnam_r _cgo_bbce630bddcc_Cfunc_mygetgrnam_r
var __cgofn__cgo_bbce630bddcc_Cfunc_mygetgrnam_r byte
var _cgo_bbce630bddcc_Cfunc_mygetgrnam_r = unsafe.Pointer(&__cgofn__cgo_bbce630bddcc_Cfunc_mygetgrnam_r)

//go:cgo_unsafe_args
func _Cfunc_mygetgrnam_r(p0 *_Ctype_char, p1 *_Ctype_struct_group, p2 *_Ctype_char, p3 _Ctype_size_t, p4 **_Ctype_struct_group) (r1 _Ctype_int) {
        _cgo_runtime_cgocall(_cgo_bbce630bddcc_Cfunc_mygetgrnam_r, uintptr(unsafe.Pointer(&p0)))
        if _Cgo_always_false {
                _Cgo_use(p0)
                _Cgo_use(p1)
                _Cgo_use(p2)
                _Cgo_use(p3)
                _Cgo_use(p4)
        }
        return
}
//go:cgo_import_static _cgo_bbce630bddcc_Cfunc_mygetgrouplist
//go:linkname __cgofn__cgo_bbce630bddcc_Cfunc_mygetgrouplist _cgo_bbce630bddcc_Cfunc_mygetgrouplist
var __cgofn__cgo_bbce630bddcc_Cfunc_mygetgrouplist byte
var _cgo_bbce630bddcc_Cfunc_mygetgrouplist = unsafe.Pointer(&__cgofn__cgo_bbce630bddcc_Cfunc_mygetgrouplist)

//go:cgo_unsafe_args
func _Cfunc_mygetgrouplist(p0 *_Ctype_char, p1 _Ctype___gid_t, p2 *_Ctype___gid_t, p3 *_Ctype_int) (r1 _Ctype_int) {
        _cgo_runtime_cgocall(_cgo_bbce630bddcc_Cfunc_mygetgrouplist, uintptr(unsafe.Pointer(&p0)))
        if _Cgo_always_false {
                _Cgo_use(p0)
                _Cgo_use(p1)
                _Cgo_use(p2)
                _Cgo_use(p3)
        }
        return
}
//go:cgo_import_static _cgo_bbce630bddcc_Cfunc_mygetpwnam_r
//go:linkname __cgofn__cgo_bbce630bddcc_Cfunc_mygetpwnam_r _cgo_bbce630bddcc_Cfunc_mygetpwnam_r
var __cgofn__cgo_bbce630bddcc_Cfunc_mygetpwnam_r byte
var _cgo_bbce630bddcc_Cfunc_mygetpwnam_r = unsafe.Pointer(&__cgofn__cgo_bbce630bddcc_Cfunc_mygetpwnam_r)

//go:cgo_unsafe_args
func _Cfunc_mygetpwnam_r(p0 *_Ctype_char, p1 *_Ctype_struct_passwd, p2 *_Ctype_char, p3 _Ctype_size_t, p4 **_Ctype_struct_passwd) (r1 _Ctype_int) {
        _cgo_runtime_cgocall(_cgo_bbce630bddcc_Cfunc_mygetpwnam_r, uintptr(unsafe.Pointer(&p0)))
        if _Cgo_always_false {
                _Cgo_use(p0)
                _Cgo_use(p1)
                _Cgo_use(p2)
                _Cgo_use(p3)
                _Cgo_use(p4)
        }
        return
}
//go:cgo_import_static _cgo_bbce630bddcc_Cfunc_mygetpwuid_r
//go:linkname __cgofn__cgo_bbce630bddcc_Cfunc_mygetpwuid_r _cgo_bbce630bddcc_Cfunc_mygetpwuid_r
var __cgofn__cgo_bbce630bddcc_Cfunc_mygetpwuid_r byte
var _cgo_bbce630bddcc_Cfunc_mygetpwuid_r = unsafe.Pointer(&__cgofn__cgo_bbce630bddcc_Cfunc_mygetpwuid_r)

//go:cgo_unsafe_args
func _Cfunc_mygetpwuid_r(p0 _Ctype_int, p1 *_Ctype_struct_passwd, p2 *_Ctype_char, p3 _Ctype_size_t, p4 **_Ctype_struct_passwd) (r1 _Ctype_int) {
        _cgo_runtime_cgocall(_cgo_bbce630bddcc_Cfunc_mygetpwuid_r, uintptr(unsafe.Pointer(&p0)))
        if _Cgo_always_false {
                _Cgo_use(p0)
                _Cgo_use(p1)
                _Cgo_use(p2)
                _Cgo_use(p3)
                _Cgo_use(p4)
        }
        return
}
//go:cgo_import_static _cgo_bbce630bddcc_Cfunc_realloc
//go:linkname __cgofn__cgo_bbce630bddcc_Cfunc_realloc _cgo_bbce630bddcc_Cfunc_realloc
var __cgofn__cgo_bbce630bddcc_Cfunc_realloc byte
var _cgo_bbce630bddcc_Cfunc_realloc = unsafe.Pointer(&__cgofn__cgo_bbce630bddcc_Cfunc_realloc)

//go:cgo_unsafe_args
func _Cfunc_realloc(p0 unsafe.Pointer, p1 _Ctype_size_t) (r1 unsafe.Pointer) {
        _cgo_runtime_cgocall(_cgo_bbce630bddcc_Cfunc_realloc, uintptr(unsafe.Pointer(&p0)))
        if _Cgo_always_false {
                _Cgo_use(p0)
                _Cgo_use(p1)
        }
        return
}
//go:cgo_import_static _cgo_bbce630bddcc_Cfunc_sysconf
//go:linkname __cgofn__cgo_bbce630bddcc_Cfunc_sysconf _cgo_bbce630bddcc_Cfunc_sysconf
var __cgofn__cgo_bbce630bddcc_Cfunc_sysconf byte
var _cgo_bbce630bddcc_Cfunc_sysconf = unsafe.Pointer(&__cgofn__cgo_bbce630bddcc_Cfunc_sysconf)

//go:cgo_unsafe_args
func _Cfunc_sysconf(p0 _Ctype_int) (r1 _Ctype_long) {
        _cgo_runtime_cgocall(_cgo_bbce630bddcc_Cfunc_sysconf, uintptr(unsafe.Pointer(&p0)))
        if _Cgo_always_false {
                _Cgo_use(p0)
        }
        return
}

//go:cgo_import_static _cgo_bbce630bddcc_Cfunc__Cmalloc
//go:linkname __cgofn__cgo_bbce630bddcc_Cfunc__Cmalloc _cgo_bbce630bddcc_Cfunc__Cmalloc
var __cgofn__cgo_bbce630bddcc_Cfunc__Cmalloc byte
var _cgo_bbce630bddcc_Cfunc__Cmalloc = unsafe.Pointer(&__cgofn__cgo_bbce630bddcc_Cfunc__Cmalloc)

//go:linkname runtime_throw runtime.throw
func runtime_throw(string)

//go:cgo_unsafe_args
func _cgo_cmalloc(p0 uint64) (r1 unsafe.Pointer) {
        _cgo_runtime_cgocall(_cgo_bbce630bddcc_Cfunc__Cmalloc, uintptr(unsafe.Pointer(&p0)))
        if r1 == nil {
                runtime_throw("runtime: C malloc failed")
        }
        return
}

[ianlancetaylor]

If you pass -debug-define to cgo it will emit #define lines on stderr that you can read to pick out redefinitions. I think that should suffice to handle this case. Maybe. Give it a try.

[mdempsky]

Can repro on !netbsd by adding $GOROOT/misc/cgo/test/issue38649.go with contents:

package cgotest

// #define issue38649int int
import "C"

var _ C.issue38649int

and then running go test -run=TestCgo go/internal/srcimporter.

[mdempsky]

@ianlancetaylor Thanks, I'll take a look at the -debug-define output. That might work for srcimporter, because it runs cgo itself; but in general, we don't currently have control over how cgo is invoked. E.g., go/packages on its underlying driver (e.g., cmd/go) to invoke cmd/cgo and supply the generated _cgo_gotypes.go file.

[ianlancetaylor]

I don't know the issues here, but we could perhaps change cgo to emit special comments for macros in the _cgo_gotypes.go file.

[mdempsky]

@ianlancetaylor The goal is given the unmodified original Go source files and the _cgo_gotypes.go file generated from running cgo on them, to have go/types able to figure out the Go type of all C.foo identifiers.

The way CL 33677 does that is it has a list of all the prefixes that cgo currently emits, and it tries looking each of them up. E.g., for C.foo it looks up _Ciconst_foo, _Cfconst_foo, _Ctype_foo, _Cvar_foo, ... until one succeeds. It also knows like that _Cvar_foo is actually a pointer to the variable and so it needs an implicit dereference, or that _Cmacro_foo is a function that needs to be called to evaluate to the value.

The problem is here C.gid_t is being translated by cgo into an identifier not found using this approach.

It would be best for go/types if the special comments were spelled type _Ctype_gid_t = _Ctype___gid_t. :)

[mdempsky]

Looking at (*Package).rewriteName, it looks like the expr = r.Name.Type.Go lines are the concerning ones.

It looks like those lines predate type aliases. I'd imagine we can just emit the type alias under the normal mangled name, and then use the normal mangling. I'll look into this.

[gopherbot]

Change https://golang.org/cl/230037 mentions this issue: cmd/cgo: use type aliases for #define type macros

[gopherbot]

Change https://golang.org/cl/230038 mentions this issue: Revert "go/types: add UsesCgo config to support _cgo_gotypes.go"

[andybons]

Seems that there are multiple failures being masked: reverting https://golang.org/cl/33677 gives a different test failure on NetBSD:

--- FAIL: TestSegv (0.00s)

    --- FAIL: TestSegv/Segv (0.01s)
        crash_cgo_test.go:569: 
        crash_cgo_test.go:571: expected crash from signal
    --- FAIL: TestSegv/SegvInCgo (0.01s)
        crash_cgo_test.go:569: 
        crash_cgo_test.go:571: expected crash from signal
FAIL
FAIL	runtime	39.518s

If this is due to a different change, I’d really like us to revert liberally and soon to get us back into a passing state.

[gopherbot]

Change https://golang.org/cl/230309 mentions this issue: go/internal/srcimporter: disable TestCgo on NetBSD

[josharian]

That is #38595 and it was supposed to be fixed by https://go-review.googlesource.com/c/go/+/229484. cc @ianlancetaylor

[andybons]

Thanks, @josharian. Will keep an eye the builders and report back on that issue if it’s still failing with the same TestSegv failure.